Skip to content

Commit

Permalink
fix(timeline api): adding modification category (#11345)
Browse files Browse the repository at this point in the history
Co-authored-by: Chris Collins <[email protected]>
  • Loading branch information
sakethvarma397 and chriscollins3456 authored Sep 11, 2024
1 parent c3e53a1 commit 728a41e
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,24 @@ public DatasetSchemaFieldChangeEvent(
String description,
String fieldPath,
Urn fieldUrn,
boolean nullable) {
boolean nullable,
SchemaFieldModificationCategory modificationCategory) {
super(
entityUrn,
category,
operation,
modifier,
ImmutableMap.of(
"fieldPath", fieldPath,
"fieldUrn", fieldUrn.toString(),
"nullable", nullable),
"fieldPath",
fieldPath,
"fieldUrn",
fieldUrn.toString(),
"nullable",
nullable,
"modificationCategory",
modificationCategory != null
? modificationCategory.toString()
: SchemaFieldModificationCategory.OTHER.toString()),
auditStamp,
semVerChange,
description);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.linkedin.metadata.timeline.data.dataset;

/*
* Enum to allow us to distinguish between the different schema field modifications when creating entity change events.
*/
public enum SchemaFieldModificationCategory {
// when a schema field is renamed
RENAME,
// when a schema field has a type change
TYPE_CHANGE,
// a default option when no other modification category has been given
OTHER,
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.linkedin.metadata.timeline.data.ChangeTransaction;
import com.linkedin.metadata.timeline.data.SemanticChangeType;
import com.linkedin.metadata.timeline.data.dataset.DatasetSchemaFieldChangeEvent;
import com.linkedin.metadata.timeline.data.dataset.SchemaFieldModificationCategory;
import com.linkedin.schema.SchemaField;
import com.linkedin.schema.SchemaFieldArray;
import com.linkedin.schema.SchemaMetadata;
Expand Down Expand Up @@ -246,6 +247,7 @@ private static void processFieldPathDataTypeChange(
.fieldPath(curBaseField.getFieldPath())
.fieldUrn(getSchemaFieldUrn(datasetUrn, curBaseField))
.nullable(curBaseField.isNullable())
.modificationCategory(SchemaFieldModificationCategory.TYPE_CHANGE)
.auditStamp(auditStamp)
.build());
}
Expand Down Expand Up @@ -483,6 +485,7 @@ private static ChangeEvent generateRenameEvent(
.fieldPath(curBaseField.getFieldPath())
.fieldUrn(getSchemaFieldUrn(datasetUrn, curBaseField))
.nullable(curBaseField.isNullable())
.modificationCategory(SchemaFieldModificationCategory.RENAME)
.auditStamp(auditStamp)
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.linkedin.common.AuditStamp;
import com.linkedin.common.urn.Urn;
import com.linkedin.metadata.timeline.data.ChangeEvent;
import com.linkedin.metadata.timeline.data.dataset.SchemaFieldModificationCategory;
import com.linkedin.mxe.SystemMetadata;
import com.linkedin.restli.internal.server.util.DataMapUtils;
import com.linkedin.schema.SchemaField;
Expand Down Expand Up @@ -42,6 +43,17 @@ private static void compareDescriptions(
assertEquals(expectedDescriptions, actualDescriptions);
}

private static void compareModificationCategories(
Set<String> expectedCategories, List<ChangeEvent> actual) {
Set<String> actualModificationCategories = new HashSet<>();
actual.forEach(
changeEvent -> {
actualModificationCategories.add(
changeEvent.getParameters().get("modificationCategory").toString());
});
assertEquals(expectedCategories, actualModificationCategories);
}

private static Aspect<SchemaMetadata> getSchemaMetadata(List<SchemaField> schemaFieldList) {
return new Aspect<>(
new SchemaMetadata().setFields(new SchemaFieldArray(schemaFieldList)),
Expand Down Expand Up @@ -70,14 +82,17 @@ public void testNativeSchemaBackwardIncompatibleChange() throws Exception {
Set.of(
"A backwards incompatible change due to native datatype of the field 'ID' changed from 'NUMBER(16,1)' to 'NUMBER(10,1)'."),
actual);

compareModificationCategories(
Set.of(SchemaFieldModificationCategory.TYPE_CHANGE.toString()), actual);
List<ChangeEvent> actual2 = test.getChangeEvents(urn, entity, aspect, to, from, auditStamp);
// Test single field going from NUMBER(10,1) -> NUMBER(16,1)
assertEquals(1, actual2.size());
compareDescriptions(
Set.of(
"A backwards incompatible change due to native datatype of the field 'ID' changed from 'NUMBER(10,1)' to 'NUMBER(16,1)'."),
actual2);
compareModificationCategories(
Set.of(SchemaFieldModificationCategory.TYPE_CHANGE.toString()), actual);
}

@Test
Expand All @@ -104,6 +119,11 @@ public void testNativeSchemaFieldAddition() throws Exception {
"A backwards incompatible change due to native datatype of the field 'ID' changed from 'NUMBER(16,1)' to 'NUMBER(10,1)'.",
"A forwards & backwards compatible change due to the newly added field 'aa'."),
actual);
compareModificationCategories(
Set.of(
SchemaFieldModificationCategory.TYPE_CHANGE.toString(),
SchemaFieldModificationCategory.OTHER.toString()),
actual);
}

@Test
Expand All @@ -127,6 +147,8 @@ public void testSchemaFieldRename() throws Exception {
"A forwards & backwards compatible change due to renaming of the field 'ID to ID2'."),
actual);
assertEquals(1, actual.size());
compareModificationCategories(
Set.of(SchemaFieldModificationCategory.RENAME.toString()), actual);
}

