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

Jump to bottom

Add fall-back support for "timeless" java.util.Date deserialization in DefaultDateTypeAdapter #2669

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add fall-back support for "timeless" java.util.Date deserialization in DefaultDateTypeAdapter #2669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bf02672
Update DefaultDateTypeAdapter.java to include support for "timeless" …
MuffinTheMan Apr 23, 2024
11e9928
Add additional test case to DefaultDateTypeAdapterTest.java for "time…
MuffinTheMan Apr 23, 2024
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
2 changes: 2 additions & 0 deletions gson/src/main/java/com/google/gson/internal/bind/DefaultDateTypeAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ private DefaultDateTypeAdapter(DateType<T> dateType, int dateStyle, int timeStyl
if (JavaVersion.isJava9OrLater()) {
dateFormats.add(PreJava9DateFormatProvider.getUsDateTimeFormat(dateStyle, timeStyle));
}
// Include fall-back for "timeless" Date (could be from a java.sql.Date, for example)
dateFormats.add(DateFormat.getDateInstance(dateStyle, Locale.US));
Comment on lines +137 to +138
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems a bit brittle for the use case you are describing because the java.sql.Date adapter actually uses a hardcoded pattern (with default locale) and not a date style:

gson/gson/src/main/java/com/google/gson/internal/sql/SqlDateTypeAdapter.java

Line 52 in 18b0892

private final DateFormat format = new SimpleDateFormat("MMM d, yyyy");

Copy link
Author

Choose a reason for hiding this comment

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

This is certainly interesting that it's a hardcoded SimpleDateFormat in the SQL adapter.

Regarding your overall point (being uncertain a change like I'm proposing should be made at all), the logic in DefaultDateTypeAdapter::deserializeToDate() is already built to try a couple different methods of parsing, so I don't see much harm in adding one more attempt (though I can see a slippery slope rebuttal that isn't necessarily invalid). If I write a custom TypeAdapter (which is currently my workaround), I have to basically copy the exact DefaultDateTypeAdapter and make this one change, which seems a bit silly (but maybe it's still the right answer 🤷‍♂️) for what seems like a pretty valid use-case (see my example code in the PR description). My other alternative is to update classes where fields are java.util.Date to be explicitly java.sql.Date where applicable, but this isn't realistic (if we could do this easily, we'd be switching from Date to something like LocalDate anyway).

Copy link
Collaborator

@Marcono1234 Marcono1234 Apr 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I have to basically copy the exact DefaultDateTypeAdapter and make this one change

You can avoid this by writing a TypeAdapterFactory which first tries to deserialize as java.util.Date and if that fails tries java.sql.Date instead:

DateSqlFallbackFactory (click to expand)

Note that I haven't tested this extensively, and this implementation is also quite verbose. And if parsing fails, the JSON path in the exception message will not be that helpful, saying $ (root element) instead of the actual path.

class DateSqlFallbackFactory implements TypeAdapterFactory {
  @Override
  public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> type) {
    if (type.getRawType() != java.util.Date.class) {
      return null;
    }

    TypeAdapter<JsonElement> jsonElementAdapter = gson.getAdapter(JsonElement.class);
    TypeAdapter<Date> utilDateAdapter = gson.getDelegateAdapter(this, TypeToken.get(Date.class));
    TypeAdapter<java.sql.Date> sqlDateAdapter = gson.getAdapter(java.sql.Date.class);

    @SuppressWarnings("unchecked")
    TypeAdapter<T> adapter = (TypeAdapter<T>) new TypeAdapter<Date>() {
      @Override
      public Date read(JsonReader in) throws IOException {
        // First read as JsonElement tree to be able to parse it twice (once as java.util.Date,
        // once as java.sql.Date) if necessary
        JsonElement json = jsonElementAdapter.read(in);
        try {
          return utilDateAdapter.fromJsonTree(json);
        }
        // Note: This makes assumptions about the exception type thrown by Gson's built-in
        // adapter for java.util.Date
        catch (JsonParseException e) {
          try {
            return sqlDateAdapter.fromJsonTree(json);
          } catch (Exception suppressed) {
            e.addSuppressed(suppressed);
            throw e;
          }
        }
      }

      @Override
      public void write(JsonWriter out, Date value) throws IOException {
        utilDateAdapter.write(out, value);
      }
    };
    return adapter;
  }
}

If you haven't stored the JSON data for any java.util.Date and SQL subclasses persistently yet, then maybe a reliable alternative would be to use GsonBuilder.setDateFormat(String) to specify a locale insensitive pattern (if possible).

}

@Override
Expand Down
1 change: 1 addition & 0 deletions gson/src/test/java/com/google/gson/internal/bind/DefaultDateTypeAdapterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ public void testParsingDatesFormattedWithUsLocale() throws Exception {
Locale.setDefault(Locale.US);
try {
assertParsed("Jan 1, 1970 0:00:00 AM", DefaultDateTypeAdapter.DEFAULT_STYLE_FACTORY);
assertParsed("Jan 1, 1970", DefaultDateTypeAdapter.DEFAULT_STYLE_FACTORY);
assertParsed(
"1/1/70 0:00 AM", DateType.DATE.createAdapterFactory(DateFormat.SHORT, DateFormat.SHORT));
assertParsed(
Expand Down