swagger-api / swagger-codegen

swagger-codegen contains a template-driven engine to generate documentation, API clients and server stubs in different languages by parsing your OpenAPI / Swagger definition.
http://swagger.io
Apache License 2.0
16.91k stars 6.03k forks source link

Swagger spec date format not supported by Java client #3087

Closed tadhgpearson closed 8 years ago

tadhgpearson commented 8 years ago
Description

The generated Java client does not correctly parse the Swagger date format from API responses.

According to the swagger spec a response field of type string, format date must return a full-date defined by RFC 3339 which specifies the format as

full-date = date-fullyear "-" date-month "-" date-mday

This is not supported by the default Java client, which generates using a single date format, used for both date and date-time which is

DateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSXXX");

This means simple date fields are not supported and cause the client to throw an InvalidFormatException during the parsing of a response model which contains Date fields..

Swagger-codegen version

2.1.6

Swagger declaration file content or url

Source swagger file is https://github.com/amadeus-travel-innovation-sandbox/sandbox-content/blob/master/swagger.yml

Command line used for generation

java -jar modules\swagger-codegen-cli\target\swagger-codegen-cli.jar generate -i https://raw.githubusercontent.com/amadeus-travel-innovation-sandbox/sandbox-content/master/swagger.yml -l java -o /java-client

Steps to reproduce
  1. Build java client using above command
  2. Register on sandbox.amadeus.com and create a new app to get an API key
  3. Create a new unit test using the java client to invoke our hotel API to demonstrate the bug, like so:

    private static final String APIKEY = "YOUR_API_KEY_HERE";
    private static final DefaultApi API = new DefaultApi();
    private static final DateTimeFormatter DATE = ISODateTimeFormat.date();

    @Test
    public void testHotelAirportSearch() throws ApiException {
        LocalDate checkIn = new LocalDate().plusWeeks(6);
        LocalDate checkOut = checkIn.plusDays(1);
        HotelSearchResponse hotelSearchRS = API.hotelAirportSearch(APIKEY, "BOS", DATE.print(checkIn), DATE.print(checkOut), 25, "EN", "USD", null, null, 3, false, false, null);
        assertEquals("US", hotelSearchRS.getResults().get(0).getAddress().getCountry()); 
        assertEquals(checkIn, hotelSearchRS.getResults().get(0).getRooms().get(0).getRates().get(0).getStartDate());
    }
Related issues

No noted related issues

Suggest a Fix

There are two good approaches to this. Either define a date and a dateTime formatter, and use them appropriately, or define a looser date parser for all API responses that can handle any valid ISO Date/Time

cbornet commented 8 years ago

The best way is to use the joda date lib (or JSR310 on java8). It has been implemented on feign and retrofit clients (generate with -DdateLibrary=joda) When all java clients are done, we will be able to switch the default date lib to joda.

cbornet commented 8 years ago

Actually the default(jersey) and jersey2 libs also have joda support

tadhgpearson commented 8 years ago

Sure. I prefer Joda too, or even Java 8, and I'd be happy if you'd make it the default. But this is the default option right now and it's broken. If you agree it's an issue, and you have a preferred approach to fixing it, I'd be happy to do a fix and provide a pull request.

cbornet commented 8 years ago

In the end it will be the default. For now you can use -DdateLibrary=joda

tadhgpearson commented 8 years ago

Sure... but I also need it to work with the default option. For example, right now on editor.swagger.io, you can't choose the options, so the default code output is broken.

wing328 commented 8 years ago

@tadhgpearson yes, editor.swagger.io only uses the default option. Please pull the latest master to generate the SDK instead: https://github.com/swagger-api/swagger-codegen#getting-started (there are more than 120+ enhancements,bug fixes since v2.1.6 release)

tadhgpearson commented 8 years ago

Thanks for the clarification @wing328
Yes, I am using the latest master, and the issue still exists in it. I didn't realize that this wasn't the same as v2.1.6, sorry about that.

wing328 commented 8 years ago

When generating the Java client, did you use -DdateLibrary=joda as suggested by @cbornet ?

Which Java HTTP library did you use? default, jersey2, okhttp-gson, etc?

tadhgpearson commented 8 years ago

I'm using the default HTTP library. Seeing the resolution, does this mean that support for java.util.Date has been removed?

cbornet commented 8 years ago

You can still use it with -DdateLibrary=legacy but I encourage you to use joda if you can.

tadhgpearson commented 8 years ago

LOL - no encouragement needed!