Skip to content

Commit

Permalink
Standardize JSON serialization of V3 API model insant fields
Browse files Browse the repository at this point in the history 
Previously the JSON serialization relied on the default Jackson behavior which was to defer to `toString()` however that followed the ISO standard and would serialize to whatever fractionional
number of digits were available. On Java 8 JDKs this was ok as it would always do milliseconds. On a Java 11 JDK it would expand to 6 digits of precision which still adhers to the spec. The
problem was that within Netflix there is a widely used client with a long upgrade path that hardcoded some 3 digit precision expectation in its parsing rather than using a standard ISO8601 parsing
library for the time strings returned by the API.

This change creates a custom serializer which truncates all available Instants to millisecond precition before preceeding with the standard serialization logic of calling `toStriong()`. This should
ensure backwards compatibility as we run the server on Java 11 JDKs.
  • Loading branch information
@tgianos
tgianos committed Jan 27, 2022
1 parent 3f09f62 commit 12fa839
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 7 deletions.
14 changes: 7 additions & 7 deletions genie-client/src/test/groovy/com/netflix/genie/client/GenieClientUtilsSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@ class GenieClientUtilsSpec extends Specification {
noExceptionThrown()
results == expectedResults
where:
body | expectedResults
"{}" | []
"[]" | []
"{\"_embedded\": []}" | []
"{\"_embedded\": {\"notRight\": []}}" | []
"{\"_embedded\": {\"jobSearchResultList\": {}}}" | []
"{\"_embedded\": {\"jobSearchResultList\": [{\"id\":\"1234\",\"name\":\"testJob\",\"user\":\"tgianos\",\"status\":\"SUCCEEDED\",\"started\":\"1970-01-01T00:00:50Z\",\"finished\":\"1970-01-01T00:00:52Z\",\"clusterName\":null,\"commandName\":null,\"runtime\":\"PT2S\"}]}}" | [new JobSearchResult("1234", "testJob", "tgianos", JobStatus.SUCCEEDED, Instant.ofEpochSecond(50), Instant.ofEpochSecond(52), null, null)]
body | expectedResults
"{}" | []
"[]" | []
"{\"_embedded\": []}" | []
"{\"_embedded\": {\"notRight\": []}}" | []
"{\"_embedded\": {\"jobSearchResultList\": {}}}" | []
"{\"_embedded\": {\"jobSearchResultList\": [{\"id\":\"1234\",\"name\":\"testJob\",\"user\":\"tgianos\",\"status\":\"SUCCEEDED\",\"started\":\"1970-01-01T00:00:50Z\",\"finished\":\"1970-01-01T00:00:52.001Z\",\"clusterName\":null,\"commandName\":null,\"runtime\":\"PT2.001S\"}]}}" | [new JobSearchResult("1234", "testJob", "tgianos", JobStatus.SUCCEEDED, Instant.ofEpochSecond(50), Instant.ofEpochMilli(52001), null, null)]
}