@Test
Expand Down
36 changes: 24 additions & 12 deletions smoke-test/tests/openapi/v1/timeline.json
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV1,PROD),service)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to the newly added field 'service'."
Expand All @@ -364,7 +365,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=string].type",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV1,PROD),service.type)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to the newly added field 'service.type'."
Expand All @@ -377,7 +379,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV1,PROD),service.provider)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to the newly added field 'service.provider'."
Expand All @@ -390,7 +393,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider.[type=int].id",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV1,PROD),service.provider.id)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to the newly added field 'service.provider.id'."
Expand All @@ -403,7 +407,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider.[type=string].name",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV1,PROD),service.provider.name)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to the newly added field 'service.provider.name'."
Expand All @@ -416,7 +421,8 @@
"parameters": {
"fieldPath": "property_id",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV1,PROD),property_id)",
"nullable": false
"nullable": false,
"modificationCategory": "OTHER"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to the newly added field 'property_id'."
Expand All @@ -438,7 +444,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider.[type=int].id",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV1,PROD),service.provider.id)",
"nullable": true
"nullable": true,
"modificationCategory": "RENAME"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to renaming of the field 'service.provider.id to service.provider.id2'."
Expand All @@ -451,7 +458,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider.[type=int].id3",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV1,PROD),service.provider.id3)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to the newly added field 'service.provider.id3'."
Expand All @@ -464,7 +472,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider.[type=string].name",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV1,PROD),service.provider.name)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MAJOR",
"description": "A backwards incompatible change due to removal of field: 'service.provider.name'."
Expand All @@ -486,7 +495,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider.[type=int].id2",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV1,PROD),service.provider.id2)",
"nullable": true
"nullable": true,
"modificationCategory": "RENAME"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to renaming of the field 'service.provider.id2 to service.provider.id'."
Expand All @@ -499,7 +509,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider.[type=int].id3",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV1,PROD),service.provider.id3)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MAJOR",
"description": "A backwards incompatible change due to removal of field: 'service.provider.id3'."
Expand All @@ -512,7 +523,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider.[type=string].name",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV1,PROD),service.provider.name)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to the newly added field 'service.provider.name'."
Expand Down
36 changes: 24 additions & 12 deletions smoke-test/tests/openapi/v2/timeline.json
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV2,PROD),service)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to the newly added field 'service'."
Expand All @@ -364,7 +365,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=string].type",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV2,PROD),service.type)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to the newly added field 'service.type'."
Expand All @@ -377,7 +379,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV2,PROD),service.provider)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to the newly added field 'service.provider'."
Expand All @@ -390,7 +393,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider.[type=int].id",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV2,PROD),service.provider.id)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to the newly added field 'service.provider.id'."
Expand All @@ -403,7 +407,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider.[type=string].name",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV2,PROD),service.provider.name)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to the newly added field 'service.provider.name'."
Expand All @@ -416,7 +421,8 @@
"parameters": {
"fieldPath": "property_id",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV2,PROD),property_id)",
"nullable": false
"nullable": false,
"modificationCategory": "OTHER"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to the newly added field 'property_id'."
Expand All @@ -438,7 +444,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider.[type=int].id",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV2,PROD),service.provider.id)",
"nullable": true
"nullable": true,
"modificationCategory": "RENAME"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to renaming of the field 'service.provider.id to service.provider.id2'."
Expand All @@ -451,7 +458,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider.[type=int].id3",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV2,PROD),service.provider.id3)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to the newly added field 'service.provider.id3'."
Expand All @@ -464,7 +472,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider.[type=string].name",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV2,PROD),service.provider.name)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MAJOR",
"description": "A backwards incompatible change due to removal of field: 'service.provider.name'."
Expand All @@ -486,7 +495,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider.[type=int].id2",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV2,PROD),service.provider.id2)",
"nullable": true
"nullable": true,
"modificationCategory": "RENAME"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to renaming of the field 'service.provider.id2 to service.provider.id'."
Expand All @@ -499,7 +509,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider.[type=int].id3",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV2,PROD),service.provider.id3)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MAJOR",
"description": "A backwards incompatible change due to removal of field: 'service.provider.id3'."
Expand All @@ -512,7 +523,8 @@
"parameters": {
"fieldPath": "[version=2.0].[type=struct].[type=struct].service.[type=struct].provider.[type=string].name",
"fieldUrn": "urn:li:schemaField:(urn:li:dataset:(urn:li:dataPlatform:test,datasetTimelineV2,PROD),service.provider.name)",
"nullable": true
"nullable": true,
"modificationCategory": "OTHER"
},
"semVerChange": "MINOR",
"description": "A forwards & backwards compatible change due to the newly added field 'service.provider.name'."
Expand Down

0 comments on commit 728a41e

Please sign in to comment.