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.

Sign up for GitHub

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

[python] Fix date-time parsing #6458

Merged
merged 8 commits into from
Jun 4, 2020
Merged

[python] Fix date-time parsing #6458

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7f71c5f
Date and date-time examples
jirikuncar May 27, 2020
54d14bd
[python-experimental] fix date-time processing
jirikuncar May 27, 2020
b419234
Apply suggestions from code review
jirikuncar May 29, 2020
12ffeb0
Resolve conflicts
jirikuncar Jun 1, 2020
6eae87f
Merge remote-tracking branch 'origin/jirikuncar/python-experimental/d…
jirikuncar Jun 1, 2020
24832a1
Fix date(-time) example parsing
jirikuncar Jun 1, 2020
5da6b76
Merge remote-tracking branch 'upstream/master' into jirikuncar/python…
jirikuncar Jun 1, 2020
a1ba8b3
Add unit test for toDefaultValue
jirikuncar Jun 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 20 additions & 16 deletions ...tor/src/main/java/org/openapitools/codegen/languages/PythonClientExperimentalCodegen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,9 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.text.DateFormat;
import java.text.SimpleDateFormat;
import java.time.OffsetDateTime;
import java.time.ZoneOffset;
import java.time.format.DateTimeFormatter;
import java.io.File;
import java.util.*;
import java.util.regex.Pattern;
Expand Down Expand Up @@ -180,15 +181,15 @@ public String getName() {
return "python-experimental";
}

