diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f2da9bea8..116292caf5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ and what APIs have changed, if applicable. ## [Unreleased] +## [29.49.8] - 2024-01-19 +- add WIRE_COMPATIBLE compatility checker mode. + ## [29.49.7] - 2024-01-18 adjust dual read monitoring data match logic and log rate limiter @@ -5615,7 +5618,8 @@ patch operations can re-use these classes for generating patch messages. ## [0.14.1] -[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.49.7...master +[Unreleased]: https://github.com/linkedin/rest.li/compare/v29.49.8...master +[29.49.8]: https://github.com/linkedin/rest.li/compare/v29.49.7...v29.49.8 [29.49.7]: https://github.com/linkedin/rest.li/compare/v29.49.6...v29.49.7 [29.49.6]: https://github.com/linkedin/rest.li/compare/v29.49.5...v29.49.6 [29.49.5]: https://github.com/linkedin/rest.li/compare/v29.49.4...v29.49.5 diff --git a/data/src/main/java/com/linkedin/data/schema/compatibility/CompatibilityChecker.java b/data/src/main/java/com/linkedin/data/schema/compatibility/CompatibilityChecker.java index 4a5ec2cd4a..6e5dab8353 100644 --- a/data/src/main/java/com/linkedin/data/schema/compatibility/CompatibilityChecker.java +++ b/data/src/main/java/com/linkedin/data/schema/compatibility/CompatibilityChecker.java @@ -553,7 +553,7 @@ private void checkEnum(EnumDataSchema older, EnumDataSchema newer) if (!newerOnlySymbols.isEmpty()) { - appendMessage(CompatibilityMessage.Impact.BREAKS_OLD_READER, + appendMessage(CompatibilityMessage.Impact.ENUM_VALUE_ADDED, "new enum added symbols %s", newerOnlySymbols); } diff --git a/data/src/main/java/com/linkedin/data/schema/compatibility/CompatibilityMessage.java b/data/src/main/java/com/linkedin/data/schema/compatibility/CompatibilityMessage.java index adb4a33dc2..ef6780454e 100644 --- a/data/src/main/java/com/linkedin/data/schema/compatibility/CompatibilityMessage.java +++ b/data/src/main/java/com/linkedin/data/schema/compatibility/CompatibilityMessage.java @@ -73,7 +73,11 @@ public enum Impact /** * Enum symbol order changed, which is a compatible change. */ - ENUM_SYMBOLS_ORDER_CHANGE(false); + ENUM_SYMBOLS_ORDER_CHANGE(false), + /** + * New enum value added, which is wire compatible change. However, old readers may not be able to handle it. + */ + ENUM_VALUE_ADDED(false); private final boolean _error; diff --git a/data/src/test/java/com/linkedin/data/schema/compatibility/TestCompatibilityChecker.java b/data/src/test/java/com/linkedin/data/schema/compatibility/TestCompatibilityChecker.java index f6a149bfbf..39bd29a2e6 100644 --- a/data/src/test/java/com/linkedin/data/schema/compatibility/TestCompatibilityChecker.java +++ b/data/src/test/java/com/linkedin/data/schema/compatibility/TestCompatibilityChecker.java @@ -491,7 +491,6 @@ public void testUnion() throws IOException "[ { \"type\" : \"enum\", \"name\" : \"a.b.Enum\", \"symbols\" : [ \"B\", \"D\", \"E\" ] } ]", _dataAndSchema, true, - "ERROR :: BREAKS_OLD_READER :: /union/a.b.Enum/symbols :: new enum added symbols E", "ERROR :: BREAKS_NEW_READER :: /union/a.b.Enum/symbols :: new enum removed symbols A, C" }, { @@ -499,7 +498,6 @@ public void testUnion() throws IOException "[ \"string\", { \"type\" : \"enum\", \"name\" : \"a.b.Enum\", \"symbols\" : [ \"B\", \"D\", \"E\" ] }, \"int\" ]", _dataAndSchema, true, - "ERROR :: BREAKS_OLD_READER :: /union/a.b.Enum/symbols :: new enum added symbols E", "ERROR :: BREAKS_NEW_READER :: /union/a.b.Enum/symbols :: new enum removed symbols A, C" }, { @@ -787,7 +785,6 @@ public void testEnum() throws IOException "{ \"type\" : \"enum\", \"name\" : \"a.b.Enum\", \"symbols\" : [ \"B\", \"D\", \"E\" ] }", _dataAndSchema, true, - "ERROR :: BREAKS_OLD_READER :: /a.b.Enum/symbols :: new enum added symbols E", "ERROR :: BREAKS_NEW_READER :: /a.b.Enum/symbols :: new enum removed symbols A, C" }, { diff --git a/gradle.properties b/gradle.properties index 84c571bc27..a51473b977 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,4 +1,4 @@ -version=29.49.7 +version=29.49.8 group=com.linkedin.pegasus org.gradle.configureondemand=true org.gradle.parallel=true diff --git a/restli-tools/src/main/java/com/linkedin/restli/tools/compatibility/CompatibilityInfoMap.java b/restli-tools/src/main/java/com/linkedin/restli/tools/compatibility/CompatibilityInfoMap.java index 7b5654cd5b..80a77d5bb4 100644 --- a/restli-tools/src/main/java/com/linkedin/restli/tools/compatibility/CompatibilityInfoMap.java +++ b/restli-tools/src/main/java/com/linkedin/restli/tools/compatibility/CompatibilityInfoMap.java @@ -42,9 +42,11 @@ public class CompatibilityInfoMap public CompatibilityInfoMap() { _restSpecMap.put(CompatibilityInfo.Level.INCOMPATIBLE, new ArrayList<>()); + _restSpecMap.put(CompatibilityInfo.Level.WIRE_COMPATIBLE, new ArrayList<>()); _restSpecMap.put(CompatibilityInfo.Level.COMPATIBLE, new ArrayList<>()); _modelMap.put(CompatibilityInfo.Level.INCOMPATIBLE, new ArrayList<>()); + _modelMap.put(CompatibilityInfo.Level.WIRE_COMPATIBLE, new ArrayList<>()); _modelMap.put(CompatibilityInfo.Level.COMPATIBLE, new ArrayList<>()); _annotationMap.put(CompatibilityInfo.Level.INCOMPATIBLE, new ArrayList<>()); @@ -128,7 +130,11 @@ public void addModelInfo(CompatibilityMessage message) } else { - infoType = CompatibilityInfo.Type.TYPE_INFO; + if (message.getImpact() == CompatibilityMessage.Impact.ENUM_VALUE_ADDED) { + infoType = CompatibilityInfo.Type.ENUM_VALUE_ADDED; + } else { + infoType = CompatibilityInfo.Type.TYPE_INFO; + } } info = new CompatibilityInfo(Arrays.asList(message.getPath()), infoType, infoMessage); _modelMap.get(infoType.getLevel()).add(info); @@ -194,9 +200,10 @@ private static void createSummaryForInfo(Collection info, public boolean isCompatible(CompatibilityLevel level) { final Collection incompatibles = getIncompatibles(); + final Collection wireCompatibles = getWireCompatibles(); final Collection compatibles = getCompatibles(); - return isCompatible(incompatibles, compatibles, level); + return isCompatible(incompatibles, wireCompatibles, compatibles, level); } public boolean isRestSpecCompatible(CompatibilityLevel level) @@ -204,20 +211,22 @@ public boolean isRestSpecCompatible(CompatibilityLevel level) final Collection incompatibles = getRestSpecIncompatibles(); final Collection compatibles = getRestSpecCompatibles(); - return isCompatible(incompatibles, compatibles, level); + return isCompatible(incompatibles, new ArrayList<>(), compatibles, level); } public boolean isModelCompatible(CompatibilityLevel level) { final Collection incompatibles = getModelIncompatibles(); + final Collection wireCompatibles = getModelWireCompatibles(); final Collection compatibles = getModelCompatibles(); - return isCompatible(incompatibles, compatibles, level); + return isCompatible(incompatibles, wireCompatibles, compatibles, level); } - private boolean isCompatible(Collection incompatibles, Collection compatibles, CompatibilityLevel level) + private boolean isCompatible(Collection incompatibles, Collection wireCompatibles, Collection compatibles, CompatibilityLevel level) { - return ((incompatibles.isEmpty() || level.ordinal() < CompatibilityLevel.BACKWARDS.ordinal()) && + return ((incompatibles.isEmpty() || level.ordinal() < CompatibilityLevel.WIRE_COMPATIBLE.ordinal()) && + (wireCompatibles.isEmpty() || level.ordinal() < CompatibilityLevel.BACKWARDS.ordinal()) && (compatibles.isEmpty() || level.ordinal() < CompatibilityLevel.EQUIVALENT.ordinal())); } @@ -254,6 +263,15 @@ public Collection getCompatibles() return get(CompatibilityInfo.Level.COMPATIBLE); } + /** + * @return check results in the backwards wire compatibility category. + * empty collection if called before checking any files + */ + public Collection getWireCompatibles() + { + return get(CompatibilityInfo.Level.WIRE_COMPATIBLE); + } + public Collection getRestSpecIncompatibles() { return getRestSpecInfo(CompatibilityInfo.Level.INCOMPATIBLE); @@ -274,6 +292,11 @@ public Collection getModelCompatibles() return getModelInfo(CompatibilityInfo.Level.COMPATIBLE); } + public Collection getModelWireCompatibles() + { + return getModelInfo(CompatibilityInfo.Level.WIRE_COMPATIBLE); + } + public Collection get(CompatibilityInfo.Level level) { Collection infos = new ArrayList<>(getRestSpecInfo(level)); @@ -347,7 +370,7 @@ public boolean isAnnotationCompatible() */ public boolean isAnnotationCompatible(CompatibilityLevel level) { - return isCompatible(_annotationMap.get(CompatibilityInfo.Level.INCOMPATIBLE), + return isCompatible(_annotationMap.get(CompatibilityInfo.Level.INCOMPATIBLE), new ArrayList<>(), _annotationMap.get(CompatibilityInfo.Level.COMPATIBLE), level); } diff --git a/restli-tools/src/main/java/com/linkedin/restli/tools/compatibility/CompatibilityReport.java b/restli-tools/src/main/java/com/linkedin/restli/tools/compatibility/CompatibilityReport.java index 69c56971c9..bb0fcce7fd 100644 --- a/restli-tools/src/main/java/com/linkedin/restli/tools/compatibility/CompatibilityReport.java +++ b/restli-tools/src/main/java/com/linkedin/restli/tools/compatibility/CompatibilityReport.java @@ -26,7 +26,7 @@ public CompatibilityReport(CompatibilityInfoMap infoMap, CompatibilityLevel comp *
  • [RS-C] - String describing a restspec change that is backward compatible.
  • *
  • [RS-I] - String describing a restspec change that is backward incompatible.
  • *
  • [MD-C] - String describing a model(PDSC) change that is backward compatible.
  • - *
  • [MD-C] - String describing a model(PDSC) change that is backward incompatible.
  • + *
  • [MD-I] - String describing a model(PDSC) change that is backward incompatible.
  • *
  • [RS-COMPAT] - Boolean indicating if the full compatibility check was restspec backward compatible for the provided compatibilityLevel
  • *
  • [MD-COMPAT] - Boolean indicating if the full compatibility check was model backward compatible for the provided compatibilityLevel
  • * diff --git a/restli-tools/src/main/java/com/linkedin/restli/tools/idlcheck/CompatibilityInfo.java b/restli-tools/src/main/java/com/linkedin/restli/tools/idlcheck/CompatibilityInfo.java index ac73b4d6f6..f1315996e6 100644 --- a/restli-tools/src/main/java/com/linkedin/restli/tools/idlcheck/CompatibilityInfo.java +++ b/restli-tools/src/main/java/com/linkedin/restli/tools/idlcheck/CompatibilityInfo.java @@ -28,7 +28,13 @@ public class CompatibilityInfo public enum Level { INCOMPATIBLE, - COMPATIBLE + COMPATIBLE, + + /** + * Old readers can deserialize changes serialized by new writers, but may not be able to handle them correctly. + * Currently only used for adding new enum values. + **/ + WIRE_COMPATIBLE } public enum Type @@ -48,6 +54,7 @@ public enum Type TYPE_UNKNOWN(Level.INCOMPATIBLE, "Type cannot be resolved: %s"), VALUE_NOT_EQUAL(Level.INCOMPATIBLE, "Current value \"%2$s\" does not match the previous value \"%1$s\""), VALUE_WRONG_OPTIONALITY(Level.INCOMPATIBLE, "\"%s\" may not be removed because it exists in the previous version"), + ENUM_VALUE_ADDED(Level.WIRE_COMPATIBLE, "%s, new enum value may break old readers"), TYPE_BREAKS_OLD_READER(Level.INCOMPATIBLE, "%s, breaks old readers"), TYPE_BREAKS_NEW_READER(Level.INCOMPATIBLE, "%s, breaks new readers"), TYPE_BREAKS_NEW_AND_OLD_READERS(Level.INCOMPATIBLE, "%s, breaks new and old readers"), diff --git a/restli-tools/src/main/java/com/linkedin/restli/tools/idlcheck/CompatibilityLevel.java b/restli-tools/src/main/java/com/linkedin/restli/tools/idlcheck/CompatibilityLevel.java index bbd8a17778..68d079960c 100644 --- a/restli-tools/src/main/java/com/linkedin/restli/tools/idlcheck/CompatibilityLevel.java +++ b/restli-tools/src/main/java/com/linkedin/restli/tools/idlcheck/CompatibilityLevel.java @@ -28,6 +28,7 @@ public enum CompatibilityLevel { OFF, IGNORE, + WIRE_COMPATIBLE, BACKWARDS, EQUIVALENT; diff --git a/restli-tools/src/test/java/com/linkedin/restli/tools/snapshot/check/TestPegasusSchemaSnapshotCompatibilityChecker.java b/restli-tools/src/test/java/com/linkedin/restli/tools/snapshot/check/TestPegasusSchemaSnapshotCompatibilityChecker.java index dae8cf2a75..ffeeb07d30 100644 --- a/restli-tools/src/test/java/com/linkedin/restli/tools/snapshot/check/TestPegasusSchemaSnapshotCompatibilityChecker.java +++ b/restli-tools/src/test/java/com/linkedin/restli/tools/snapshot/check/TestPegasusSchemaSnapshotCompatibilityChecker.java @@ -48,7 +48,8 @@ public void testCompatiblePegasusSchemaSnapshot(String prevSchema, String currSc @Test(dataProvider = "incompatibleInputFiles") public void testIncompatiblePegasusSchemaSnapshot(String prevSchema, String currSchema, - Collection expectedIncompatibilityErrors, Collection expectedCompatibilityDiffs ) + Collection expectedIncompatibilityErrors, Collection expectedWireCompatibilityDiffs, + Collection expectedCompatibilityDiffs ) { PegasusSchemaSnapshotCompatibilityChecker checker = new PegasusSchemaSnapshotCompatibilityChecker(); CompatibilityInfoMap infoMap = checker.checkPegasusSchemaCompatibility(snapshotDir + FS + prevSchema, snapshotDir + FS + currSchema, @@ -58,6 +59,7 @@ public void testIncompatiblePegasusSchemaSnapshot(String prevSchema, String curr Assert.assertTrue(infoMap.isModelCompatible(CompatibilityLevel.IGNORE)); final Collection modelIncompatibles = infoMap.getModelIncompatibles(); + final Collection modelWireCompatibles = infoMap.getModelWireCompatibles(); final Collection modelCompatibles = infoMap.getModelCompatibles(); for (CompatibilityInfo error : expectedIncompatibilityErrors) @@ -65,6 +67,11 @@ public void testIncompatiblePegasusSchemaSnapshot(String prevSchema, String curr Assert.assertTrue(modelIncompatibles.contains(error), "Reported model incompatibles should contain: " + error.toString()); modelIncompatibles.remove(error); } + for (CompatibilityInfo diff : expectedWireCompatibilityDiffs) + { + Assert.assertTrue(modelWireCompatibles.contains(diff), "Reported model wireCompatibles should contain: " + diff.toString()); + modelWireCompatibles.remove(diff); + } for (CompatibilityInfo diff : expectedCompatibilityDiffs) { Assert.assertTrue(modelCompatibles.contains(diff), "Reported model compatibles should contain: " + diff.toString()); @@ -72,6 +79,7 @@ public void testIncompatiblePegasusSchemaSnapshot(String prevSchema, String curr } Assert.assertTrue(modelIncompatibles.isEmpty()); + Assert.assertTrue(modelWireCompatibles.isEmpty()); Assert.assertTrue(modelCompatibles.isEmpty()); } @@ -145,8 +153,9 @@ private Object[][] compatibleInputFiles() { return new Object[][] { - { "BirthInfo.pdl", "compatibleSchemaSnapshot/BirthInfo.pdl", CompatibilityLevel.EQUIVALENT, CompatibilityOptions.Mode.DATA }, + { "Date.pdl", "compatibleSchemaSnapshot/Date.pdl", CompatibilityLevel.EQUIVALENT, CompatibilityOptions.Mode.DATA }, { "Foo.pdl", "compatibleSchemaSnapshot/Foo.pdl", CompatibilityLevel.BACKWARDS, CompatibilityOptions.Mode.EXTENSION }, + { "BirthInfo.pdl", "compatibleSchemaSnapshot/BirthInfo.pdl", CompatibilityLevel.WIRE_COMPATIBLE, CompatibilityOptions.Mode.DATA }, }; } @@ -169,8 +178,16 @@ private Object[][] incompatibleInputFiles() { "BirthInfo.pdl", "incompatibleSchemaSnapshot/BirthInfo.pdl", incompatibilityErrors, + new HashSet<>(), compatibilityDiffs - } + }, + { "BirthInfo.pdl", + "compatibleSchemaSnapshot/BirthInfo.pdl", // This is combination is compatible with WIRE_COMPATIBLE, but not with BACKWARDS + new HashSet<>(), + new HashSet<>(Arrays.asList(new CompatibilityInfo(Arrays.asList("BirthInfo", "eyeColor", "Color", "symbols"), + CompatibilityInfo.Type.ENUM_VALUE_ADDED, "new enum added symbols GREEN"))), + new HashSet<>() + }, }; } } diff --git a/restli-tools/src/test/pegasus/com/linkedin/restli/tools/pegasusSchemaSnapshotTest/BirthInfo.pdl b/restli-tools/src/test/pegasus/com/linkedin/restli/tools/pegasusSchemaSnapshotTest/BirthInfo.pdl index 4c5baecaf9..12c2af1d41 100644 --- a/restli-tools/src/test/pegasus/com/linkedin/restli/tools/pegasusSchemaSnapshotTest/BirthInfo.pdl +++ b/restli-tools/src/test/pegasus/com/linkedin/restli/tools/pegasusSchemaSnapshotTest/BirthInfo.pdl @@ -6,4 +6,10 @@ record BirthInfo { year: int location: optional Location + + eyeColor: enum Color { + BLUE + BROWN + OTHER + } } diff --git a/restli-tools/src/test/pegasusSchemaSnapshot/BirthInfo.pdl b/restli-tools/src/test/pegasusSchemaSnapshot/BirthInfo.pdl index da200c4c59..e6faff103f 100644 --- a/restli-tools/src/test/pegasusSchemaSnapshot/BirthInfo.pdl +++ b/restli-tools/src/test/pegasusSchemaSnapshot/BirthInfo.pdl @@ -7,4 +7,9 @@ record BirthInfo { longitude: float name: optional string } + eyeColor: enum Color { + BLUE + BROWN + OTHER + } } diff --git a/restli-tools/src/test/pegasusSchemaSnapshot/compatibleSchemaSnapshot/BirthInfo.pdl b/restli-tools/src/test/pegasusSchemaSnapshot/compatibleSchemaSnapshot/BirthInfo.pdl index b7dae3df68..d9b9453bd6 100644 --- a/restli-tools/src/test/pegasusSchemaSnapshot/compatibleSchemaSnapshot/BirthInfo.pdl +++ b/restli-tools/src/test/pegasusSchemaSnapshot/compatibleSchemaSnapshot/BirthInfo.pdl @@ -11,4 +11,10 @@ record BirthInfo { longitude: float name: optional string } + eyeColor: enum Color { + BLUE + BROWN + GREEN + OTHER + } } \ No newline at end of file diff --git a/restli-tools/src/test/pegasusSchemaSnapshot/compatibleSchemaSnapshot/Date.pdl b/restli-tools/src/test/pegasusSchemaSnapshot/compatibleSchemaSnapshot/Date.pdl new file mode 100644 index 0000000000..1d8ae900ad --- /dev/null +++ b/restli-tools/src/test/pegasusSchemaSnapshot/compatibleSchemaSnapshot/Date.pdl @@ -0,0 +1,5 @@ +record Date { + day: int + month: int + year: int +} \ No newline at end of file diff --git a/restli-tools/src/test/pegasusSchemaSnapshot/incompatibleSchemaSnapshot/BirthInfo.pdl b/restli-tools/src/test/pegasusSchemaSnapshot/incompatibleSchemaSnapshot/BirthInfo.pdl index 2c23162431..00ff51d5f4 100644 --- a/restli-tools/src/test/pegasusSchemaSnapshot/incompatibleSchemaSnapshot/BirthInfo.pdl +++ b/restli-tools/src/test/pegasusSchemaSnapshot/incompatibleSchemaSnapshot/BirthInfo.pdl @@ -12,4 +12,9 @@ record BirthInfo { name: optional string } name: string + eyeColor: enum Color { + BLUE + BROWN + OTHER + } } \ No newline at end of file