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

Support Java 1.6 #3568

Closed
zambien opened this issue Aug 10, 2016 · 23 comments
Closed

Support Java 1.6 #3568

zambien opened this issue Aug 10, 2016 · 23 comments

Comments

@zambien
Copy link

zambien commented Aug 10, 2016

Description

Would it be acceptable to add Java 1.6 support to this project? I have created a fork at zambien/swagger-codegen which has most of the changes needed to do this (replace java.util.Objects and associated code/mustaches with google guava). I work in an environment which runs Java 6 servers and there is not an opportunity to upgrade them unfortunately.

Swagger-codegen version

master

Suggest a Fix

Please take a look at my fork and let me know if a pull request to include Java 1.6 support is acceptable.

@wing328
Copy link
Contributor

wing328 commented Aug 10, 2016

What about running swagger generator via docker instead?

https://hub.docker.com/r/swaggerapi/swagger-generator/

@wing328 wing328 added this to the v2.2.2 milestone Aug 10, 2016
@ePaul
Copy link
Contributor

ePaul commented Aug 10, 2016

Just to make sure: Do you need Java 6 for running Codegen, or for running the generated code?
The latter certainly should be supported.

@fehguy
Copy link
Contributor

fehguy commented Aug 10, 2016

I don't think going to java 6 makes sense. It's been EOL'd for a while now, and I don't want to constrain updates in the core code to not use java 7 features.

As @ePaul said, you can certainly use generated clients with java6 (for now).

@zambien
Copy link
Author

zambien commented Aug 10, 2016

Thanks for the clarifying questions and responses.

I'm trying to get a generated client to work in Java 6. The generated client code includes java.util.Objects, java.util.Objects.equals, and java.util.Objects.hash which are not in Java 6. I have tried playing around with targetCompatibility to no avail. Is there something I'm missing that will create a Java 6 set of compatible client Java classes?

The fork:

https://github.com/zambien/swagger-codegen

Replaces all of the java.util.Objects java code and mustache files with google guava as well as including guava where it is required.

@ePaul
Copy link
Contributor

ePaul commented Aug 10, 2016

I guess for the generated Java files (i.e. the Mustache files) we can/should do that, I wouldn't do it for the generator code (i.e. CodegenModel and friends).

Maybe we can introduce an option for the equals/hashCode implementations, whether to use Java 8 or Guava, like we are doing for JodaTime vs. Java8 time?

@zambien
Copy link
Author

zambien commented Aug 10, 2016

I think that is an excellent idea. I was making changes as I was starting to understand the project which is why I didn't create a PR just yet.

@ashughes
Copy link

I'm running into this issue as well with Objects.equals() and Objects.hash(). In our case, I'm using the generated code on Android, so we're stuck with the Java 6 requirement unless we have minSdk 19.

