Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Restriction parameter to prevent more than 7 parameters in the next PRs #693

Merged
merged 2 commits into from
Sep 16, 2022
Merged

Restriction parameter to prevent more than 7 parameters in the next PRs #693

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f14f456
Restriction as parameter to prevent more than 7 parameters in methods…
sultan Sep 14, 2022
eaac20e
Restriction as parameter to prevent more than 7 parameters in methods…
sultan Sep 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions src/main/java/org/codehaus/mojo/versions/AbstractVersionsReport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,18 +234,10 @@ protected abstract void doGenerateReport( Locale locale, Sink sink )
* @since 1.0-alpha-1
*/
protected ArtifactVersion findLatestVersion( Artifact artifact, VersionRange versionRange,
boolean allowingSnapshots, boolean usePluginRepositories )
Boolean allowingSnapshots, boolean usePluginRepositories )
throws MavenReportException
{
boolean includeSnapshots = this.allowSnapshots;
if ( allowingSnapshots )
{
includeSnapshots = true;
}
if ( allowingSnapshots )
{
includeSnapshots = false;
}
Comment on lines -240 to -248
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting ... looks like never was working

Copy link
Contributor

@andrzejj0 andrzejj0 Sep 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, not really. It kinda worked, but I really doubt its usefulness. I contemplated removing it too, but I saw it being used.

this.allowSnapshots was a Boolean value, could also be null. Local allowingSnaphots was a primitive boolean, but it was a kind of an override for the value in the Mojo.

So, if allowingSnapshots method argument was set, the code overrote the value from the Mojo object with the method argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two methods with Boolean overrides. One of them was with the boolean primitive and seems not used. I did not felt removing this one. Might consider marking deprecated ?

boolean includeSnapshots = allowingSnapshots != null ? allowingSnapshots : this.allowSnapshots;
try
{
final ArtifactVersions artifactVersions =
Expand Down
13 changes: 2 additions & 11 deletions src/main/java/org/codehaus/mojo/versions/AbstractVersionsUpdaterMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,18 +313,9 @@ protected ArtifactVersion findLatestVersion( Artifact artifact, VersionRange ver
boolean allowDowngrade )
throws ArtifactMetadataRetrievalException, MojoExecutionException
{
boolean includeSnapshots = this.allowSnapshots;
if ( Boolean.TRUE.equals( allowingSnapshots ) )
{
includeSnapshots = true;
}
if ( Boolean.FALSE.equals( allowingSnapshots ) )
{
includeSnapshots = false;
}
boolean includeSnapshots = allowingSnapshots != null ? allowingSnapshots : this.allowSnapshots;
final ArtifactVersions artifactVersions = getHelper().lookupArtifactVersions( artifact, usePluginRepositories );
return artifactVersions.getNewestVersion( versionRange, null, null, includeSnapshots,
true, true, allowDowngrade );
return artifactVersions.getNewestVersion( versionRange, null, includeSnapshots, allowDowngrade );
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/org/codehaus/mojo/versions/UseLatestSnapshotsMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.apache.maven.artifact.metadata.ArtifactMetadataRetrievalException;
import org.apache.maven.artifact.versioning.ArtifactVersion;
import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
import org.apache.maven.artifact.versioning.Restriction;
import org.apache.maven.model.Dependency;
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.plugin.MojoFailureException;
Expand Down Expand Up @@ -173,7 +174,8 @@ private void useLatestSnapshots( ModifiedPomXMLEventReader pom, Collection<Depen
ArtifactVersion upperBound =
segment >= 0 ? versionComparator.incrementSegment( lowerBound, segment ) : null;
getLog().info( "Upper bound: " + ( upperBound == null ? "none" : upperBound.toString() ) );
ArtifactVersion[] newer = versions.getVersions( lowerBound, upperBound, true, false, false );
Restriction restriction = new Restriction( lowerBound, false, upperBound, false );
ArtifactVersion[] newer = versions.getVersions( restriction, true );
getLog().debug( "Candidate versions " + Arrays.asList( newer ) );

String latestVersion;
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/org/codehaus/mojo/versions/UseNextSnapshotsMojo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.apache.maven.artifact.metadata.ArtifactMetadataRetrievalException;
import org.apache.maven.artifact.versioning.ArtifactVersion;
import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
import org.apache.maven.artifact.versioning.Restriction;
import org.apache.maven.model.Dependency;
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.plugin.MojoFailureException;
Expand Down Expand Up @@ -155,7 +156,8 @@ private void useNextSnapshots( ModifiedPomXMLEventReader pom, Collection<Depende
ArtifactVersion upperBound =
segment >= 0 ? versionComparator.incrementSegment( lowerBound, segment ) : null;
getLog().info( "Upper bound: " + ( upperBound == null ? "none" : upperBound.toString() ) );
ArtifactVersion[] newer = versions.getVersions( lowerBound, upperBound, true, false, false );
Restriction restriction = new Restriction( lowerBound, false, upperBound, false );
ArtifactVersion[] newer = versions.getVersions( restriction, true );
getLog().debug( "Candidate versions " + Arrays.asList( newer ) );
for ( ArtifactVersion artifactVersion : newer )
{
Expand Down
102 changes: 42 additions & 60 deletions src/main/java/org/codehaus/mojo/versions/api/AbstractVersionDetails.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.apache.maven.artifact.ArtifactUtils;
import org.apache.maven.artifact.versioning.ArtifactVersion;
import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
import org.apache.maven.artifact.versioning.Restriction;
import org.apache.maven.artifact.versioning.VersionRange;
import org.codehaus.mojo.versions.ordering.InvalidSegmentException;
import org.codehaus.mojo.versions.ordering.VersionComparator;
Expand Down Expand Up @@ -129,23 +130,25 @@ public final ArtifactVersion[] getVersions()

public final ArtifactVersion[] getVersions( VersionRange versionRange, boolean includeSnapshots )
{
return getVersions( versionRange, null, null, includeSnapshots, true, true );
return getVersions( versionRange, null, includeSnapshots );
}

public final ArtifactVersion[] getVersions( ArtifactVersion currentVersion, ArtifactVersion upperBound )
public final ArtifactVersion[] getVersions( ArtifactVersion lowerBound, ArtifactVersion upperBound )
{
return getVersions( currentVersion, upperBound, isIncludeSnapshots() );
return getVersions( lowerBound, upperBound, isIncludeSnapshots() );
}

public final ArtifactVersion[] getVersions( ArtifactVersion currentVersion, ArtifactVersion upperBound,
public final ArtifactVersion[] getVersions( ArtifactVersion lowerBound, ArtifactVersion upperBound,
boolean includeSnapshots )
{
return getVersions( currentVersion, upperBound, includeSnapshots, false, false );
Restriction restriction = new Restriction( lowerBound, false, upperBound, false );
return getVersions( restriction, includeSnapshots );
}

private ArtifactVersion[] getNewerVersions( ArtifactVersion version, boolean includeSnapshots )
{
return getVersions( version, null, includeSnapshots, false, true );
Restriction restriction = new Restriction( version, false, null, false );
return getVersions( restriction, includeSnapshots );
}

public final ArtifactVersion getNewestVersion( ArtifactVersion lowerBound, ArtifactVersion upperBound )
Expand All @@ -156,25 +159,23 @@ public final ArtifactVersion getNewestVersion( ArtifactVersion lowerBound, Artif
public final ArtifactVersion getNewestVersion( ArtifactVersion lowerBound, ArtifactVersion upperBound,
boolean includeSnapshots )
{
return getNewestVersion( lowerBound, upperBound, includeSnapshots, false, false );
Restriction restriction = new Restriction( lowerBound, false, upperBound, false );
return getNewestVersion( restriction, includeSnapshots );
}

public final ArtifactVersion getNewestVersion( VersionRange versionRange, ArtifactVersion lowerBound,
ArtifactVersion upperBound, boolean includeSnapshots,
boolean includeLower, boolean includeUpper )
public final ArtifactVersion getNewestVersion( VersionRange versionRange, Restriction restriction,
boolean includeSnapshots )
{
return getNewestVersion( versionRange, lowerBound, upperBound, includeSnapshots, includeLower,
includeUpper, false );
return getNewestVersion( versionRange, restriction, includeSnapshots, false );
}

private static <T> Iterable<T> reverse( T[] array )
{
return Arrays.stream( array ).sorted( Collections.reverseOrder() ).collect( Collectors.toList() );
}

public final ArtifactVersion getNewestVersion( VersionRange versionRange, ArtifactVersion lowerBound,
ArtifactVersion upperBound, boolean includeSnapshots,
boolean includeLower, boolean includeUpper, boolean allowDowngrade )
public final ArtifactVersion getNewestVersion( VersionRange versionRange, Restriction restriction,
boolean includeSnapshots, boolean allowDowngrade )
{
final VersionComparator versionComparator = getVersionComparator();
// reverse( getVersions( ... ) ) will contain versions sorted from latest to oldest,
Expand All @@ -186,13 +187,7 @@ public final ArtifactVersion getNewestVersion( VersionRange versionRange, Artifa
{
continue;
}
int lower = lowerBound == null ? -1 : versionComparator.compare( lowerBound, candidate );
int upper = upperBound == null ? +1 : versionComparator.compare( upperBound, candidate );
if ( lower > 0 || upper < 0 )
{
continue;
}
if ( ( !includeLower && lower == 0 ) || ( !includeUpper && upper == 0 ) )
if ( restriction != null && !restriction.containsVersion( candidate ) )
{
continue;
}
Expand All @@ -205,16 +200,14 @@ public final ArtifactVersion getNewestVersion( VersionRange versionRange, Artifa
return null;
}

public final ArtifactVersion getNewestVersion( ArtifactVersion lowerBound, ArtifactVersion upperBound,
boolean includeSnapshots, boolean includeLower,
boolean includeUpper )
public final ArtifactVersion getNewestVersion( Restriction restriction, boolean includeSnapshots )
{
return getNewestVersion( null, lowerBound, upperBound, includeSnapshots, includeLower, includeUpper );
return getNewestVersion( null, restriction, includeSnapshots );
}

public final ArtifactVersion getNewestVersion( VersionRange versionRange, boolean includeSnapshots )
{
return getNewestVersion( versionRange, null, null, includeSnapshots, true, true );
return getNewestVersion( versionRange, null, includeSnapshots );
}

public final boolean containsVersion( String version )
Expand Down Expand Up @@ -276,7 +269,8 @@ public final ArtifactVersion[] getNewerVersions( String versionString, int upper
ArtifactVersion upperBound = upperBoundSegment == -1 ? null
: getVersionComparator().incrementSegment( lowerBound, upperBoundSegment );

return getVersions( lowerBound, upperBound, includeSnapshots, allowDowngrade, allowDowngrade );
Restriction restriction = new Restriction( lowerBound, allowDowngrade, upperBound, allowDowngrade );
return getVersions( restriction, includeSnapshots );
}

public final ArtifactVersion getOldestVersion( ArtifactVersion lowerBound, ArtifactVersion upperBound )
Expand All @@ -286,25 +280,24 @@ public final ArtifactVersion getOldestVersion( ArtifactVersion lowerBound, Artif

public final ArtifactVersion getOldestVersion( VersionRange versionRange, boolean includeSnapshots )
{
return getOldestVersion( versionRange, null, null, includeSnapshots, true, true );
return getOldestVersion( versionRange, null, includeSnapshots );
}

public final ArtifactVersion getOldestVersion( ArtifactVersion lowerBound, ArtifactVersion upperBound,
boolean includeSnapshots )
{
return getOldestVersion( lowerBound, upperBound, includeSnapshots, false, false );
Restriction restriction = new Restriction( lowerBound, false, upperBound, false );
return getOldestVersion( restriction, includeSnapshots );
}

public final ArtifactVersion getOldestVersion( ArtifactVersion lowerBound, ArtifactVersion upperBound,
boolean includeSnapshots, boolean includeLower,
boolean includeUpper )
public final ArtifactVersion getOldestVersion( Restriction restriction,
boolean includeSnapshots )
{
return getOldestVersion( null, lowerBound, upperBound, includeSnapshots, includeLower, includeUpper );
return getOldestVersion( null, restriction, includeSnapshots );
}

public final ArtifactVersion getOldestVersion( VersionRange versionRange, ArtifactVersion lowerBound,
ArtifactVersion upperBound, boolean includeSnapshots,
boolean includeLower, boolean includeUpper )
public final ArtifactVersion getOldestVersion( VersionRange versionRange, Restriction restriction,
boolean includeSnapshots )
{
ArtifactVersion oldest = null;
final VersionComparator versionComparator = getVersionComparator();
Expand All @@ -314,13 +307,7 @@ public final ArtifactVersion getOldestVersion( VersionRange versionRange, Artifa
{
continue;
}
int lower = lowerBound == null ? -1 : versionComparator.compare( lowerBound, candidate );
int upper = upperBound == null ? +1 : versionComparator.compare( upperBound, candidate );
if ( lower > 0 || upper < 0 )
{
continue;
}
if ( ( !includeLower && lower == 0 ) || ( !includeUpper && upper == 0 ) )
if ( restriction != null && !restriction.containsVersion( candidate ) )
{
continue;
}
Expand All @@ -340,15 +327,13 @@ else if ( versionComparator.compare( oldest, candidate ) > 0 )
return oldest;
}

public final ArtifactVersion[] getVersions( ArtifactVersion lowerBound, ArtifactVersion upperBound,
boolean includeSnapshots, boolean includeLower, boolean includeUpper )
public final ArtifactVersion[] getVersions( Restriction restriction, boolean includeSnapshots )
{
return getVersions( null, lowerBound, upperBound, includeSnapshots, includeLower, includeUpper );
return getVersions( null, restriction, includeSnapshots );
}

public final ArtifactVersion[] getVersions( VersionRange versionRange, ArtifactVersion lowerBound,
ArtifactVersion upperBound, boolean includeSnapshots,
boolean includeLower, boolean includeUpper )
public final ArtifactVersion[] getVersions( VersionRange versionRange, Restriction restriction,
boolean includeSnapshots )
{
final VersionComparator versionComparator = getVersionComparator();
Set<ArtifactVersion> result = new TreeSet<>( versionComparator );
Expand All @@ -358,13 +343,7 @@ public final ArtifactVersion[] getVersions( VersionRange versionRange, ArtifactV
{
continue;
}
int lower = lowerBound == null ? -1 : versionComparator.compare( lowerBound, candidate );
int upper = upperBound == null ? +1 : versionComparator.compare( upperBound, candidate );
if ( lower > 0 || upper < 0 )
{
continue;
}
if ( ( !includeLower && lower == 0 ) || ( !includeUpper && upper == 0 ) )
if ( restriction != null && !restriction.containsVersion( candidate ) )
{
continue;
}
Expand Down Expand Up @@ -505,17 +484,20 @@ public final ArtifactVersion[] getAllUpdates( VersionRange versionRange )

public ArtifactVersion getOldestUpdate( VersionRange versionRange, boolean includeSnapshots )
{
return getOldestVersion( versionRange, getCurrentVersion(), null, includeSnapshots, false, true );
Restriction restriction = new Restriction( getCurrentVersion(), false, null, false );
return getOldestVersion( versionRange, restriction, includeSnapshots );
}

public ArtifactVersion getNewestUpdate( VersionRange versionRange, boolean includeSnapshots )
{
return getNewestVersion( versionRange, getCurrentVersion(), null, includeSnapshots, false, true );
Restriction restriction = new Restriction( getCurrentVersion(), false, null, false );
return getNewestVersion( versionRange, restriction, includeSnapshots );
}

public ArtifactVersion[] getAllUpdates( VersionRange versionRange, boolean includeSnapshots )
{
return getVersions( versionRange, getCurrentVersion(), null, includeSnapshots, false, true );
Restriction restriction = new Restriction( getCurrentVersion(), false, null, false );
return getVersions( versionRange, restriction, includeSnapshots );
}

/**
Expand Down
18 changes: 10 additions & 8 deletions src/main/java/org/codehaus/mojo/versions/api/PropertyVersions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.apache.maven.artifact.versioning.ArtifactVersion;
import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
import org.apache.maven.artifact.versioning.OverConstrainedVersionException;
import org.apache.maven.artifact.versioning.Restriction;
import org.apache.maven.artifact.versioning.VersionRange;
import org.apache.maven.project.MavenProject;
import org.codehaus.mojo.versions.Property;
Expand Down Expand Up @@ -342,25 +343,25 @@ public ArtifactVersion getNewestVersion( String currentVersion, Property propert
? VersionRange.createFromVersionSpec( property.getVersion() ) : null;
helper.getLog().debug( "Property ${" + property.getName() + "}: Restricting results to " + range );

ArtifactVersion lowerBoundArtifactVersion = helper.createArtifactVersion( currentVersion );
ArtifactVersion lowerBound = helper.createArtifactVersion( currentVersion );
if ( allowDowngrade )
{
Optional<String> updatedVersion = getLowerBound( lowerBoundArtifactVersion, unchangedSegment );
lowerBoundArtifactVersion = updatedVersion.map( helper::createArtifactVersion ).orElse( null );
Optional<String> updatedVersion = getLowerBound( lowerBound, unchangedSegment );
lowerBound = updatedVersion.map( helper::createArtifactVersion ).orElse( null );
}
if ( helper.getLog().isDebugEnabled() )
{
helper.getLog().debug( "lowerBoundArtifactVersion: " + lowerBoundArtifactVersion );
helper.getLog().debug( "lowerBoundArtifactVersion: " + lowerBound );
}

ArtifactVersion upperBound = null;
if ( unchangedSegment != -1 )
{
upperBound = getVersionComparator().incrementSegment( lowerBoundArtifactVersion, unchangedSegment );
upperBound = getVersionComparator().incrementSegment( lowerBound, unchangedSegment );
helper.getLog().debug( "Property ${" + property.getName() + "}: upperBound is: " + upperBound );
}
ArtifactVersion result =
getNewestVersion( range, lowerBoundArtifactVersion, upperBound, includeSnapshots, false, false );
Restriction restriction = new Restriction( lowerBound, false, upperBound, false );
ArtifactVersion result = getNewestVersion( range, restriction, includeSnapshots );

helper.getLog().debug( "Property ${" + property.getName() + "}: Current winner is: " + result );

Expand Down Expand Up @@ -427,7 +428,8 @@ private ArtifactVersion getNewestVersion( String currentVersion, VersionsHelper
{
upperBound = getVersionComparator().incrementSegment( lowerBound, segment );
}
return getNewestVersion( range, lowerBound, upperBound, includeSnapshots, false, false );
Restriction restriction = new Restriction( lowerBound, false, upperBound, false );
return getNewestVersion( range, restriction, includeSnapshots );
}

private final class PropertyVersionComparator
Expand Down
Loading