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: commits-unsubscr...@xtable.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org