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

Remove guava and use java 8 constructs (#1082) #2082

Closed
wants to merge 7 commits into from

Conversation

copa2
Copy link

@copa2 copa2 commented Oct 29, 2017

This PR removes the guava library dependency with java 8 constructs.
(Fix issue #1082)

The main reason is that other code also uses guava in a incompatible version and guava is not always binary compatible.

Incompatiblilites:

  • This will update jdk from 1.6 to 1.8 as minimum version
  • Binary incompatiblities
  • Public API changes where guava constructs have been exposed

What is missing:

  • Documentation updates (will follow / references to guava)
  • Cleanups (Predicates, Function can be written more compact with java 8)

Checked alternatives:

  • Using shading plugin to shade guava. This makes each sub-project artifact big due whole guava library has to be shaded into the artifact.

@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@springfox springfox deleted a comment Oct 29, 2017
@dilipkrish
Copy link
Member

Thank you @copa2. Will definitely pull in this change after the 2.8.0 release. Currently the library supports down to java 6 and this is a breaking change => up rev to 3.0.0 after this release.

@dilipkrish dilipkrish added the PR label Oct 29, 2017
@springfox springfox deleted a comment Nov 2, 2017
@springfox springfox deleted a comment Nov 2, 2017
@springfox springfox deleted a comment Nov 2, 2017
@springfox springfox deleted a comment Nov 2, 2017
@springfox springfox deleted a comment Nov 2, 2017
@springfox springfox deleted a comment Nov 2, 2017
@springfox springfox deleted a comment Nov 2, 2017
@copa2
Copy link
Author

copa2 commented Nov 2, 2017

Hi dilipkrish

Just checked the failure I receive from circleci.
Problem is that the CachingOperationReader already today is probably not doing what is expected.

return cache.getUnchecked(new OperationCachingEquivalence().wrap(outerContext));

Each entry receives a new OperationCachingEquivalence which calls the Wrapper.
Within the Wrapper the equals checks if the Equivalence is the same Object
if (this.equivalence.equals(that.equivalence)) { before it calls doEquivalent. So all entries will never be equal an no caching will take place today.

Normally you only create one Equivalence object and then call wrap on the same. If I change the code on master like this i will get the same failure. To fix it properly we should probably reconsider what is equivalent for the Operation. Currently it fails in the PetStoreResource for getPetInTx and getPetInCA. It is looked equivalent, as the key data is the same. Probably we have additionally to check the static params or use the methodName. Any suggestions?

@copa2 copa2 closed this Nov 2, 2017
@copa2 copa2 reopened this Nov 2, 2017
# Conflicts:
#	springfox-spring-web/src/main/java/springfox/documentation/spring/web/readers/parameter/ExpansionContext.java
#	springfox-spring-web/src/test/java/springfox/documentation/spring/web/dummy/controllers/BugsController.java
@copa2
Copy link
Author

copa2 commented Nov 2, 2017

This PR is replaced by PR #2086, due unneeded crlf.

@copa2 copa2 closed this Nov 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants