Skip to content

Exception handling and build improvements#119

Merged
ctubbsii merged 2 commits into
apache:mainfrom
ctubbsii:exception-improvements
Mar 17, 2026
Merged

Exception handling and build improvements#119
ctubbsii merged 2 commits into
apache:mainfrom
ctubbsii:exception-improvements

Conversation

@ctubbsii

Copy link
Copy Markdown
Member

Exception handling improvements:

  • Use InvalidAuthorizationException instead of raw IllegalArgumentException where it makes sense, especially to follow the javadoc that says that it will be thrown when parsing an invalid authorization inside an access expression
  • Inline quote/unquote methods from AccessEvaluatorImpl to th eonly place they are called in AccessImpl, so they can be simplified without redundant checks for empty strings
  • Move (un)escape methods from AccessEvaluatorImpl to CharUtils, since they are not used in AccessEvaluatorImpl after the quote/unquote methods were inlined

Build improvements:

  • Add junit-jupiter-engine explicitly to the classpath for tests (required to run tests in IDE; surefire on the command-line adds it automatically, so it's only needed for running tests in the IDE)
  • Relocate tests and generated files into standard directories
  • Put common test loader code in separate module, removing unneeded spotbugs suppressions and redundant test code

Exception handling improvements:

* Use InvalidAuthorizationException instead of raw
  IllegalArgumentException where it makes sense, especially to follow
  the javadoc that says that it will be thrown when parsing an invalid
  authorization inside an access expression
* Inline quote/unquote methods from AccessEvaluatorImpl to th eonly
  place they are called in AccessImpl, so they can be simplified without
  redundant checks for empty strings
* Move (un)escape methods from AccessEvaluatorImpl to CharUtils, since
  they are not used in AccessEvaluatorImpl after the quote/unquote
  methods were inlined

Build improvements:

* Add junit-jupiter-engine explicitly to the classpath for tests
  (required to run tests in IDE; surefire on the command-line adds it
  automatically, so it's only needed for running tests in the IDE)
* Relocate tests and generated files into standard directories
* Put common test loader code in separate module, removing unneeded
  spotbugs suppressions and redundant test code
@ctubbsii ctubbsii added this to the 1.0.0-beta2 milestone Mar 17, 2026
@ctubbsii ctubbsii merged commit d272aa9 into apache:main Mar 17, 2026
8 checks passed
@ctubbsii ctubbsii deleted the exception-improvements branch March 17, 2026 01:37
Comment on lines -224 to -226
// test with an input that is not a string
CharSequence charSeq = CharBuffer.wrap(illegalInput);
e = assertThrows(IllegalArgumentException.class, () -> AccessExpressionImpl.unquote(charSeq),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a reason this was removed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes. The method was inline'd into AccessImpl.unquote, and that public API method can only take a String type. So, testing this code with non-String types is neither necessary nor possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants