From 820981072f282697f4b003b54cae4c4d34e12e3c Mon Sep 17 00:00:00 2001 From: david-leifker <114954101+david-leifker@users.noreply.github.com> Date: Thu, 18 May 2023 16:46:14 -0500 Subject: [PATCH] fix(metadata-io): remove assert in favor of exceptions (#8035) --- .../linkedin/metadata/search/features/Features.java | 5 ++++- .../metadata/timeline/TimelineServiceImpl.java | 5 ++++- .../EditableDatasetPropertiesChangeEventGenerator.java | 7 ++++++- .../EditableSchemaMetadataChangeEventGenerator.java | 6 +++++- .../GlossaryTermsChangeEventGenerator.java | 7 ++++++- .../InstitutionalMemoryChangeEventGenerator.java | 7 ++++++- .../eventgenerator/OwnershipChangeEventGenerator.java | 6 +++++- .../SchemaMetadataChangeEventGenerator.java | 10 ++++++++-- .../timeseries/elastic/query/ESAggregatedStatsDAO.java | 5 ++++- 9 files changed, 48 insertions(+), 10 deletions(-) diff --git a/metadata-io/src/main/java/com/linkedin/metadata/search/features/Features.java b/metadata-io/src/main/java/com/linkedin/metadata/search/features/Features.java index 165d19bed0..f1250ecd61 100644 --- a/metadata-io/src/main/java/com/linkedin/metadata/search/features/Features.java +++ b/metadata-io/src/main/java/com/linkedin/metadata/search/features/Features.java @@ -55,7 +55,10 @@ public class Features { @Nonnull public static List merge(@Nonnull List featureList1, @Nonnull List featureList2) { - assert (featureList1.size() == featureList2.size()); + if (featureList1.size() != featureList2.size()) { + throw new IllegalArgumentException(String.format("Expected both lists to have the same number of elements. %s != %s", + featureList1.size(), featureList2.size())); + } return Streams.zip(featureList1.stream(), featureList2.stream(), Features::merge).collect(Collectors.toList()); } } diff --git a/metadata-io/src/main/java/com/linkedin/metadata/timeline/TimelineServiceImpl.java b/metadata-io/src/main/java/com/linkedin/metadata/timeline/TimelineServiceImpl.java index 7093283b71..abd172170e 100644 --- a/metadata-io/src/main/java/com/linkedin/metadata/timeline/TimelineServiceImpl.java +++ b/metadata-io/src/main/java/com/linkedin/metadata/timeline/TimelineServiceImpl.java @@ -413,7 +413,10 @@ public class TimelineServiceImpl implements TimelineService { SemanticVersion curGroupVersion = null; long transactionId = FIRST_TRANSACTION_ID - 1; for (Map.Entry> entry : changeTransactionsMap.entrySet()) { - assert (transactionId < entry.getKey()); + if (transactionId >= entry.getKey()) { + throw new IllegalArgumentException(String.format("transactionId should be < previous. %s >= %s", + transactionId, entry.getKey())); + } transactionId = entry.getKey(); SemanticChangeType highestChangeInGroup = SemanticChangeType.NONE; ChangeTransaction highestChangeTransaction = entry.getValue().stream() diff --git a/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/EditableDatasetPropertiesChangeEventGenerator.java b/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/EditableDatasetPropertiesChangeEventGenerator.java index 42fbde3a99..a10565a7c9 100644 --- a/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/EditableDatasetPropertiesChangeEventGenerator.java +++ b/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/EditableDatasetPropertiesChangeEventGenerator.java @@ -84,11 +84,16 @@ public class EditableDatasetPropertiesChangeEventGenerator @Override public ChangeTransaction getSemanticDiff(EntityAspect previousValue, EntityAspect currentValue, ChangeCategory element, JsonPatch rawDiff, boolean rawDiffsRequested) { + + if (currentValue == null) { + throw new IllegalArgumentException("EntityAspect currentValue should not be null"); + } + if (!previousValue.getAspect().equals(EDITABLE_DATASET_PROPERTIES_ASPECT_NAME) || !currentValue.getAspect() .equals(EDITABLE_DATASET_PROPERTIES_ASPECT_NAME)) { throw new IllegalArgumentException("Aspect is not " + EDITABLE_DATASET_PROPERTIES_ASPECT_NAME); } - assert (currentValue != null); + List changeEvents = new ArrayList<>(); if (element == ChangeCategory.DOCUMENTATION) { EditableDatasetProperties baseDatasetProperties = getEditableDatasetPropertiesFromAspect(previousValue); diff --git a/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/EditableSchemaMetadataChangeEventGenerator.java b/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/EditableSchemaMetadataChangeEventGenerator.java index 876481b352..4a1de4c342 100644 --- a/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/EditableSchemaMetadataChangeEventGenerator.java +++ b/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/EditableSchemaMetadataChangeEventGenerator.java @@ -221,11 +221,15 @@ public class EditableSchemaMetadataChangeEventGenerator extends EntityChangeEven public ChangeTransaction getSemanticDiff(EntityAspect previousValue, EntityAspect currentValue, ChangeCategory element, JsonPatch rawDiff, boolean rawDiffsRequested) { + if (currentValue == null) { + throw new IllegalArgumentException("EntityAspect currentValue should not be null"); + } + if (!previousValue.getAspect().equals(EDITABLE_SCHEMA_METADATA_ASPECT_NAME) || !currentValue.getAspect() .equals(EDITABLE_SCHEMA_METADATA_ASPECT_NAME)) { throw new IllegalArgumentException("Aspect is not " + EDITABLE_SCHEMA_METADATA_ASPECT_NAME); } - assert (currentValue != null); + EditableSchemaMetadata baseEditableSchemaMetadata = getEditableSchemaMetadataFromAspect(previousValue); EditableSchemaMetadata targetEditableSchemaMetadata = getEditableSchemaMetadataFromAspect(currentValue); List changeEvents = new ArrayList<>(); diff --git a/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/GlossaryTermsChangeEventGenerator.java b/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/GlossaryTermsChangeEventGenerator.java index 1a1ad534eb..22b2033ec5 100644 --- a/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/GlossaryTermsChangeEventGenerator.java +++ b/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/GlossaryTermsChangeEventGenerator.java @@ -133,11 +133,16 @@ public class GlossaryTermsChangeEventGenerator extends EntityChangeEventGenerato @Override public ChangeTransaction getSemanticDiff(EntityAspect previousValue, EntityAspect currentValue, ChangeCategory element, JsonPatch rawDiff, boolean rawDiffsRequested) { + + if (currentValue == null) { + throw new IllegalArgumentException("EntityAspect currentValue should not be null"); + } + if (!previousValue.getAspect().equals(GLOSSARY_TERMS_ASPECT_NAME) || !currentValue.getAspect() .equals(GLOSSARY_TERMS_ASPECT_NAME)) { throw new IllegalArgumentException("Aspect is not " + GLOSSARY_TERMS_ASPECT_NAME); } - assert (currentValue != null); + GlossaryTerms baseGlossaryTerms = getGlossaryTermsFromAspect(previousValue); GlossaryTerms targetGlossaryTerms = getGlossaryTermsFromAspect(currentValue); List changeEvents = new ArrayList<>(); diff --git a/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/InstitutionalMemoryChangeEventGenerator.java b/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/InstitutionalMemoryChangeEventGenerator.java index 74ab4f8746..a23d76e477 100644 --- a/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/InstitutionalMemoryChangeEventGenerator.java +++ b/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/InstitutionalMemoryChangeEventGenerator.java @@ -152,11 +152,16 @@ public class InstitutionalMemoryChangeEventGenerator extends EntityChangeEventGe @Override public ChangeTransaction getSemanticDiff(EntityAspect previousValue, EntityAspect currentValue, ChangeCategory element, JsonPatch rawDiff, boolean rawDiffsRequested) { + + if (currentValue == null) { + throw new IllegalArgumentException("EntityAspect currentValue should not be null"); + } + if (!previousValue.getAspect().equals(INSTITUTIONAL_MEMORY_ASPECT_NAME) || !currentValue.getAspect() .equals(INSTITUTIONAL_MEMORY_ASPECT_NAME)) { throw new IllegalArgumentException("Aspect is not " + INSTITUTIONAL_MEMORY_ASPECT_NAME); } - assert (currentValue != null); + InstitutionalMemory baseInstitutionalMemory = getInstitutionalMemoryFromAspect(previousValue); InstitutionalMemory targetInstitutionalMemory = getInstitutionalMemoryFromAspect(currentValue); List changeEvents = new ArrayList<>(); diff --git a/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/OwnershipChangeEventGenerator.java b/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/OwnershipChangeEventGenerator.java index 217aafc829..f5697aea25 100644 --- a/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/OwnershipChangeEventGenerator.java +++ b/metadata-io/src/main/java/com/linkedin/metadata/timeline/eventgenerator/OwnershipChangeEventGenerator.java @@ -147,11 +147,15 @@ public class OwnershipChangeEventGenerator extends EntityChangeEventGenerator schemaFields = new ArrayList<>(schemaMetadata.getFields()); schemaFields.sort(Comparator.comparing(SchemaField::getFieldPath)); schemaMetadata.setFields(new SchemaFieldArray(schemaFields)); @@ -453,7 +455,11 @@ public class SchemaMetadataChangeEventGenerator extends EntityChangeEventGenerat SchemaMetadata baseSchema = getSchemaMetadataFromAspect(previousValue); SchemaMetadata targetSchema = getSchemaMetadataFromAspect(currentValue); - assert (targetSchema != null); + + if (targetSchema == null) { + throw new IllegalStateException("SchemaMetadata targetSchema should not be null"); + } + List changeEvents; try { changeEvents = new ArrayList<>( diff --git a/metadata-io/src/main/java/com/linkedin/metadata/timeseries/elastic/query/ESAggregatedStatsDAO.java b/metadata-io/src/main/java/com/linkedin/metadata/timeseries/elastic/query/ESAggregatedStatsDAO.java index 85db53c7fe..55f030b018 100644 --- a/metadata-io/src/main/java/com/linkedin/metadata/timeseries/elastic/query/ESAggregatedStatsDAO.java +++ b/metadata-io/src/main/java/com/linkedin/metadata/timeseries/elastic/query/ESAggregatedStatsDAO.java @@ -464,7 +464,10 @@ public class ESAggregatedStatsDAO { // 3.1 Do a DFS of the aggregation tree and generate the rows. rowGenHelper(filterAgg.getAggregations(), 0, groupingBuckets.length, rows, rowAcc, ImmutableList.copyOf(groupingBuckets), ImmutableList.copyOf(aggregationSpecs), aspectSpec); - assert (rowAcc.isEmpty()); + + if (!rowAcc.isEmpty()) { + throw new IllegalStateException("Expected stack to be empty."); + } resultTable.setRows(new StringArrayArray(rows)); return resultTable;