Skip to content
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.

Sign up for GitHub

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

fix(change-events): guard against NPE's #7264

Merged
merged 1 commit into from
Feb 7, 2023
Merged

fix(change-events): guard against NPE's #7264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion ...din/metadata/timeline/eventgenerator/DataProcessInstanceRunEventChangeEventGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,10 @@ private Map<String, Object> buildParameters(@Nonnull final DataProcessInstanceRu
parameters.put(RUN_RESULT_KEY, runEvent.getResult().getType().toString());
}

DataProcessInstanceRelationships relationships = getRelationships(entityUrnString);
final DataProcessInstanceRelationships relationships = getRelationships(entityUrnString);
if (relationships == null) {
return parameters;
}

if (relationships.hasParentInstance()) {
parameters.put(PARENT_INSTANCE_URN_KEY, relationships.getParentInstance().toString());
Expand Down
56 changes: 37 additions & 19 deletions ...ava/com/linkedin/metadata/timeline/eventgenerator/SchemaMetadataChangeEventGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,17 @@
import java.util.Set;
import java.util.stream.Collectors;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang.StringUtils;

import static com.linkedin.metadata.timeline.eventgenerator.ChangeEventGeneratorUtils.*;


@Slf4j
public class SchemaMetadataChangeEventGenerator extends EntityChangeEventGenerator<SchemaMetadata> {
private static final String SCHEMA_METADATA_ASPECT_NAME = "schemaMetadata";
private static final String BACKWARDS_INCOMPATIBLE_DESC = "A backwards incompatible change due to";
private static final String FORWARDS_COMPATIBLE_DESC = "A forwards compatible change due to ";
private static final String BACK_AND_FORWARD_COMPATIBLE_DESC = "A forwards & backwards compatible change due to ";
private static final String FIELD_DESCRIPTION_ADDED_FORMAT =
"The description '%s' for the field '%s' has been added.";
Expand All @@ -42,11 +44,11 @@ public class SchemaMetadataChangeEventGenerator extends EntityChangeEventGenerat
private static final String FIELD_DESCRIPTION_MODIFIED_FORMAT =
"The description for the field '%s' has been changed from '%s' to '%s'.";

private static ChangeEvent getDescriptionChange(SchemaField baseField, SchemaField targetField,
private static ChangeEvent getDescriptionChange(@Nullable SchemaField baseField, @Nullable SchemaField targetField,
String datasetFieldUrn, AuditStamp auditStamp) {
String baseDesciption = (baseField != null) ? baseField.getDescription() : null;
String baseDescription = (baseField != null) ? baseField.getDescription() : null;
String targetDescription = (targetField != null) ? targetField.getDescription() : null;
if (baseDesciption == null && targetDescription != null) {
if (baseDescription == null && targetDescription != null) {
// Description got added.
return ChangeEvent.builder()
.operation(ChangeOperation.ADD)
Expand All @@ -57,25 +59,25 @@ private static ChangeEvent getDescriptionChange(SchemaField baseField, SchemaFie
.auditStamp(auditStamp)
.build();
}
if (baseDesciption != null && targetDescription == null) {
if (baseDescription != null && targetDescription == null) {
// Description removed.
return ChangeEvent.builder()
.operation(ChangeOperation.REMOVE)
.semVerChange(SemanticChangeType.MINOR)
.category(ChangeCategory.DOCUMENTATION)
.entityUrn(datasetFieldUrn)
.description(String.format(FIELD_DESCRIPTION_REMOVED_FORMAT, baseDesciption, baseField.getFieldPath()))
.description(String.format(FIELD_DESCRIPTION_REMOVED_FORMAT, baseDescription, baseField.getFieldPath()))
.auditStamp(auditStamp)
.build();
}
if (baseDesciption != null && !baseDesciption.equals(targetDescription)) {
if (baseDescription != null && !baseDescription.equals(targetDescription)) {
// Description Change
return ChangeEvent.builder()
.operation(ChangeOperation.MODIFY)
.semVerChange(SemanticChangeType.PATCH)
.category(ChangeCategory.DOCUMENTATION)
.entityUrn(datasetFieldUrn)
.description(String.format(FIELD_DESCRIPTION_MODIFIED_FORMAT, baseField.getFieldPath(), baseDesciption,
.description(String.format(FIELD_DESCRIPTION_MODIFIED_FORMAT, baseField.getFieldPath(), baseDescription,
targetDescription))
.auditStamp(auditStamp)
.build();
Expand All @@ -84,7 +86,7 @@ private static ChangeEvent getDescriptionChange(SchemaField baseField, SchemaFie
}

private static List<ChangeEvent> getGlobalTagChangeEvents(SchemaField baseField, SchemaField targetField,
String parentUrn,
String parentUrnStr,
String datasetFieldUrn,
AuditStamp auditStamp) {

Expand All @@ -96,17 +98,25 @@ private static List<ChangeEvent> getGlobalTagChangeEvents(SchemaField baseField,
if (baseField != null || targetField != null) {
String fieldPath = targetField != null ? targetField.getFieldPath() : baseField.getFieldPath();
// 2. Convert EntityTagChangeEvent into a SchemaFieldTagChangeEvent.
final Urn parentUrn;
try {
parentUrn = UrnUtils.getUrn(parentUrnStr);
} catch (Exception e) {
log.error("Failed to parse parentUrnStr: {}", parentUrnStr, e);
return Collections.emptyList();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

}

return convertEntityTagChangeEvents(
fieldPath,
UrnUtils.getUrn(parentUrn),
parentUrn,
entityTagChangeEvents);
}

return Collections.emptyList();
}

private static List<ChangeEvent> getGlossaryTermsChangeEvents(SchemaField baseField, SchemaField targetField,
String parentUrn,
String parentUrnStr,
String datasetFieldUrn,
AuditStamp auditStamp) {

Expand All @@ -118,9 +128,17 @@ private static List<ChangeEvent> getGlossaryTermsChangeEvents(SchemaField baseFi
if (targetField != null || baseField != null) {
String fieldPath = targetField != null ? targetField.getFieldPath() : baseField.getFieldPath();
// 2. Convert EntityGlossaryTermChangeEvent into a SchemaFieldGlossaryTermChangeEvent.
final Urn parentUrn;
try {
parentUrn = UrnUtils.getUrn(parentUrnStr);
} catch (Exception e) {
log.error("Failed to parse parentUrnStr: {}", parentUrnStr, e);
return Collections.emptyList();
}

return convertEntityGlossaryTermChangeEvents(
fieldPath,
UrnUtils.getUrn(parentUrn),
parentUrn,
entityGlossaryTermsChangeEvents);
}

Expand Down Expand Up @@ -233,7 +251,7 @@ BACKWARDS_INCOMPATIBLE_DESC, getFieldPathV1(curTargetField), curBaseField.getNat
renamedFields.add(renamedField);
}
} else {
// The targetField got added or a rename occurred. Forward & backwards compatible change + minor version bump.
// The targetField got added or a renaming occurred. Forward & backwards compatible change + minor version bump.
SchemaField renamedField = findRenamedField(curTargetField,
baseFields.subList(baseFieldIdx, baseFields.size()), renamedFields);
if (renamedField == null) {
Expand Down Expand Up @@ -271,7 +289,7 @@ BACKWARDS_INCOMPATIBLE_DESC, getFieldPathV1(curTargetField), curBaseField.getNat
changeEvents.addAll(primaryKeyChangeEvents);

// Handle foreign key constraint change events.
List<ChangeEvent> foreignKeyChangeEvents = getForeignKeyChangeEvents(baseSchema, targetSchema);
List<ChangeEvent> foreignKeyChangeEvents = getForeignKeyChangeEvents();
changeEvents.addAll(foreignKeyChangeEvents);

return changeEvents;
Expand Down Expand Up @@ -371,7 +389,6 @@ private static ChangeEvent generateRenameEvent(Urn datasetUrn, SchemaField curBa
.build();
}

@SuppressWarnings("ConstantConditions")
private static SchemaMetadata getSchemaMetadataFromAspect(EntityAspect entityAspect) {
if (entityAspect != null && entityAspect.getMetadata() != null) {
return RecordUtils.toRecordTemplate(SchemaMetadata.class, entityAspect.getMetadata());
Expand All @@ -380,7 +397,7 @@ private static SchemaMetadata getSchemaMetadataFromAspect(EntityAspect entityAsp
}

@SuppressWarnings("UnnecessaryLocalVariable")
private static List<ChangeEvent> getForeignKeyChangeEvents(SchemaMetadata baseSchema, SchemaMetadata targetSchema) {
private static List<ChangeEvent> getForeignKeyChangeEvents() {
List<ChangeEvent> foreignKeyChangeEvents = new ArrayList<>();
// TODO: Implement the diffing logic.
return foreignKeyChangeEvents;
Expand Down Expand Up @@ -435,10 +452,11 @@ public ChangeTransaction getSemanticDiff(EntityAspect previousValue, EntityAspec
SchemaMetadata baseSchema = getSchemaMetadataFromAspect(previousValue);
SchemaMetadata targetSchema = getSchemaMetadataFromAspect(currentValue);
assert (targetSchema != null);
List<ChangeEvent> changeEvents = new ArrayList<>();
List<ChangeEvent> changeEvents;
try {
changeEvents.addAll(
computeDiffs(baseSchema, targetSchema, DatasetUrn.createFromString(currentValue.getUrn()), changeCategory, null));
changeEvents = new ArrayList<>(
computeDiffs(baseSchema, targetSchema, DatasetUrn.createFromString(currentValue.getUrn()), changeCategory,
null));
} catch (URISyntaxException e) {
throw new IllegalArgumentException("Malformed DatasetUrn " + currentValue.getUrn());
}
Expand Down