The reason we use Java instead of Android for the codegen language, is we want to use retrofit2, which is not an option for the android generator (I'm not sure why). However, I just tried generating for Android and it did not use the Java 7 Objects.equals() and Objects.hash(). So, as mentioned above, it would be great to have an alternative option such as Guava, or even the explicit code that the Android codegen uses.

@wing328
Copy link
Contributor

wing328 commented Sep 29, 2016

@ashughes if we provide an option to remove Objects.equals() and Objects.hash() in the Java API client, would that meet your requirement?

@ashughes
Copy link

@wing328 I think so. We just started using Swagger, so we haven't used it extensively yet, but that's the only Android compatibility issue we've run into so far.

@cbornet
Copy link
Contributor

cbornet commented Oct 14, 2016

We should probably have a "javaVersion" option that would provide some defaults:

  • jdk6 : no java.util.Object, threetenbp as default date lib
  • jdk7 : threetenbp as default date lib
  • jdk8 : java.time as default date lib

@ashughes
Copy link

I agree with @cbornet. However, on Android the default date lib should be ThreeTenABP. Maybe there could be an "androidVersion" or "androidMinSdk" option with defaults:

  • 1 : no java.util.Objects, ThreeTenABP as default date lib
  • 19 : ThreeTenABP as default date lib

@cbornet
Copy link
Contributor

cbornet commented Oct 14, 2016

Or "sdk" which can be java6, java7,java8 and android.

@ashughes
Copy link

Right, but it would also be good to have different Android versions, since on API 19+ it's okay to use java.util.Objects.

@wing328
Copy link
Contributor

wing328 commented Oct 15, 2016

I've added an option supportJava6 (default to false) to the latest master via #3919.

Please pull the latest to give it a try.

@k1w1m8
Copy link
Contributor

k1w1m8 commented Oct 18, 2016

We have a similar issue with wishing to run generated client code on Java6 (in our case jersey2 but it doesn't seem to make a difference). So, this would be perfect for us. I tried the latest and found that equals and hashcode methods have been removed from the model classes. Excellent.

However, there are a few remaining issues:

  • java.util.Objects import is still present
  • ApiClient.downloadFileFromResponse references java.nio.file.Files which is similarly not present in Java6.
  • config help for supportJava6 prints (Default: false) twice. Not so important ;-)

@wing328
Copy link
Contributor

wing328 commented Oct 18, 2016

Thanks for doing a test. I'm working on 1 and 3

ApiClient.downloadFileFromResponse references java.nio.file.Files which is similarly not present in Java6.

How should that be changed to conform to Java6?

@k1w1m8
Copy link
Contributor

k1w1m8 commented Oct 18, 2016

It's relying on a copy method. I guess options would be to provide an
inline copy implementation or use an alternative library to do the copy
such as commons.io.

On Tue, 18 Oct 2016 7:02 pm wing328 notifications@github.com wrote:

Thanks for doing a test. I'm working on 1 and 3

ApiClient.downloadFileFromResponse references java.nio.file.Files which is
similarly not present in Java6.

How should that be changed to conform to Java6?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#3568 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKp_EI_XlSUKo534n2kfP40DVVQKTkM1ks5q1H0AgaJpZM4Jgqh7
.

wing328 added a commit to wing328/swagger-codegen that referenced this issue Oct 18, 2016
@wing328
Copy link
Contributor

wing328 commented Oct 18, 2016

I've fixed 1 & 3 via #4021.

Do you have cycle to contribute the suggested fix?

wing328 added a commit that referenced this issue Oct 18, 2016
@wing328
Copy link
Contributor

wing328 commented Oct 18, 2016

#4021 merged into master.

@k1w1m8
Copy link
Contributor

k1w1m8 commented Oct 18, 2016

I think given we already have commons.io as a dependency hopefully we should be able to trivially substitute Files.copy with FileUtils.copyFile for Java6. I'll take a look tomorrow.

@wing328
Copy link
Contributor

wing328 commented Oct 18, 2016

@k1w1m8 thanks for offering help to contribute the enhancement.

@scannerscan
Copy link
Contributor

Just added new PR #4039 with idk6 compatible hashCode and equals implementation.

@wing328
Copy link
Contributor

wing328 commented Oct 20, 2016

PR merged. Thanks @scannerscan for the contribution.

@wing328 wing328 closed this as completed Oct 20, 2016
acramatte added a commit to comerge/swagger-codegen that referenced this issue Oct 25, 2016
* upstream/master: (43 commits)
  [Java] BeanValidation + JAXRS CXF server generator (swagger-api#4068)
  update JS petstore samples
  nancyfx basePath => modulePath toggle (swagger-api#4053)
  Throw an Error object instead of a string
  [Qt5/C++] Arrays of primitive types fix (swagger-api#4046)
  fix NancyFX string parser (swagger-api#4048)
  Fix sanitizeTag to retain numbers and underscore in tag names
  add test for java6 jersey2 client
  hashCode and equals support for jdk6 for jersey2 (swagger-api#4039)
  update gitignore to include PetStore.pro.user
  increase timeout value for qt5 petstore test
  update undertow readme
  Java6 support for jersey2 (swagger-api#4033)
  [java] Allow setting test folder
  fix reserved word handling in model name (sinatra)
  better handling of reserved words for sintatra, dart
  improvements based on swagger-api#3568 (swagger-api#4021)
  [csharp] add missing anchor tags in readme (swagger-api#4019)
  Issue 3651 (swagger-api#4014)
  Added company (swagger-api#4015)
  ...
davidgri pushed a commit to davidgri/swagger-codegen that referenced this issue May 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants