-
Notifications
You must be signed in to change notification settings - Fork 39
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
feat: JSON schema evolution support #1760
Changes from 3 commits
8623fc1
5a58508
634a3b3
9a80230
e9b1968
b4e2a5f
2fcd76a
a88d5a1
207c8c0
5529ca7
f3f5d70
df96c1d
9d32311
8845087
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
/* | ||
* Copyright 2021 Lightbend Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package kalix.javasdk; | ||
|
||
import com.fasterxml.jackson.databind.JsonNode; | ||
|
||
import java.util.List; | ||
|
||
/** | ||
* Allows to specify dedicated strategy for JSON schema evolution. | ||
* <p> | ||
* It is used when deserializing data of older version than the | ||
* {@link JacksonMigration#currentVersion}. You implement the transformation of the | ||
* JSON structure in the {@link JacksonMigration#transform} method. If you have changed the | ||
* class name you should add it to {@link JacksonMigration#supportedClassNames}. | ||
*/ | ||
public abstract class JacksonMigration { | ||
aludwiko marked this conversation as resolved.
Show resolved
Hide resolved
johanandren marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/** | ||
* Define current version, that is, the value used when serializing new data. The first version, when no | ||
* migration was used, is always 0. | ||
*/ | ||
public abstract int currentVersion(); | ||
|
||
/** | ||
* Define the supported forward version this migration can read (must be greater or equal than `currentVersion`). | ||
* If this value is different from {@link JacksonMigration#currentVersion} a {@link JacksonMigration#transform} will be used to downcast | ||
* the received payload to the current schema. | ||
*/ | ||
public int supportedForwardVersion() { | ||
return currentVersion(); | ||
} | ||
|
||
/** | ||
* Implement the transformation of the incoming JSON structure to the current | ||
* JSON structure. The `JsonNode` is mutable so you can add and remove fields, | ||
* or change values. Note that you have to cast to specific sub-classes such | ||
* as `ObjectNode` and `ArrayNode` to get access to mutators. | ||
* | ||
* @param fromVersion the version of the old data | ||
* @param jsonNode the incoming JSON data | ||
*/ | ||
public JsonNode transform(int fromVersion, JsonNode jsonNode) { | ||
return jsonNode; | ||
} | ||
|
||
/** | ||
* Override this method if you have changed the class name. Return | ||
* all old class names. | ||
*/ | ||
public List<String> supportedClassNames() { | ||
return List.of(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,25 +17,27 @@ | |
package kalix.javasdk; | ||
|
||
import akka.Done; | ||
import com.fasterxml.jackson.core.JacksonException; | ||
import com.fasterxml.jackson.annotation.JsonAutoDetect; | ||
import com.fasterxml.jackson.annotation.JsonCreator; | ||
import com.fasterxml.jackson.annotation.PropertyAccessor; | ||
import com.fasterxml.jackson.core.JsonGenerator; | ||
import com.fasterxml.jackson.core.JsonParser; | ||
import com.fasterxml.jackson.core.JsonProcessingException; | ||
import com.fasterxml.jackson.core.JsonToken; | ||
import com.fasterxml.jackson.databind.DeserializationContext; | ||
import com.fasterxml.jackson.databind.DeserializationFeature; | ||
import com.fasterxml.jackson.databind.JsonDeserializer; | ||
import com.fasterxml.jackson.databind.JsonMappingException; | ||
import com.fasterxml.jackson.databind.JsonNode; | ||
import com.fasterxml.jackson.databind.JsonSerializer; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.fasterxml.jackson.databind.SerializationFeature; | ||
import com.fasterxml.jackson.databind.SerializerProvider; | ||
import com.fasterxml.jackson.databind.module.SimpleModule; | ||
import com.google.protobuf.Any; | ||
import com.google.protobuf.ByteString; | ||
import com.google.protobuf.UnsafeByteOperations; | ||
import kalix.javasdk.impl.ByteStringEncoding; | ||
import com.fasterxml.jackson.annotation.JsonAutoDetect; | ||
import com.fasterxml.jackson.annotation.JsonCreator; | ||
import com.fasterxml.jackson.annotation.PropertyAccessor; | ||
import com.fasterxml.jackson.core.JsonProcessingException; | ||
import com.fasterxml.jackson.databind.DeserializationFeature; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
import com.fasterxml.jackson.databind.SerializationFeature; | ||
import com.google.protobuf.*; | ||
|
||
import java.io.IOException; | ||
import java.util.Collection; | ||
|
@@ -130,11 +132,27 @@ public static <T> Any encodeJson(T value, String jsonType) { | |
* the JSON string as bytes as value and a type URL starting with "json.kalix.io/". | ||
* | ||
* @param valueClass The type of class to deserialize the object to, the class must have the | ||
* proper Jackson annotations for deserialization. | ||
* proper Jackson annotations for deserialization. | ||
* @param any The protobuf Any object to deserialize. | ||
* @return The decoded object | ||
* @throws IllegalArgumentException if the given value cannot be decoded to a T | ||
*/ | ||
public static <T> T decodeJson(Class<T> valueClass, Any any) { | ||
return decodeJson(valueClass, any, Optional.empty()); | ||
} | ||
|
||
/** | ||
* Decode the given protobuf Any object to an instance of T using Jackson. The object must have | ||
* the JSON string as bytes as value and a type URL starting with "json.kalix.io/". | ||
* | ||
* @param valueClass The type of class to deserialize the object to, the class must have the | ||
* proper Jackson annotations for deserialization. | ||
* @param any The protobuf Any object to deserialize. | ||
* @param jacksonMigration The optional @{@link JacksonMigration} implementation used for deserialization. | ||
* @return The decoded object | ||
* @throws IllegalArgumentException if the given value cannot be decoded to a T | ||
*/ | ||
public static <T> T decodeJson(Class<T> valueClass, Any any, Optional<? extends JacksonMigration> jacksonMigration) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make the signature with Optional param internal and just have one overload with migration and one without, for Java? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought about this, but then all the invocation would look like:
which is also smelly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have some serious doubts about this signature. I want to show serialization test examples for the user. From the test perspective, I need to pass the migration explicitly:
the test can be green, but the in the actual implementation the linking There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The main question is if we want to support migration for JSON in Proto SDK, like here: https://docs.kalix.io/java-protobuf/actions-publishing-subscribing.html#_json If that's the case then I would move not only @ Migration annotation but also @ TypeName, which is even more realistic use case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought user API for proto would skip the reflection-migration-lookup and be that the user supplies migrations manually, for that it just seems messy that they'd need to deal with optional, but for the reflective may-have-migration use I understand why it needs to be optional. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another thought: if we support annotation-detection of migration also for proto, can we just hide it inside the JsonSupport instead and the user doesn't have to pass anything? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "I thought user API for proto would skip the reflection-migration-lookup" - in some sense, it will, because the user will just use the "can we just hide it inside the" - exactly that's my point one method without any extra params. The "only" drawback is that some code-first annotations will be from the |
||
if (!any.getTypeUrl().startsWith(KALIX_JSON)) { | ||
throw new IllegalArgumentException( | ||
"Protobuf bytes with type url [" | ||
|
@@ -145,7 +163,24 @@ public static <T> T decodeJson(Class<T> valueClass, Any any) { | |
} else { | ||
try { | ||
ByteString decodedBytes = ByteStringEncoding.decodePrimitiveBytes(any.getValue()); | ||
return objectMapper.readValue(decodedBytes.toByteArray(), valueClass); | ||
if (jacksonMigration.isPresent()) { | ||
int fromVersion = parseVersion(any.getTypeUrl()); | ||
JacksonMigration migration = jacksonMigration.get(); | ||
int currentVersion = migration.currentVersion(); | ||
int supportedForwardVersion = migration.supportedForwardVersion(); | ||
if (fromVersion < currentVersion) { | ||
return migrate(valueClass, decodedBytes, fromVersion, migration); | ||
} else if (fromVersion == currentVersion) { | ||
return objectMapper.readValue(decodedBytes.toByteArray(), valueClass); | ||
} else if (fromVersion <= supportedForwardVersion) { | ||
return migrate(valueClass, decodedBytes, fromVersion, migration); | ||
} else { | ||
throw new IllegalStateException("Migration version " + supportedForwardVersion + " is " + | ||
"behind version " + fromVersion + " of deserialized type [" + valueClass.getName() + "]"); | ||
} | ||
} else { | ||
return objectMapper.readValue(decodedBytes.toByteArray(), valueClass); | ||
} | ||
} catch (IOException e) { | ||
throw new IllegalArgumentException( | ||
"JSON with type url [" | ||
|
@@ -158,14 +193,29 @@ public static <T> T decodeJson(Class<T> valueClass, Any any) { | |
} | ||
} | ||
|
||
private static <T> T migrate(Class<T> valueClass, ByteString decodedBytes, int fromVersion, JacksonMigration jacksonMigration) throws IOException { | ||
JsonNode jsonNode = objectMapper.readTree(decodedBytes.toByteArray()); | ||
JsonNode newJsonNode = jacksonMigration.transform(fromVersion, jsonNode); | ||
return objectMapper.treeToValue(newJsonNode, valueClass); | ||
} | ||
|
||
private static int parseVersion(String typeUrl) { | ||
if (typeUrl.contains("#")) { //TODO can we assume that there will be ony one "#" ?? | ||
johanandren marked this conversation as resolved.
Show resolved
Hide resolved
|
||
String maybeVersion = typeUrl.split("#")[1]; | ||
return Integer.parseInt(maybeVersion); | ||
} else { | ||
return 0; | ||
} | ||
} | ||
|
||
public static <T, C extends Collection<T>> C decodeJsonCollection(Class<T> valueClass, Class<C> collectionType, Any any) { | ||
if (!any.getTypeUrl().startsWith(KALIX_JSON)) { | ||
throw new IllegalArgumentException( | ||
"Protobuf bytes with type url [" | ||
+ any.getTypeUrl() | ||
+ "] cannot be decoded as JSON, must start with [" | ||
+ KALIX_JSON | ||
+ "]"); | ||
"Protobuf bytes with type url [" | ||
+ any.getTypeUrl() | ||
+ "] cannot be decoded as JSON, must start with [" | ||
+ KALIX_JSON | ||
+ "]"); | ||
} else { | ||
try { | ||
ByteString decodedBytes = ByteStringEncoding.decodePrimitiveBytes(any.getValue()); | ||
|
@@ -212,9 +262,9 @@ public void serialize(Done value, JsonGenerator gen, SerializerProvider serializ | |
class DoneDeserializer extends JsonDeserializer<Done> { | ||
|
||
@Override | ||
public Done deserialize(JsonParser p, DeserializationContext ctxt) throws IOException, JacksonException { | ||
if (p.currentToken() == JsonToken.START_OBJECT && p.nextToken() == JsonToken.END_OBJECT){ | ||
return Done.getInstance(); | ||
public Done deserialize(JsonParser p, DeserializationContext ctxt) throws IOException { | ||
if (p.currentToken() == JsonToken.START_OBJECT && p.nextToken() == JsonToken.END_OBJECT) { | ||
return Done.getInstance(); | ||
} else { | ||
throw JsonMappingException.from(ctxt, "Cannot deserialize Done class, expecting empty object '{}'"); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
/* | ||
* Copyright 2021 Lightbend Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package kalix.javasdk; | ||
|
||
import com.fasterxml.jackson.annotation.JsonCreator; | ||
import com.fasterxml.jackson.annotation.JsonProperty; | ||
|
||
import java.util.Objects; | ||
import java.util.Optional; | ||
|
||
|
||
public class DummyClass { | ||
public String stringValue; | ||
public int intValue; | ||
public Optional<String> optionalStringValue; | ||
|
||
@JsonCreator | ||
public DummyClass(@JsonProperty("stringValue") String stringValue, @JsonProperty("intValue") int intValue, @JsonProperty("optionalStringValue") Optional<String> optionalStringValue) { | ||
this.stringValue = stringValue; | ||
this.intValue = intValue; | ||
this.optionalStringValue = optionalStringValue; | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (this == o) return true; | ||
if (o == null || getClass() != o.getClass()) return false; | ||
DummyClass that = (DummyClass) o; | ||
return intValue == that.intValue && Objects.equals(stringValue, that.stringValue) && Objects.equals(optionalStringValue, that.optionalStringValue); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(stringValue, intValue, optionalStringValue); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
/* | ||
* Copyright 2021 Lightbend Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package kalix.javasdk; | ||
|
||
import com.fasterxml.jackson.annotation.JsonCreator; | ||
import com.fasterxml.jackson.annotation.JsonProperty; | ||
|
||
import java.util.Objects; | ||
|
||
public class DummyClass2 { | ||
public String stringValue; | ||
public int intValue; | ||
public String mandatoryStringValue; | ||
|
||
@JsonCreator | ||
public DummyClass2(@JsonProperty("stringValue") String stringValue, @JsonProperty("intValue") int intValue, @JsonProperty("mandatoryStringValue") String mandatoryStringValue) { | ||
this.stringValue = stringValue; | ||
this.intValue = intValue; | ||
this.mandatoryStringValue = mandatoryStringValue; | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (this == o) return true; | ||
if (o == null || getClass() != o.getClass()) return false; | ||
DummyClass2 that = (DummyClass2) o; | ||
return intValue == that.intValue && Objects.equals(stringValue, that.stringValue) && Objects.equals(mandatoryStringValue, that.mandatoryStringValue); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(stringValue, intValue, mandatoryStringValue); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "DummyClass2{" + | ||
"stringValue='" + stringValue + '\'' + | ||
", intValue=" + intValue + | ||
", mandatoryStringValue='" + mandatoryStringValue + '\'' + | ||
'}'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
/* | ||
* Copyright 2021 Lightbend Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package kalix.javasdk; | ||
|
||
import com.fasterxml.jackson.databind.JsonNode; | ||
import com.fasterxml.jackson.databind.node.ObjectNode; | ||
import com.fasterxml.jackson.databind.node.TextNode; | ||
|
||
public class DummyClass2Migration extends JacksonMigration { | ||
@Override | ||
public int currentVersion() { | ||
return 1; | ||
} | ||
|
||
@Override | ||
public JsonNode transform(int fromVersion, JsonNode jsonNode) { | ||
if (fromVersion < 1) { | ||
return ((ObjectNode) jsonNode).set("mandatoryStringValue", TextNode.valueOf("mandatory-value")); | ||
} else { | ||
return jsonNode; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm reusing some test classes.