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.

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

Fixing #251: proper handling of InvalidSegmentException and InvalidVersionSpecificationException #666

Merged

Conversation

jarmoniuk
Copy link
Contributor

@jarmoniuk jarmoniuk commented Sep 2, 2022

The situations indicated in the exceptions were not handled properly throughout the code, in some places, MojoExcecutionException was simply being thrown leading to the whole run being stopped; InvalidVersionSpecificationException was unchecked, leading to the same.

Handling those situations in the Mojo itself rather than in the methods so that a proper handling could be effected where necessary.

Because of all that, a rather sizeable refactoring was due.

More PRs will follow, I want to refactor and polish the code more, but this one is what's necessary for resolving this issue.

So, like I said in the linked issue:

The more I look into it, the more issues I see:

  • segment appears to be 0-based (see e.g. AbstractVersionsUpaterMojo.determineUnchangedSegment or MavenVersionComparator.innerIncrementSegment), but the comparison against the segment count suffers from the 1-off problem:
if ( segment > segmentCount )
{
    throw new InvalidSegmentException( segment, segmentCount,
            version.toString() );
}
  • segment determination flags (allowMajorUpdates, etc.) are actually not at all independent, but are actually a sort of enum; so an actual enum or an unchanged segment index would be better;
  • part of the above: it's not actually possible to have allowMajorUpdates = true and allowMinorUpdates = false; allowMajorUpdates = true implies all others also being true;

There are probably more problems with this, but I'll limit the scope of this item to the issue OP raised only. The other findings will be done as separate issues.

@jarmoniuk
Copy link
Contributor Author

@slawekjaranowski please review 😄

@slawekjaranowski slawekjaranowski merged commit 5fa3693 into mojohaus:master Sep 6, 2022
@jarmoniuk jarmoniuk deleted the issue-251-invalid-segment branch September 6, 2022 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid segment error when version contains more than 3 segments
2 participants