the-other-tim-brown commented on code in PR #569:
URL: https://github.com/apache/incubator-xtable/pull/569#discussion_r1957174855
##########
xtable-api/src/test/java/org/apache/xtable/model/metadata/TestTableSyncMetadata.java:
##########
@@ -76,4 +96,26 @@ void failToParseJsonWithMissingLastSyncedInstant() {
TableSyncMetadata.fromJson(
"{\"instantsToConsiderForNextSync\":[\"2020-08-21T11:15:30Z\",\"2024-01-21T12:15:30Z\"],\"version\":0}"));
}
+
+ @Test
+ void parseJsonWithNewFieldsAdded() {
Review Comment:
Is this already covered in the test above?
##########
xtable-api/src/test/java/org/apache/xtable/model/metadata/TestTableSyncMetadata.java:
##########
@@ -56,7 +59,24 @@ private static Stream<Arguments> provideMetadataAndJson() {
"{\"lastInstantSynced\":\"2020-07-04T10:15:30Z\",\"instantsToConsiderForNextSync\":[],\"version\":0}"),
Arguments.of(
TableSyncMetadata.of(Instant.parse("2020-07-04T10:15:30.00Z"),
null),
- "{\"lastInstantSynced\":\"2020-07-04T10:15:30Z\",\"version\":0}"));
+ "{\"lastInstantSynced\":\"2020-07-04T10:15:30Z\",\"version\":0}"),
+ // New version of metadata and JSON where two new fields are added
Review Comment:
"new" will be relative in the future so we should mention the names of the
fields added
##########
xtable-core/src/main/java/org/apache/xtable/delta/DeltaConversionTarget.java:
##########
@@ -258,10 +258,15 @@ public Optional<String> getTargetCommitIdentifier(String
sourceIdentifier) {
Optional<TableSyncMetadata> optionalMetadata =
TableSyncMetadata.fromJson(sourceMetadataJson.get());
if (!optionalMetadata.isPresent()) {
- return Optional.empty();
+ continue;
}
TableSyncMetadata metadata = optionalMetadata.get();
+ // For backward compatability since the older version JSON might not
contain new fields
Review Comment:
I think this will be covered at line 270 so we don't need the extra
condition right?
Is there a test case where we will have a `null` to cover this case?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
