Upgrade base pom 15.3 -> 15.7 #204
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change was necessary to get the required jersey server version to work with the most recent AWS SDK. However, it required a handful of internal changes to get it to behave properly. Most importantly, these were - Major version bump from jersey 1 to jersey 2, which included migrating between groupId's. It only required one piece of code-rewriting, which was reasonably small. - Migrating groupId's from com.codahale.metrics to io.dropwizard.metrics. There was no real code changes involved, but Maven couldn't detect artifact conflicts between these two because they had different groupId's even though they were the same artifact. This meant that the dependency resolution had to be done by hand, so I just migrated to the newer version.
Fixes 3 versioning issues: 1. There was a version override left for a dependency that was no longer used in the project (sun / old jersey). This was removed. 2. 3.1.4 is the more-recent version of Metrics-Guice, but it is hosted only at JCenter, not at Maven Central. In the interest of not pushing needing to add an additional repository for artifacts, I rolled the version back slightly. 3. Bump the AWS SDK version. This was the ultimate goal of this PR.
This corrects the injection for the auth filter, which was not being constructed correctly before, and fixes how it itself gets injected into Jersey (just using the resource provider, not both the resource provider and an @Producer annotation). It also makes a few neatness changes in the pom files. It still will not work for serving static files.
Move from Dropwizard-Guice & implicit bindings to Dropwizard-Guicier and explicit bindings. Doesn't work, as bindings still aren't quite set up right.
I had written small test for the injector for the auth feature to make sure the injector worked, but the version of AssertJ that the basepom pulls in is 3.x == Java8+. This removes AssertJ as a dependency and just uses standard JUnit assertions, which might let us stay on Java7.
Was injecting the HttpServletContext and relying on that to pull out the query parameters. This was creating a scoping competition between Guice and Jersey to inject certain fields, which was a mess. This no longer injects the request context and relies on a slightly more-raw way of pulling out query parameters. I had originally moved away from this because it was barfing over certain encodings, eg a '%' in the URL that didn't correspond to an escape character. This was compounded by some earlier issues. It shouldn't come up now in a serious way because those issues are resolved, but it will likely still barf in some circumstances.
Merged
Java-8-ify Baragon
|
This one is looking good in staging/qa going to merge this. Thanks @PtrTeixeira |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change was necessary to get the required jersey server version to
work with the most recent AWS SDK. However, it required a handful of
internal changes to get it to behave properly. Most importantly, these were
between groupId's. It only required one piece of code-rewriting, which
was reasonably small.
There was no real code changes involved, but Maven couldn't detect
artifact conflicts between these two because they had different
groupId's even though they were the same artifact. This meant that
the dependency resolution had to be done by hand, so I just
migrated to the newer version.