def "Unsuccessful search result request throws client exception"() {
Expand Down
4 changes: 4 additions & 0 deletions genie-common/src/main/java/com/netflix/genie/common/dto/BaseDTO.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
package com.netflix.genie.common.dto;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import com.netflix.genie.common.external.util.GenieObjectMapper;
import com.netflix.genie.common.util.JsonUtils;
import lombok.EqualsAndHashCode;
import lombok.Getter;

Expand All @@ -41,7 +43,9 @@ public abstract class BaseDTO implements Serializable {

@Size(max = 255, message = "Max length for the ID is 255 characters")
private final String id;
@JsonSerialize(using = JsonUtils.OptionalInstantMillisecondSerializer.class)
private final Instant created;
@JsonSerialize(using = JsonUtils.OptionalInstantMillisecondSerializer.class)
private final Instant updated;

/**
Expand Down
3 changes: 3 additions & 0 deletions genie-common/src/main/java/com/netflix/genie/common/dto/Job.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import com.fasterxml.jackson.databind.ser.std.ToStringSerializer;
import com.netflix.genie.common.util.JsonUtils;
import com.netflix.genie.common.util.TimeUtils;
import lombok.Getter;
import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -50,7 +51,9 @@ public class Job extends CommonDTO {
private final JobStatus status;
@Size(max = 255, message = "Max length of the status message is 255 characters")
private final String statusMsg;
@JsonSerialize(using = JsonUtils.OptionalInstantMillisecondSerializer.class)
private final Instant started;
@JsonSerialize(using = JsonUtils.OptionalInstantMillisecondSerializer.class)
private final Instant finished;
@Size(max = 1024, message = "Max character length is 1024 characters for the archive location")
private final String archiveLocation;
Expand Down
3 changes: 3 additions & 0 deletions genie-common/src/main/java/com/netflix/genie/common/dto/JobExecution.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import com.netflix.genie.common.external.dtos.v4.ArchiveStatus;
import com.netflix.genie.common.util.JsonUtils;
import lombok.Getter;

import javax.annotation.Nullable;
Expand Down Expand Up @@ -64,6 +66,7 @@ public class JobExecution extends BaseDTO {
message = "The delay between checks must be at least 1 millisecond. Probably should be much more than that"
)
private final Long checkDelay;
@JsonSerialize(using = JsonUtils.OptionalInstantMillisecondSerializer.class)
private final Instant timeout;
private final Integer exitCode;
@Min(
Expand Down
3 changes: 3 additions & 0 deletions genie-common/src/main/java/com/netflix/genie/common/dto/search/JobSearchResult.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.fasterxml.jackson.databind.ser.std.ToStringSerializer;
import com.fasterxml.jackson.datatype.jsr310.deser.DurationDeserializer;
import com.netflix.genie.common.dto.JobStatus;
import com.netflix.genie.common.util.JsonUtils;
import com.netflix.genie.common.util.TimeUtils;
import lombok.EqualsAndHashCode;
import lombok.Getter;
Expand All @@ -48,7 +49,9 @@ public class JobSearchResult extends BaseSearchResult {
private static final long serialVersionUID = -3886685874572773514L;

private final JobStatus status;
@JsonSerialize(using = JsonUtils.OptionalInstantMillisecondSerializer.class)
private final Instant started;
@JsonSerialize(using = JsonUtils.OptionalInstantMillisecondSerializer.class)
private final Instant finished;
private final String clusterName;
private final String commandName;
Expand Down
52 changes: 52 additions & 0 deletions genie-common/src/main/java/com/netflix/genie/common/util/JsonUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@
*/
package com.netflix.genie.common.util;

import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.JsonSerializer;
import com.fasterxml.jackson.databind.SerializerProvider;
import com.netflix.genie.common.exceptions.GenieException;
import com.netflix.genie.common.exceptions.GenieServerException;
import com.netflix.genie.common.external.util.GenieObjectMapper;
Expand All @@ -29,8 +32,11 @@
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.io.IOException;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.Collection;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

/**
Expand Down Expand Up @@ -125,4 +131,50 @@ public static String joinArguments(final List<String> commandArgs) {
.collect(Collectors.joining(StringUtils.SPACE));
}
}

/**
* Truncate instants to millisecond precision during ISO 8601 serialization to string for backwards compatibility.
*
* @author tgianos
* @since 4.1.2
*/
public static class InstantMillisecondSerializer extends JsonSerializer<Instant> {

/**
* {@inheritDoc}
*/
@Override
public void serialize(
final Instant value,
final JsonGenerator gen,
final SerializerProvider serializers
) throws IOException {
gen.writeString(value.truncatedTo(ChronoUnit.MILLIS).toString());
}
}

/**
* Truncate instants to millisecond precision during ISO 8601 serialization to string for backwards compatibility.
*
* @author tgianos
* @since 4.1.2
*/
public static class OptionalInstantMillisecondSerializer extends JsonSerializer<Optional<Instant>> {

/**
* {@inheritDoc}
*/
@Override
public void serialize(
final Optional<Instant> value,
final JsonGenerator gen,
final SerializerProvider serializers
) throws IOException {
if (value.isPresent()) {
gen.writeString(value.get().truncatedTo(ChronoUnit.MILLIS).toString());
} else {
gen.writeNull();
}
}
}
}
50 changes: 50 additions & 0 deletions genie-common/src/test/groovy/com/netflix/genie/common/util/JsonUtilsSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@
*/
package com.netflix.genie.common.util

import com.fasterxml.jackson.core.JsonGenerator
import com.fasterxml.jackson.databind.SerializerProvider
import spock.lang.Specification
import spock.lang.Unroll

import java.time.Instant

/**
* Specifications for {@link JsonUtils}.
*
Expand Down Expand Up @@ -59,4 +63,50 @@ class JsonUtilsSpec extends Specification {
["arg1", "arg2", "arg3", "arg4", "arg5", "'"] | "'arg1' 'arg2' 'arg3' 'arg4' 'arg5' '''"
["foo/bar", "--hello", "\$world"] | "'foo/bar' '--hello' '\$world'"
}

@Unroll
def "Can serialize instant #instant to millisecond precision string #instantString"() {
def generator = Mock(JsonGenerator)
def serializer = new JsonUtils.InstantMillisecondSerializer()

when:
serializer.serialize(instant, generator, Mock(SerializerProvider))

then:
1 * generator.writeString(instantString)

where:
instant | instantString
Instant.ofEpochMilli(52) | "1970-01-01T00:00:00.052Z"
Instant.ofEpochMilli(52000) | "1970-01-01T00:00:52Z"
Instant.ofEpochMilli(52000).plusNanos(7) | "1970-01-01T00:00:52Z"
Instant.ofEpochMilli(52001) | "1970-01-01T00:00:52.001Z"
Instant.ofEpochMilli(52001).plusNanos(1) | "1970-01-01T00:00:52.001Z"
}

@Unroll
def "Can serialize optional instant #instant to millisecond precision string #instantString"() {
def generator = Mock(JsonGenerator)
def serializer = new JsonUtils.OptionalInstantMillisecondSerializer()

when:
serializer.serialize((Optional<Instant>) instant, generator, Mock(SerializerProvider))

then:
if (instant.isPresent()) {
1 * generator.writeString(instantString)
}
else {
1 * generator.writeNull()
}

where:
instant | instantString
Optional.of(Instant.ofEpochMilli(52)) | "1970-01-01T00:00:00.052Z"
Optional.of(Instant.ofEpochMilli(52000)) | "1970-01-01T00:00:52Z"
Optional.of(Instant.ofEpochMilli(52000).plusNanos(7)) | "1970-01-01T00:00:52Z"
Optional.of(Instant.ofEpochMilli(52001)) | "1970-01-01T00:00:52.001Z"
Optional.of(Instant.ofEpochMilli(52001).plusNanos(1)) | "1970-01-01T00:00:52.001Z"
Optional.empty() | "blah"
}
}

0 comments on commit 12fa839

Please sign in to comment.