public String dateToString(Schema p, Date date, DateFormat dateFormatter, DateFormat dateTimeFormatter) {
public String dateToString(Schema p, OffsetDateTime date, DateTimeFormatter dateFormatter, DateTimeFormatter dateTimeFormatter) {
// converts a date into a date or date-time python string
if (!(ModelUtils.isDateSchema(p) || ModelUtils.isDateTimeSchema(p))) {
throw new RuntimeException("passed schema must be of type Date or DateTime");
}
if (ModelUtils.isDateSchema(p)) {
return "dateutil_parser('" + dateFormatter.format(date) + "').date()";
return "dateutil_parser('" + date.format(dateFormatter) + "').date()";
}
return "dateutil_parser('" + dateTimeFormatter.format(date) + "')";
return "dateutil_parser('" + date.format(dateTimeFormatter) + "')";
}

/**
Expand All @@ -212,20 +213,19 @@ public String toDefaultValue(Schema p) {
}

// convert datetime and date enums if they exist
DateFormat iso8601Date = new SimpleDateFormat("yyyy-MM-dd", Locale.ROOT);
DateFormat iso8601DateTime = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSXXX", Locale.ROOT);
TimeZone utc = TimeZone.getTimeZone("UTC");
iso8601Date.setTimeZone(utc);
iso8601DateTime.setTimeZone(utc);
DateTimeFormatter iso8601Date = DateTimeFormatter.ofPattern("yyyy-MM-dd", Locale.ROOT);
jirikuncar marked this conversation as resolved.
Show resolved Hide resolved
DateTimeFormatter iso8601DateTime = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSXXX", Locale.ROOT);
jirikuncar marked this conversation as resolved.
Show resolved Hide resolved
iso8601Date.withZone(ZoneOffset.UTC.normalized());
jirikuncar marked this conversation as resolved.
Show resolved Hide resolved
iso8601DateTime.withZone(ZoneOffset.UTC.normalized());

if (ModelUtils.isDateSchema(p) || ModelUtils.isDateTimeSchema(p)) {
List<Object> currentEnum = p.getEnum();
List<String> fixedEnum = new ArrayList<String>();
String fixedValue = null;
Date date = null;
OffsetDateTime date = null;
if (currentEnum != null && !currentEnum.isEmpty()) {
for (Object enumItem : currentEnum) {
date = (Date) enumItem;
date = (OffsetDateTime) enumItem;
fixedValue = dateToString(p, date, iso8601Date, iso8601DateTime);
fixedEnum.add(fixedValue);
}
Expand All @@ -235,15 +235,19 @@ public String toDefaultValue(Schema p) {
// convert the example if it exists
Object currentExample = p.getExample();
if (currentExample != null) {
date = (Date) currentExample;
try {
date = (OffsetDateTime) currentExample;
} catch (ClassCastException e) {
date = ((Date) currentExample).toInstant().atOffset(ZoneOffset.UTC);
Copy link
Contributor

@spacether spacether May 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Can you explain what this code is doing?
Is it assigning the UTC time zone if the date lacks a time zone?
If so I am concerned that we are adding information which is not present.
Is there a spec which directs us to do this?

What happens when invalid values are set as examples/defaults like "1-2" or "NotADateOrDateTime"?
How about adding a sample spec with invalid dates at:
https://github.com/OpenAPITools/openapi-generator/tree/master/modules/openapi-generator/src/test/resources/2_0/issue-xxx.yaml
And adding a pythonclientexperimentaltest.java test case?

Copy link
Member

Choose a reason for hiding this comment

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

@spacether this is a common pattern. Whatever the date refers to (LocalDate without offset, UTC with offset), it'll represent a single instant since the epoch. Once you have that instant, the only way to accurately represent it is at UTC offset. That call gives you an OffsetDateTime which can accurately hold ISO8601 date-time as defined in the OpenAPI Specification.

jirikuncar marked this conversation as resolved.
Show resolved Hide resolved
}
fixedValue = dateToString(p, date, iso8601Date, iso8601DateTime);
fixedEnum.add(fixedValue);
p.setExample(fixedValue);
}

// fix defaultObject
if (defaultObject != null) {
date = (Date) defaultObject;
date = (OffsetDateTime) defaultObject;
fixedValue = dateToString(p, date, iso8601Date, iso8601DateTime);
p.setDefault(fixedValue);
defaultObject = fixedValue;
Expand Down Expand Up @@ -887,15 +891,15 @@ public String getSimpleTypeDeclaration(Schema schema) {
* Primitive types in the OAS specification are implemented in Python using the corresponding
* Python primitive types.
* Composed types (e.g. allAll, oneOf, anyOf) are represented in Python using list of types.
*
*
* The caller should set the prefix and suffix arguments to empty string, except when
* getTypeString invokes itself recursively. A non-empty prefix/suffix may be specified
* to wrap the return value in a python dict, list or tuple.
*
* Examples:
* - "bool, date, float" The data must be a bool, date or float.
* - "[bool, date]" The data must be an array, and the array items must be a bool or date.
*
*
* @param p The OAS schema.
* @param prefix prepended to the returned value.
* @param suffix appended to the returned value.
Expand Down
6 changes: 5 additions & 1 deletion ...hon-experimental/petstore-with-fake-endpoints-models-for-testing-with-http-signature.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,7 @@ paths:
description: None
type: string
format: date-time
example: '2020-02-02T20:20:20.22222Z'
password:
description: None
type: string
Expand Down Expand Up @@ -1202,6 +1203,7 @@ components:
shipDate:
type: string
format: date-time
example: '2020-02-02T20:20:20.000222Z'
jirikuncar marked this conversation as resolved.
Show resolved Hide resolved
status:
type: string
description: Order Status
Expand Down Expand Up @@ -1439,7 +1441,7 @@ components:
maximum: 543.2
minimum: 32.1
type: number
multipleOf: 32.5
multipleOf: 32.5
float:
type: number
format: float
Expand All @@ -1462,9 +1464,11 @@ components:
date:
type: string
format: date
example: '2020-02-02'
dateTime:
type: string
format: date-time
example: '2020-02-02T20:20:20.000222Z'
jirikuncar marked this conversation as resolved.
Show resolved Hide resolved
jirikuncar marked this conversation as resolved.
Show resolved Hide resolved
uuid:
type: string
format: uuid
Expand Down