From 827202de1887b86df97b73a0ed773e790ff6f562 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Mon, 12 May 2025 21:36:13 -0400 Subject: [PATCH 01/12] Add write support for Soft Delete Aspect --- .../metadata/dao/EbeanLocalAccess.java | 30 ++++++++-- .../linkedin/metadata/dao/EbeanLocalDAO.java | 41 ++++++++++--- .../metadata/dao/IEbeanLocalAccess.java | 26 +++++++- .../metadata/dao/utils/EBeanDAOUtils.java | 59 +++++++++++++++---- .../metadata/dao/EbeanLocalDAOTest.java | 4 +- .../metadata/dao/utils/EBeanDAOUtilsTest.java | 56 +++++++++++++++++- .../testing/SoftDeletedAspectCopy.pdl | 7 +++ .../metadata/aspect/AuditedAspect.pdl | 0 .../metadata/aspect/SoftDeletedAspect.pdl | 9 +++ 9 files changed, 202 insertions(+), 30 deletions(-) rename {dao-api => validators}/src/main/pegasus/com/linkedin/metadata/aspect/AuditedAspect.pdl (100%) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java index b79564cd6..ebb1c1c97 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java @@ -97,12 +97,26 @@ public void ensureSchemaUpToDate() { @Transactional public int add(@Nonnull URN urn, @Nullable ASPECT newValue, @Nonnull Class aspectClass, @Nonnull AuditStamp auditStamp, @Nullable IngestionTrackingContext ingestionTrackingContext, boolean isTestMode) { - return addWithOptimisticLocking(urn, newValue, aspectClass, auditStamp, null, ingestionTrackingContext, isTestMode); + return addWithOptimisticLocking(urn, null, newValue, aspectClass, auditStamp, null, ingestionTrackingContext, isTestMode); + } + + @Override + @Transactional + public int addWithOldValue( + @Nonnull URN urn, + @Nullable ASPECT oldValue, + @Nullable ASPECT newValue, + @Nonnull Class aspectClass, + @Nonnull AuditStamp auditStamp, + @Nullable IngestionTrackingContext ingestionTrackingContext, + boolean isTestMode) { + return addWithOptimisticLocking(urn, oldValue, newValue, aspectClass, auditStamp, null, ingestionTrackingContext, isTestMode); } @Override public int addWithOptimisticLocking( @Nonnull URN urn, + @Nullable ASPECT oldValue, @Nullable ASPECT newValue, @Nonnull Class aspectClass, @Nonnull AuditStamp auditStamp, @@ -135,7 +149,15 @@ public int addWithOptimisticLocking( // newValue is null if aspect is to be soft-deleted. if (newValue == null) { - return sqlUpdate.setParameter("metadata", DELETED_VALUE).execute(); + if (oldValue == null) { + // Shouldn't happen: shouldn't run delete() on an already-null/deleted aspect, or if calling to soft-delete, should + // pass in the old value. The delete() operation here will still work without issue since there is nothing + // technically wrong, but this is an unexpected usage pathway. + log.warn(String.format( + "OldValue should not be null when NewValue is null (soft-deletion). Urn: <%s> and aspect: <%s>", urn, aspectClass)); + } + return sqlUpdate.setParameter("metadata", + RecordUtils.toJsonString(createSoftDeletedAspect(aspectClass, oldValue, oldTimestamp))).execute(); } AuditedAspect auditedAspect = new AuditedAspect() @@ -149,8 +171,8 @@ public int addWithOptimisticLocking( auditedAspect.setEmitter(ingestionTrackingContext.getEmitter(), SetMode.IGNORE_NULL); } - final String metadata = toJsonString(auditedAspect); - return sqlUpdate.setParameter("metadata", metadata).execute(); + final String metadata = toJsonString(auditedAspect); + return sqlUpdate.setParameter("metadata", metadata).execute(); } /** diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java index 48e46bb39..4c5685f6b 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java @@ -634,7 +634,7 @@ protected long saveLatest(@Nonnull URN urn, @Non } } - insert(urn, newValue, aspectClass, newAuditStamp, LATEST_VERSION, trackingContext, isTestMode); + insertWithOldValue(urn, oldValue, newValue, aspectClass, newAuditStamp, LATEST_VERSION, trackingContext, isTestMode); } // This method will handle relationship ingestions and soft-deletions @@ -784,7 +784,7 @@ protected AspectEntry getLatest(@Nonnull } final ExtraInfo extraInfo = toExtraInfo(latest); - if (isSoftDeletedAspect(latest, aspectClass)) { + if (isSoftDeletedAspect(latest)) { return new AspectEntry<>(null, extraInfo, true); } @@ -878,7 +878,8 @@ protected void updateWithOptimisticLocking(@Nonn // DUAL WRITE: 1) update aspect table, 2) update entity table. // Note: when cold-archive is enabled, this method: updateWithOptimisticLocking will not be called. _server.execute(oldSchemaSqlUpdate); - return _localAccess.addWithOptimisticLocking(urn, (ASPECT) value, aspectClass, newAuditStamp, oldTimestamp, + // oldValue set to NULL: only used for soft-delete, not supported for GMS's that use metadata_aspect + return _localAccess.addWithOptimisticLocking(urn, null, (ASPECT) value, aspectClass, newAuditStamp, oldTimestamp, trackingContext, isTestMode); }, 1); } else { @@ -888,7 +889,8 @@ protected void updateWithOptimisticLocking(@Nonn numOfUpdatedRows = runInTransactionWithRetry(() -> { // Additionally, in DUAL_SCHEMA mode: apply a regular update (no optimistic locking) to the entity table if (_schemaConfig == SchemaConfig.DUAL_SCHEMA) { - _localAccess.addWithOptimisticLocking(urn, (ASPECT) value, aspectClass, newAuditStamp, null, + // oldValue set to NULL: only used for soft-delete, not supported for GMS's that use metadata_aspect + _localAccess.addWithOptimisticLocking(urn, null, (ASPECT) value, aspectClass, newAuditStamp, null, trackingContext, isTestMode); } return _server.execute(oldSchemaSqlUpdate); @@ -905,11 +907,34 @@ protected void updateWithOptimisticLocking(@Nonn protected void insert(@Nonnull URN urn, @Nullable RecordTemplate value, @Nonnull Class aspectClass, @Nonnull AuditStamp auditStamp, long version, @Nullable IngestionTrackingContext trackingContext, boolean isTestMode) { - final EbeanMetadataAspect aspect = buildMetadataAspectBean(urn, value, aspectClass, auditStamp, version); + insertWithOldValue(urn, null, value, aspectClass, auditStamp, version, trackingContext, isTestMode); + } + + /** + * Insert that also takes the old value of the aspect into account. This argument is used in the case + * Aspect soft-deletion is supposed to occur (ie. adding null). + * + *

TODO: Figure out if this should be surfaced to {@link BaseLocalDAO}'s {@link BaseLocalDAO#insert} signature. + * Right now, since it only has 1 use case -- soft-deletion and doesn't intuitively fit into the idea of an "insert", + * we are keeping it here only. + * + * @param urn entity urn + * @param oldValue old value of the aspect + * @param newValue new value of the aspect + * @param aspectClass aspect class + * @param auditStamp audit stamp + * @param version version of the record + * @param trackingContext tracking context + * @param isTestMode test mode + */ + protected void insertWithOldValue(@Nonnull URN urn, @Nullable RecordTemplate oldValue, + @Nullable RecordTemplate newValue, @Nonnull Class aspectClass, @Nonnull AuditStamp auditStamp, long version, + @Nullable IngestionTrackingContext trackingContext, boolean isTestMode) { + final EbeanMetadataAspect aspect = buildMetadataAspectBean(urn, newValue, aspectClass, auditStamp, version); if (_schemaConfig != SchemaConfig.OLD_SCHEMA_ONLY && version == LATEST_VERSION) { // insert() could be called when updating log table (moving current versions into new history version) // the metadata entity tables shouldn't been updated. - _localAccess.add(urn, (ASPECT) value, aspectClass, auditStamp, trackingContext, isTestMode); + _localAccess.addWithOldValue(urn, (ASPECT) oldValue, (ASPECT) newValue, aspectClass, auditStamp, trackingContext, isTestMode); } // DO append change log table (metadata_aspect) if: @@ -1459,7 +1484,7 @@ URN getUrn(@Nonnull String urn) { @Nonnull static Optional toRecordTemplate(@Nonnull Class aspectClass, @Nonnull EbeanMetadataAspect aspect) { - if (isSoftDeletedAspect(aspect, aspectClass)) { + if (isSoftDeletedAspect(aspect)) { return Optional.empty(); } return Optional.of(RecordUtils.toRecordTemplate(aspectClass, aspect.getMetadata())); @@ -1468,7 +1493,7 @@ static Optional toRecordTemplate(@Nonnul @Nonnull static Optional> toRecordTemplateWithExtraInfo( @Nonnull Class aspectClass, @Nonnull EbeanMetadataAspect aspect) { - if (aspect.getMetadata() == null || isSoftDeletedAspect(aspect, aspectClass)) { + if (aspect.getMetadata() == null || isSoftDeletedAspect(aspect)) { return Optional.empty(); } final ExtraInfo extraInfo = toExtraInfo(aspect); diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/IEbeanLocalAccess.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/IEbeanLocalAccess.java index 1e8b2bb00..101591ff0 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/IEbeanLocalAccess.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/IEbeanLocalAccess.java @@ -22,7 +22,7 @@ public interface IEbeanLocalAccess { void setUrnPathExtractor(@Nonnull UrnPathExtractor urnPathExtractor); /** - * Upsert aspect into entity table. + * Upsert aspect into entity table (without Optimistic Locking). * * @param metadata aspect value * @param urn entity urn @@ -36,11 +36,33 @@ public interface IEbeanLocalAccess { int add(@Nonnull URN urn, @Nullable ASPECT newValue, @Nonnull Class aspectClass, @Nonnull AuditStamp auditStamp, @Nullable IngestionTrackingContext ingestionTrackingContext, boolean isTestMode); + /** + * Same as {@link #add(Urn, RecordTemplate, Class, AuditStamp, IngestionTrackingContext, boolean)} but takes into + * account the old value of the aspect. + * + *

TODO: Consider whether to deprecate the no-OldValue version of this method to avoid confusion. + * The only use case of the OldValue at the moment is to support soft-deletion (on the new schema). + * + * @param metadata aspect value + * @param urn entity urn + * @param oldValue old aspect value in {@link RecordTemplate} + * @param newValue aspect value in {@link RecordTemplate} + * @param aspectClass class of the aspect + * @param auditStamp audit timestamp + * @param ingestionTrackingContext the ingestionTrackingContext of the MCE responsible for this update + * @param isTestMode whether the test mode is enabled or not + * @return number of rows inserted or updated + */ + int addWithOldValue(@Nonnull URN urn, @Nullable ASPECT oldValue, @Nullable ASPECT newValue, + @Nonnull Class aspectClass, @Nonnull AuditStamp auditStamp, @Nullable IngestionTrackingContext ingestionTrackingContext, + boolean isTestMode); + /** * Update aspect on entity table with optimistic locking. (compare-and-update on oldTimestamp). * * @param metadata aspect value * @param urn entity urn + * @param oldValue old aspect value in {@link RecordTemplate} * @param newValue aspect value in {@link RecordTemplate} * @param aspectClass class of the aspect * @param auditStamp audit timestamp @@ -49,7 +71,7 @@ int add(@Nonnull URN urn, @Nullable ASPECT newVa * @param isTestMode whether the test mode is enabled or not * @return number of rows inserted or updated */ - int addWithOptimisticLocking(@Nonnull URN urn, @Nullable ASPECT newValue, + int addWithOptimisticLocking(@Nonnull URN urn, @Nullable ASPECT oldValue, @Nullable ASPECT newValue, @Nonnull Class aspectClass, @Nonnull AuditStamp auditStamp, @Nullable Timestamp oldTimestamp, @Nullable IngestionTrackingContext ingestionTrackingContext, boolean isTestMode); diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java index 6ae211bb9..45c2213c3 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java @@ -176,17 +176,15 @@ public static boolean compareResults(@Nullable ListResult resultOld, @Nul } /** - * Checks whether the aspect record has been soft deleted. - * - * @param aspect aspect value - * @param aspectClass the type of the aspect - * @return boolean representing whether the aspect record has been soft deleted + * Same as {@link #isSoftDeletedAspect(SqlRow, String)}, but for {@link EbeanMetadataAspect}. */ - public static boolean isSoftDeletedAspect(@Nonnull EbeanMetadataAspect aspect, - @Nonnull Class aspectClass) { - // Convert metadata string to record template object - final RecordTemplate metadataRecord = RecordUtils.toRecordTemplate(aspectClass, aspect.getMetadata()); - return metadataRecord.equals(DELETED_METADATA); + public static boolean isSoftDeletedAspect(@Nonnull EbeanMetadataAspect aspect) { + try { + SoftDeletedAspect softDeletedAspect = RecordUtils.toRecordTemplate(SoftDeletedAspect.class, aspect.getMetadata()); + return softDeletedAspect.hasGma_deleted(); + } catch (Exception e) { + return false; + } } @@ -278,14 +276,23 @@ private static EbeanMetadataAspect readSqlRow(Sq /** * Checks whether the entity table record has been soft deleted. + * + *

NOTE: the ability to cast the aspect to a {@link SoftDeletedAspect} is sufficient to determine + * whether the aspect has been soft-deleted. This is because there are NO current use cases where we + * store a SoftDeletedAspect with the flag set to anything other than "true". + * + *

This "shallow check" is necessary because many usages of checking soft-deletion are followed by + * a deserialization call to {@link RecordUtils#toRecordTemplate(Class, String)}, which will fail if + * we try to deserialize a SoftDeletedAspect -- to another Aspect Type -- with the flag set to "false". + * * @param sqlRow {@link SqlRow} result from MySQL server * @param columnName column name of entity table * @return boolean representing whether the aspect record has been soft deleted */ public static boolean isSoftDeletedAspect(@Nonnull SqlRow sqlRow, @Nonnull String columnName) { try { - SoftDeletedAspect aspect = RecordUtils.toRecordTemplate(SoftDeletedAspect.class, sqlRow.getString(columnName)); - return aspect.hasGma_deleted() && aspect.isGma_deleted(); + SoftDeletedAspect softDeletedAspect = RecordUtils.toRecordTemplate(SoftDeletedAspect.class, sqlRow.getString(columnName)); + return softDeletedAspect.hasGma_deleted(); } catch (Exception e) { return false; } @@ -423,6 +430,34 @@ public static LocalRelationshipCriterion buildRelationshipFieldCriterion(LocalRe return relationshipMap; } + /** + * Create a soft deleted aspect with the given old value and timestamp. + * + *

NOTE: Because current write pathways for soft-deletion do not query for the full AuditedAspect, we do not have + * many fields available for us to "recreate" it; below is all we can access without performing yet another DB READ. + * In the future, if this is a gap for un-delete support, we can consider (heavy) refactoring of these pathways. + * + * @param aspectClass the class of the aspect + * @param oldValue the old value of the aspect + * @param oldTimestamp the timestamp of the old value + * @return a SoftDeletedAspect with the given old value and timestamp + */ + @Nonnull + public static SoftDeletedAspect createSoftDeletedAspect( + @Nonnull Class aspectClass, @Nullable ASPECT oldValue, @Nullable Timestamp oldTimestamp) { + final SoftDeletedAspect softDeletedAspect = new SoftDeletedAspect().setGma_deleted(true); + if (oldValue != null) { + final AuditedAspect auditedAspect = new AuditedAspect() + .setCanonicalName(aspectClass.getCanonicalName()) + .setAspect(RecordUtils.toJsonString(oldValue)); + if (oldTimestamp != null) { + auditedAspect.setLastmodifiedon(oldTimestamp.toString()); + } + softDeletedAspect.setGma_deleted_content(auditedAspect); + } + return softDeletedAspect; + } + // Using the GmaAnnotationParser, extract the model type from the @gma.model annotation on any models. private static ModelType parseModelTypeFromGmaAnnotation(RecordTemplate model) { try { diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/EbeanLocalDAOTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/EbeanLocalDAOTest.java index 6dc3d68dd..c3b0ab682 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/EbeanLocalDAOTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/EbeanLocalDAOTest.java @@ -2432,7 +2432,7 @@ public void testGetSoftDeletedAspect() { // latest version of metadata should be null EbeanMetadataAspect aspect = getMetadata(urn, aspectName, 0); - assertTrue(isSoftDeletedAspect(aspect, AspectFoo.class)); + assertTrue(isSoftDeletedAspect(aspect)); Optional fooOptional = dao.get(AspectFoo.class, urn); assertFalse(fooOptional.isPresent()); @@ -2618,7 +2618,7 @@ public void testUndeleteSoftDeletedAspect() { if (dao.isChangeLogEnabled()) { // version=3 should correspond to soft deleted metadata EbeanMetadataAspect aspect = getMetadata(urn, aspectName, 3); - assertTrue(isSoftDeletedAspect(aspect, AspectFoo.class)); + assertTrue(isSoftDeletedAspect(aspect)); fooOptional = dao.get(AspectFoo.class, urn, 3); assertFalse(fooOptional.isPresent()); diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java index 2f2b31dd8..0df97a4fc 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java @@ -6,6 +6,7 @@ import com.linkedin.data.template.RecordTemplate; import com.linkedin.data.template.StringArray; import com.linkedin.metadata.aspect.AuditedAspect; +import com.linkedin.metadata.aspect.SoftDeletedAspect; import com.linkedin.metadata.dao.BaseLocalDAO; import com.linkedin.metadata.dao.EbeanLocalAccess; import com.linkedin.metadata.dao.EbeanMetadataAspect; @@ -52,12 +53,14 @@ import static org.mockito.Mockito.*; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; -import static org.testng.AssertJUnit.assertEquals; -import static org.testng.AssertJUnit.assertNotNull; +import static org.testng.AssertJUnit.*; public class EBeanDAOUtilsTest { + final static String SOFT_DELETED_ASPECT_WITH_DELETED_CONTENT = "{\"gma_deleted\": true," + + "\"gma_deleted_content\": {\"aspect\": \"{removed: false}\", \"canonicalName\": \"com.linkedin.common.Status\"," + + "\"lastmodifiedon\": \"0L\", \"lastmodifiedby\": \"urn:li:tester\"}}"; @Nonnull private String readSQLfromFile(@Nonnull String resourcePath) { @@ -573,6 +576,37 @@ public void testIsSoftDeletedAspect() { when(sqlRow.getString("a_aspectbaz")).thenReturn("{\"random_value\": \"baz\"}"); assertFalse(EBeanDAOUtils.isSoftDeletedAspect(sqlRow, "a_aspectbaz")); + + when(sqlRow.getString("a_aspectbaz")).thenReturn("{\"random_value\": \"baz\"}"); + assertFalse(EBeanDAOUtils.isSoftDeletedAspect(sqlRow, "a_aspectbaz")); + + // NOTE how this is should return "true"; see the explanation in the method javadoc + when(sqlRow.getString("a_aspectqux")).thenReturn("{\"gma_deleted\": false}"); + assertTrue(EBeanDAOUtils.isSoftDeletedAspect(sqlRow, "a_aspectqux")); + + when(sqlRow.getString("a_aspectbax")).thenReturn(SOFT_DELETED_ASPECT_WITH_DELETED_CONTENT); + assertTrue(EBeanDAOUtils.isSoftDeletedAspect(sqlRow, "a_aspectbax")); + } + + @Test + public void testIsSoftDeletedAspectEbeanMetadataAspect() { + EbeanMetadataAspect ebeanMetadataAspect = new EbeanMetadataAspect(); + + ebeanMetadataAspect.setMetadata("{\"gma_deleted\": true}"); + assertTrue(EBeanDAOUtils.isSoftDeletedAspect(ebeanMetadataAspect)); + + ebeanMetadataAspect.setMetadata(SOFT_DELETED_ASPECT_WITH_DELETED_CONTENT); + assertTrue(EBeanDAOUtils.isSoftDeletedAspect(ebeanMetadataAspect)); + + // NOTE how this is should return "true"; see the explanation in the method javadoc + ebeanMetadataAspect.setMetadata("{\"gma_deleted\": false}"); + assertTrue(EBeanDAOUtils.isSoftDeletedAspect(ebeanMetadataAspect)); + + ebeanMetadataAspect.setMetadata("{\"aspect\": {\"value\": \"bar\"}, \"lastmodifiedby\": \"urn:li:tester\"}"); + assertFalse(EBeanDAOUtils.isSoftDeletedAspect(ebeanMetadataAspect)); + + ebeanMetadataAspect.setMetadata("{\"random_value\": \"baz\"}"); + assertFalse(EBeanDAOUtils.isSoftDeletedAspect(ebeanMetadataAspect)); } @Test @@ -668,4 +702,22 @@ public void testExtractRelationshipsFromAspect() throws URISyntaxException { assertTrue(results.get(AnnotatedRelationshipFoo.class).contains(test3)); assertTrue(results.get(AnnotatedRelationshipBar.class).contains(new AnnotatedRelationshipBar())); } + + @Test + public void testCreateSoftDeletedAspect() { + // ideal case: nonnull oldvalue and oldtimestamp + AspectFoo fooAspect = new AspectFoo().setValue("foo"); + Timestamp timestamp = new Timestamp(0L); + + SoftDeletedAspect softDeletedAspect = EBeanDAOUtils.createSoftDeletedAspect(AspectFoo.class, fooAspect, timestamp); + assertTrue(softDeletedAspect.isGma_deleted()); + assertEquals(softDeletedAspect.getGma_deleted_content().getCanonicalName(), "com.linkedin.testing.AspectFoo"); + assertEquals(softDeletedAspect.getGma_deleted_content().getAspect(), RecordUtils.toJsonString(fooAspect)); + assertEquals(softDeletedAspect.getGma_deleted_content().getLastmodifiedon(), timestamp.toString()); + + // edge case: null oldvalue + SoftDeletedAspect softDeletedAspect2 = EBeanDAOUtils.createSoftDeletedAspect(AspectFoo.class, null, timestamp); + assertTrue(softDeletedAspect.isGma_deleted()); + assertNull(softDeletedAspect2.getGma_deleted_content()); + } } \ No newline at end of file diff --git a/testing/test-models/src/main/pegasus/com/linkedin/testing/SoftDeletedAspectCopy.pdl b/testing/test-models/src/main/pegasus/com/linkedin/testing/SoftDeletedAspectCopy.pdl index c392f06a1..9cdc5a7d0 100644 --- a/testing/test-models/src/main/pegasus/com/linkedin/testing/SoftDeletedAspectCopy.pdl +++ b/testing/test-models/src/main/pegasus/com/linkedin/testing/SoftDeletedAspectCopy.pdl @@ -1,5 +1,7 @@ namespace com.linkedin.testing +import com.linkedin.metadata.aspect.AuditedAspect + /** * Captures metadata on soft deleted aspect. This is a copy of SoftDeletedAspect. */ @@ -9,4 +11,9 @@ record SoftDeletedAspectCopy { * flag to indicate if the aspect is soft deleted in GMA */ gma_deleted: boolean + + /** + * The deleted content of the aspect + */ + gma_deleted_content: optional AuditedAspect } \ No newline at end of file diff --git a/dao-api/src/main/pegasus/com/linkedin/metadata/aspect/AuditedAspect.pdl b/validators/src/main/pegasus/com/linkedin/metadata/aspect/AuditedAspect.pdl similarity index 100% rename from dao-api/src/main/pegasus/com/linkedin/metadata/aspect/AuditedAspect.pdl rename to validators/src/main/pegasus/com/linkedin/metadata/aspect/AuditedAspect.pdl diff --git a/validators/src/main/pegasus/com/linkedin/metadata/aspect/SoftDeletedAspect.pdl b/validators/src/main/pegasus/com/linkedin/metadata/aspect/SoftDeletedAspect.pdl index 99634f194..b40e9dfd8 100644 --- a/validators/src/main/pegasus/com/linkedin/metadata/aspect/SoftDeletedAspect.pdl +++ b/validators/src/main/pegasus/com/linkedin/metadata/aspect/SoftDeletedAspect.pdl @@ -7,6 +7,15 @@ record SoftDeletedAspect { /** * Flag to indicate if the aspect is soft deleted in GMA + * + * Currently, there are no cases where the aspect is soft-deleted but this flag is NOT set to + * true: the very existence of this Aspect type indicates soft-deletion. This is to say that + * any aspect that is not soft-deleted will simply not exist -- cannot be cast -- as this type. */ gma_deleted: boolean + + /** + * The deleted content of the aspect + */ + gma_deleted_content: optional AuditedAspect } \ No newline at end of file From 00c069f1cf3feebaa8d0f7b40cffcaae3e8069e2 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Mon, 12 May 2025 23:46:13 -0400 Subject: [PATCH 02/12] add one more test --- .../linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java index 0df97a4fc..0df222314 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java @@ -715,9 +715,16 @@ public void testCreateSoftDeletedAspect() { assertEquals(softDeletedAspect.getGma_deleted_content().getAspect(), RecordUtils.toJsonString(fooAspect)); assertEquals(softDeletedAspect.getGma_deleted_content().getLastmodifiedon(), timestamp.toString()); - // edge case: null oldvalue + // edge case: null oldvalue, oldtimetstamp is irrelevant SoftDeletedAspect softDeletedAspect2 = EBeanDAOUtils.createSoftDeletedAspect(AspectFoo.class, null, timestamp); assertTrue(softDeletedAspect.isGma_deleted()); assertNull(softDeletedAspect2.getGma_deleted_content()); + + // edge case: nonnull oldvalue but null timestamp + SoftDeletedAspect softDeletedAspect3 = EBeanDAOUtils.createSoftDeletedAspect(AspectFoo.class, fooAspect, null); + assertTrue(softDeletedAspect.isGma_deleted()); + assertEquals(softDeletedAspect3.getGma_deleted_content().getCanonicalName(), "com.linkedin.testing.AspectFoo"); + assertEquals(softDeletedAspect3.getGma_deleted_content().getAspect(), RecordUtils.toJsonString(fooAspect)); + assertFalse(softDeletedAspect3.getGma_deleted_content().hasLastmodifiedon()); } } \ No newline at end of file From 23365b830a7f74241a5a930e6cd262c9768a747a Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Tue, 13 May 2025 11:58:00 -0400 Subject: [PATCH 03/12] change validation approach --- .../metadata/dao/utils/EBeanDAOUtils.java | 42 ++++++++++--------- .../metadata/dao/utils/EBeanDAOUtilsTest.java | 7 ++++ .../metadata/validator/ValidationUtils.java | 4 +- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java index 45c2213c3..0129f804f 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java @@ -1,5 +1,6 @@ package com.linkedin.metadata.dao.utils; +import com.google.common.annotations.VisibleForTesting; import com.linkedin.common.urn.Urn; import com.linkedin.data.schema.RecordDataSchema; import com.linkedin.data.template.DataTemplateUtil; @@ -19,6 +20,7 @@ import com.linkedin.metadata.query.LocalRelationshipValue; import com.linkedin.metadata.query.RelationshipField; import com.linkedin.metadata.query.UrnField; +import com.linkedin.metadata.validator.ValidationUtils; import io.ebean.EbeanServer; import io.ebean.SqlRow; import java.lang.reflect.InvocationTargetException; @@ -175,19 +177,6 @@ public static boolean compareResults(@Nullable ListResult resultOld, @Nul return false; } - /** - * Same as {@link #isSoftDeletedAspect(SqlRow, String)}, but for {@link EbeanMetadataAspect}. - */ - public static boolean isSoftDeletedAspect(@Nonnull EbeanMetadataAspect aspect) { - try { - SoftDeletedAspect softDeletedAspect = RecordUtils.toRecordTemplate(SoftDeletedAspect.class, aspect.getMetadata()); - return softDeletedAspect.hasGma_deleted(); - } catch (Exception e) { - return false; - } - } - - /** * Read {@link SqlRow} list into a {@link EbeanMetadataAspect} list. * @param sqlRows list of {@link SqlRow} @@ -275,13 +264,14 @@ private static EbeanMetadataAspect readSqlRow(Sq } /** - * Checks whether the entity table record has been soft deleted. + * Checks whether a record is Soft Deleted. * - *

NOTE: the ability to cast the aspect to a {@link SoftDeletedAspect} is sufficient to determine - * whether the aspect has been soft-deleted. This is because there are NO current use cases where we - * store a SoftDeletedAspect with the flag set to anything other than "true". + *

NOTE: Since soft deleted aspects are modeled as a specific aspect type -- SoftDeletedAspect -- the ability + * to cast the aspect to a {@link SoftDeletedAspect} is sufficient to determine whether the aspect *is* Soft Deleted. + * In other words, there are no current use cases where we store a SoftDeletedAspect with the `gma_deleted` flag set to + * anything other than "true". * - *

This "shallow check" is necessary because many usages of checking soft-deletion are followed by + *

This validation approach is necessary because many usages of checking soft-deletion are followed by * a deserialization call to {@link RecordUtils#toRecordTemplate(Class, String)}, which will fail if * we try to deserialize a SoftDeletedAspect -- to another Aspect Type -- with the flag set to "false". * @@ -290,9 +280,21 @@ private static EbeanMetadataAspect readSqlRow(Sq * @return boolean representing whether the aspect record has been soft deleted */ public static boolean isSoftDeletedAspect(@Nonnull SqlRow sqlRow, @Nonnull String columnName) { + return isSoftDeletedAspect(sqlRow.getString(columnName)); + } + + /** + * Same as {@link #isSoftDeletedAspect(SqlRow, String)}, but for {@link EbeanMetadataAspect}. + */ + public static boolean isSoftDeletedAspect(@Nonnull EbeanMetadataAspect aspect) { + return isSoftDeletedAspect(aspect.getMetadata()); + } + + private static boolean isSoftDeletedAspect(@Nonnull String metadata) { try { - SoftDeletedAspect softDeletedAspect = RecordUtils.toRecordTemplate(SoftDeletedAspect.class, sqlRow.getString(columnName)); - return softDeletedAspect.hasGma_deleted(); + SoftDeletedAspect softDeletedAspect = RecordUtils.toRecordTemplate(SoftDeletedAspect.class, metadata); + ValidationUtils.validateAgainstSchema(softDeletedAspect); + return true; } catch (Exception e) { return false; } diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java index 0df222314..6ddff0cd9 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java @@ -586,6 +586,10 @@ public void testIsSoftDeletedAspect() { when(sqlRow.getString("a_aspectbax")).thenReturn(SOFT_DELETED_ASPECT_WITH_DELETED_CONTENT); assertTrue(EBeanDAOUtils.isSoftDeletedAspect(sqlRow, "a_aspectbax")); + + when(sqlRow.getString("a_aspectbazz")) + .thenReturn("input that should fail being able to serialize as a RecordTemplate entirely"); + assertFalse(EBeanDAOUtils.isSoftDeletedAspect(sqlRow, "a_aspectbazz")); } @Test @@ -607,6 +611,9 @@ public void testIsSoftDeletedAspectEbeanMetadataAspect() { ebeanMetadataAspect.setMetadata("{\"random_value\": \"baz\"}"); assertFalse(EBeanDAOUtils.isSoftDeletedAspect(ebeanMetadataAspect)); + + ebeanMetadataAspect.setMetadata("input that should fail being able to serialize as a RecordTemplate entirely"); + assertFalse(EBeanDAOUtils.isSoftDeletedAspect(ebeanMetadataAspect)); } @Test diff --git a/validators/src/main/java/com/linkedin/metadata/validator/ValidationUtils.java b/validators/src/main/java/com/linkedin/metadata/validator/ValidationUtils.java index 4ac576271..4b68fde05 100644 --- a/validators/src/main/java/com/linkedin/metadata/validator/ValidationUtils.java +++ b/validators/src/main/java/com/linkedin/metadata/validator/ValidationUtils.java @@ -221,10 +221,10 @@ public static void throwNullFieldException(String field) { /** * Validates a model against its schema, useful for checking to make sure that a (curli) ingestion request's - * fields are all valid fields of the model. + * fields are all valid fields of the model: unrecognized fields should be disallowed. * *

This is copied from BaseLocalDAO, which uses this for Aspect-level ingestion validation. This is meant for - * Asset-level validation.

+ * Asset-level validation but can be used generally as well.

* *

Reference ticket: META-21242

*/ From 5e59344cc9d5aff45d9b5f8a5a3b35abc3ebdf03 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Tue, 13 May 2025 13:07:28 -0400 Subject: [PATCH 04/12] adjust for validation --- .../linkedin/metadata/dao/utils/EBeanDAOUtils.java | 11 ++++++----- .../metadata/dao/utils/EBeanDAOUtilsTest.java | 4 +++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java index 0129f804f..7cbef415f 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java @@ -1,6 +1,5 @@ package com.linkedin.metadata.dao.utils; -import com.google.common.annotations.VisibleForTesting; import com.linkedin.common.urn.Urn; import com.linkedin.data.schema.RecordDataSchema; import com.linkedin.data.template.DataTemplateUtil; @@ -61,6 +60,7 @@ public class EBeanDAOUtils { private static final RecordTemplate DELETED_METADATA = new SoftDeletedAspect().setGma_deleted(true); public static final String DELETED_VALUE = RecordUtils.toJsonString(DELETED_METADATA); private static final long LATEST_VERSION = 0L; + public static final String UNSET_STRING_PLACEHOLDER = "NOT_SET"; private EBeanDAOUtils() { // Utils class @@ -439,6 +439,8 @@ public static LocalRelationshipCriterion buildRelationshipFieldCriterion(LocalRe * many fields available for us to "recreate" it; below is all we can access without performing yet another DB READ. * In the future, if this is a gap for un-delete support, we can consider (heavy) refactoring of these pathways. * + *

Note that in order to adhere to Pegasus-defined schema, we need to ensure that "required" fields are set. + * * @param aspectClass the class of the aspect * @param oldValue the old value of the aspect * @param oldTimestamp the timestamp of the old value @@ -451,10 +453,9 @@ public static SoftDeletedAspect createSoftDelete if (oldValue != null) { final AuditedAspect auditedAspect = new AuditedAspect() .setCanonicalName(aspectClass.getCanonicalName()) - .setAspect(RecordUtils.toJsonString(oldValue)); - if (oldTimestamp != null) { - auditedAspect.setLastmodifiedon(oldTimestamp.toString()); - } + .setAspect(RecordUtils.toJsonString(oldValue)) + .setLastmodifiedon(oldTimestamp != null ? oldTimestamp.toString() : UNSET_STRING_PLACEHOLDER) + .setLastmodifiedby(UNSET_STRING_PLACEHOLDER); softDeletedAspect.setGma_deleted_content(auditedAspect); } return softDeletedAspect; diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java index 6ddff0cd9..07d3c4147 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java @@ -721,6 +721,7 @@ public void testCreateSoftDeletedAspect() { assertEquals(softDeletedAspect.getGma_deleted_content().getCanonicalName(), "com.linkedin.testing.AspectFoo"); assertEquals(softDeletedAspect.getGma_deleted_content().getAspect(), RecordUtils.toJsonString(fooAspect)); assertEquals(softDeletedAspect.getGma_deleted_content().getLastmodifiedon(), timestamp.toString()); + assertEquals(softDeletedAspect.getGma_deleted_content().getLastmodifiedby(), EBeanDAOUtils.UNSET_STRING_PLACEHOLDER); // edge case: null oldvalue, oldtimetstamp is irrelevant SoftDeletedAspect softDeletedAspect2 = EBeanDAOUtils.createSoftDeletedAspect(AspectFoo.class, null, timestamp); @@ -732,6 +733,7 @@ public void testCreateSoftDeletedAspect() { assertTrue(softDeletedAspect.isGma_deleted()); assertEquals(softDeletedAspect3.getGma_deleted_content().getCanonicalName(), "com.linkedin.testing.AspectFoo"); assertEquals(softDeletedAspect3.getGma_deleted_content().getAspect(), RecordUtils.toJsonString(fooAspect)); - assertFalse(softDeletedAspect3.getGma_deleted_content().hasLastmodifiedon()); + assertEquals(softDeletedAspect3.getGma_deleted_content().getLastmodifiedon(), EBeanDAOUtils.UNSET_STRING_PLACEHOLDER); + assertEquals(softDeletedAspect3.getGma_deleted_content().getLastmodifiedby(), EBeanDAOUtils.UNSET_STRING_PLACEHOLDER); } } \ No newline at end of file From 5eb46ca99148ad9ad71646996d0f1bb8f85daf11 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Thu, 5 Jun 2025 23:33:24 -0700 Subject: [PATCH 05/12] adjustments made post-review, need a screencap of db tho --- .../metadata/dao/EbeanLocalAccess.java | 24 +++++++++++---- .../linkedin/metadata/dao/EbeanLocalDAO.java | 6 ++-- .../metadata/dao/IEbeanLocalAccess.java | 3 +- .../metadata/dao/utils/EBeanDAOUtils.java | 25 ++++++---------- .../metadata/dao/utils/EBeanDAOUtilsTest.java | 29 ++++++------------- .../testing/SoftDeletedAspectCopy.pdl | 19 ++++++++++-- .../metadata/aspect/SoftDeletedAspect.pdl | 23 +++++++++++++-- 7 files changed, 77 insertions(+), 52 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java index d7470d81d..bc183eeee 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java @@ -100,7 +100,7 @@ public void ensureSchemaUpToDate() { @Transactional public int add(@Nonnull URN urn, @Nullable ASPECT newValue, @Nonnull Class aspectClass, @Nonnull AuditStamp auditStamp, @Nullable IngestionTrackingContext ingestionTrackingContext, boolean isTestMode) { - return addWithOptimisticLocking(urn, null, newValue, aspectClass, auditStamp, null, ingestionTrackingContext, isTestMode); + return addWithOptimisticLocking(urn, newValue, aspectClass, auditStamp, null, ingestionTrackingContext, isTestMode); } @Override @@ -118,6 +118,17 @@ public int addWithOldValue( @Override public int addWithOptimisticLocking( + @Nonnull URN urn, + @Nullable ASPECT newValue, + @Nonnull Class aspectClass, + @Nonnull AuditStamp auditStamp, + @Nullable Timestamp oldTimestamp, + @Nullable IngestionTrackingContext ingestionTrackingContext, + boolean isTestMode) { + return addWithOptimisticLocking(urn, null, newValue, aspectClass, auditStamp, oldTimestamp, ingestionTrackingContext, isTestMode); + } + + private int addWithOptimisticLocking( @Nonnull URN urn, @Nullable ASPECT oldValue, @Nullable ASPECT newValue, @@ -150,17 +161,18 @@ public int addWithOptimisticLocking( sqlUpdate.setParameter("a_urn", toJsonString(urn)); } - // newValue is null if aspect is to be soft-deleted. + // newValue is null if aspect is to be (soft-)deleted. if (newValue == null) { if (oldValue == null) { // Shouldn't happen: shouldn't run delete() on an already-null/deleted aspect, or if calling to soft-delete, should - // pass in the old value. The delete() operation here will still work without issue since there is nothing - // technically wrong, but this is an unexpected usage pathway. + // pass in the old value. + // Since Old Schema logic that calls this method does NOT have access to the old value (without significant refactoring) + // we make the decision to log a warning and proceed with the soft-deletion with an empty "deletedContent" value. log.warn(String.format( "OldValue should not be null when NewValue is null (soft-deletion). Urn: <%s> and aspect: <%s>", urn, aspectClass)); } - return sqlUpdate.setParameter("metadata", - RecordUtils.toJsonString(createSoftDeletedAspect(aspectClass, oldValue, oldTimestamp))).execute(); + return sqlUpdate.setParameter("metadata", RecordUtils.toJsonString( + createSoftDeletedAspect(aspectClass, oldValue, new Timestamp(timestamp), actor))).execute(); } AuditedAspect auditedAspect = new AuditedAspect() diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java index 4c5685f6b..9bfdc8381 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java @@ -878,8 +878,7 @@ protected void updateWithOptimisticLocking(@Nonn // DUAL WRITE: 1) update aspect table, 2) update entity table. // Note: when cold-archive is enabled, this method: updateWithOptimisticLocking will not be called. _server.execute(oldSchemaSqlUpdate); - // oldValue set to NULL: only used for soft-delete, not supported for GMS's that use metadata_aspect - return _localAccess.addWithOptimisticLocking(urn, null, (ASPECT) value, aspectClass, newAuditStamp, oldTimestamp, + return _localAccess.addWithOptimisticLocking(urn, (ASPECT) value, aspectClass, newAuditStamp, oldTimestamp, trackingContext, isTestMode); }, 1); } else { @@ -889,8 +888,7 @@ protected void updateWithOptimisticLocking(@Nonn numOfUpdatedRows = runInTransactionWithRetry(() -> { // Additionally, in DUAL_SCHEMA mode: apply a regular update (no optimistic locking) to the entity table if (_schemaConfig == SchemaConfig.DUAL_SCHEMA) { - // oldValue set to NULL: only used for soft-delete, not supported for GMS's that use metadata_aspect - _localAccess.addWithOptimisticLocking(urn, null, (ASPECT) value, aspectClass, newAuditStamp, null, + _localAccess.addWithOptimisticLocking(urn, (ASPECT) value, aspectClass, newAuditStamp, null, trackingContext, isTestMode); } return _server.execute(oldSchemaSqlUpdate); diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/IEbeanLocalAccess.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/IEbeanLocalAccess.java index 101591ff0..65091ef18 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/IEbeanLocalAccess.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/IEbeanLocalAccess.java @@ -62,7 +62,6 @@ int addWithOldValue(@Nonnull URN urn, @Nullable * * @param metadata aspect value * @param urn entity urn - * @param oldValue old aspect value in {@link RecordTemplate} * @param newValue aspect value in {@link RecordTemplate} * @param aspectClass class of the aspect * @param auditStamp audit timestamp @@ -71,7 +70,7 @@ int addWithOldValue(@Nonnull URN urn, @Nullable * @param isTestMode whether the test mode is enabled or not * @return number of rows inserted or updated */ - int addWithOptimisticLocking(@Nonnull URN urn, @Nullable ASPECT oldValue, @Nullable ASPECT newValue, + int addWithOptimisticLocking(@Nonnull URN urn, @Nullable ASPECT newValue, @Nonnull Class aspectClass, @Nonnull AuditStamp auditStamp, @Nullable Timestamp oldTimestamp, @Nullable IngestionTrackingContext ingestionTrackingContext, boolean isTestMode); diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java index 7cbef415f..3070b7627 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java @@ -435,30 +435,23 @@ public static LocalRelationshipCriterion buildRelationshipFieldCriterion(LocalRe /** * Create a soft deleted aspect with the given old value and timestamp. * - *

NOTE: Because current write pathways for soft-deletion do not query for the full AuditedAspect, we do not have - * many fields available for us to "recreate" it; below is all we can access without performing yet another DB READ. - * In the future, if this is a gap for un-delete support, we can consider (heavy) refactoring of these pathways. - * - *

Note that in order to adhere to Pegasus-defined schema, we need to ensure that "required" fields are set. - * * @param aspectClass the class of the aspect * @param oldValue the old value of the aspect - * @param oldTimestamp the timestamp of the old value + * @param deletedTimestamp the timestamp of the old value + * @param deletedBy the user who deleted the aspect * @return a SoftDeletedAspect with the given old value and timestamp */ @Nonnull public static SoftDeletedAspect createSoftDeletedAspect( - @Nonnull Class aspectClass, @Nullable ASPECT oldValue, @Nullable Timestamp oldTimestamp) { - final SoftDeletedAspect softDeletedAspect = new SoftDeletedAspect().setGma_deleted(true); + @Nonnull Class aspectClass, @Nullable ASPECT oldValue, @Nonnull Timestamp deletedTimestamp, @Nonnull String deletedBy) { + SoftDeletedAspect sda = new SoftDeletedAspect().setGma_deleted(true) + .setCanonicalName(aspectClass.getCanonicalName()) + .setDeletedTimestamp(deletedTimestamp.toString()) + .setDeletedBy(deletedBy); if (oldValue != null) { - final AuditedAspect auditedAspect = new AuditedAspect() - .setCanonicalName(aspectClass.getCanonicalName()) - .setAspect(RecordUtils.toJsonString(oldValue)) - .setLastmodifiedon(oldTimestamp != null ? oldTimestamp.toString() : UNSET_STRING_PLACEHOLDER) - .setLastmodifiedby(UNSET_STRING_PLACEHOLDER); - softDeletedAspect.setGma_deleted_content(auditedAspect); + sda.setDeletedContent(RecordUtils.toJsonString(oldValue)); } - return softDeletedAspect; + return sda; } // Using the GmaAnnotationParser, extract the model type from the @gma.model annotation on any models. diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java index 07d3c4147..df2859f5b 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java @@ -59,8 +59,8 @@ public class EBeanDAOUtilsTest { final static String SOFT_DELETED_ASPECT_WITH_DELETED_CONTENT = "{\"gma_deleted\": true," - + "\"gma_deleted_content\": {\"aspect\": \"{removed: false}\", \"canonicalName\": \"com.linkedin.common.Status\"," - + "\"lastmodifiedon\": \"0L\", \"lastmodifiedby\": \"urn:li:tester\"}}"; + + "\"deletedContent\": \"{removed: false}\", \"canonicalName\": \"com.linkedin.common.Status\"," + + "\"deletedTimestamp\": \"0L\", \"deletedBy\": \"urn:li:tester\"}"; @Nonnull private String readSQLfromFile(@Nonnull String resourcePath) { @@ -715,25 +715,14 @@ public void testCreateSoftDeletedAspect() { // ideal case: nonnull oldvalue and oldtimestamp AspectFoo fooAspect = new AspectFoo().setValue("foo"); Timestamp timestamp = new Timestamp(0L); + String actor = "actor"; - SoftDeletedAspect softDeletedAspect = EBeanDAOUtils.createSoftDeletedAspect(AspectFoo.class, fooAspect, timestamp); + SoftDeletedAspect softDeletedAspect = + EBeanDAOUtils.createSoftDeletedAspect(AspectFoo.class, fooAspect, timestamp, actor); assertTrue(softDeletedAspect.isGma_deleted()); - assertEquals(softDeletedAspect.getGma_deleted_content().getCanonicalName(), "com.linkedin.testing.AspectFoo"); - assertEquals(softDeletedAspect.getGma_deleted_content().getAspect(), RecordUtils.toJsonString(fooAspect)); - assertEquals(softDeletedAspect.getGma_deleted_content().getLastmodifiedon(), timestamp.toString()); - assertEquals(softDeletedAspect.getGma_deleted_content().getLastmodifiedby(), EBeanDAOUtils.UNSET_STRING_PLACEHOLDER); - - // edge case: null oldvalue, oldtimetstamp is irrelevant - SoftDeletedAspect softDeletedAspect2 = EBeanDAOUtils.createSoftDeletedAspect(AspectFoo.class, null, timestamp); - assertTrue(softDeletedAspect.isGma_deleted()); - assertNull(softDeletedAspect2.getGma_deleted_content()); - - // edge case: nonnull oldvalue but null timestamp - SoftDeletedAspect softDeletedAspect3 = EBeanDAOUtils.createSoftDeletedAspect(AspectFoo.class, fooAspect, null); - assertTrue(softDeletedAspect.isGma_deleted()); - assertEquals(softDeletedAspect3.getGma_deleted_content().getCanonicalName(), "com.linkedin.testing.AspectFoo"); - assertEquals(softDeletedAspect3.getGma_deleted_content().getAspect(), RecordUtils.toJsonString(fooAspect)); - assertEquals(softDeletedAspect3.getGma_deleted_content().getLastmodifiedon(), EBeanDAOUtils.UNSET_STRING_PLACEHOLDER); - assertEquals(softDeletedAspect3.getGma_deleted_content().getLastmodifiedby(), EBeanDAOUtils.UNSET_STRING_PLACEHOLDER); + assertEquals(softDeletedAspect.getCanonicalName(), "com.linkedin.testing.AspectFoo"); + assertEquals(softDeletedAspect.getDeletedContent(), RecordUtils.toJsonString(fooAspect)); + assertEquals(softDeletedAspect.getDeletedTimestamp(), timestamp.toString()); + assertEquals(softDeletedAspect.getDeletedBy(), actor); } } \ No newline at end of file diff --git a/testing/test-models/src/main/pegasus/com/linkedin/testing/SoftDeletedAspectCopy.pdl b/testing/test-models/src/main/pegasus/com/linkedin/testing/SoftDeletedAspectCopy.pdl index 9cdc5a7d0..5287ba64f 100644 --- a/testing/test-models/src/main/pegasus/com/linkedin/testing/SoftDeletedAspectCopy.pdl +++ b/testing/test-models/src/main/pegasus/com/linkedin/testing/SoftDeletedAspectCopy.pdl @@ -13,7 +13,22 @@ record SoftDeletedAspectCopy { gma_deleted: boolean /** - * The deleted content of the aspect + * The deleted content (oldValue) of the aspect */ - gma_deleted_content: optional AuditedAspect + deletedContent: optional string + + /** + * The canonical class name of the aspect. + */ + canonicalName: optional string + + /** + * Audit timestamp to track when this aspect was deleted. + */ + deletedTimestamp: optional string + + /** + * Audit information to track who deleted this aspect. + */ + deletedBy: optional string } \ No newline at end of file diff --git a/validators/src/main/pegasus/com/linkedin/metadata/aspect/SoftDeletedAspect.pdl b/validators/src/main/pegasus/com/linkedin/metadata/aspect/SoftDeletedAspect.pdl index b40e9dfd8..432b4e7a1 100644 --- a/validators/src/main/pegasus/com/linkedin/metadata/aspect/SoftDeletedAspect.pdl +++ b/validators/src/main/pegasus/com/linkedin/metadata/aspect/SoftDeletedAspect.pdl @@ -15,7 +15,26 @@ record SoftDeletedAspect { gma_deleted: boolean /** - * The deleted content of the aspect + * The deleted content (oldValue) of the aspect. + * Marked as optional for backwards compatibility with existing Deleted Aspects. */ - gma_deleted_content: optional AuditedAspect + deletedContent: optional string + + /** + * The canonical class name of the aspect. + * Marked as optional for backwards compatibility with existing Deleted Aspects. + */ + canonicalName: optional string + + /** + * Audit timestamp to track when this aspect was deleted. + * Marked as optional for backwards compatibility with existing Deleted Aspects. + */ + deletedTimestamp: optional string + + /** + * Audit information to track who deleted this aspect. + * Marked as optional for backwards compatibility with existing Deleted Aspects. + */ + deletedBy: optional string } \ No newline at end of file From b50ca5150d2200a6b3c2d64c959c1ea732096250 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Thu, 5 Jun 2025 23:35:13 -0700 Subject: [PATCH 06/12] move file --- .../main/pegasus/com/linkedin/metadata/aspect/AuditedAspect.pdl | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {validators => dao-api}/src/main/pegasus/com/linkedin/metadata/aspect/AuditedAspect.pdl (100%) diff --git a/validators/src/main/pegasus/com/linkedin/metadata/aspect/AuditedAspect.pdl b/dao-api/src/main/pegasus/com/linkedin/metadata/aspect/AuditedAspect.pdl similarity index 100% rename from validators/src/main/pegasus/com/linkedin/metadata/aspect/AuditedAspect.pdl rename to dao-api/src/main/pegasus/com/linkedin/metadata/aspect/AuditedAspect.pdl From 2b290ceb83978821b9e29f72a39bcbf7b75e5c34 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Thu, 5 Jun 2025 23:41:20 -0700 Subject: [PATCH 07/12] docs and removal --- .../java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java index 3070b7627..03fcc2109 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java @@ -60,7 +60,6 @@ public class EBeanDAOUtils { private static final RecordTemplate DELETED_METADATA = new SoftDeletedAspect().setGma_deleted(true); public static final String DELETED_VALUE = RecordUtils.toJsonString(DELETED_METADATA); private static final long LATEST_VERSION = 0L; - public static final String UNSET_STRING_PLACEHOLDER = "NOT_SET"; private EBeanDAOUtils() { // Utils class @@ -435,6 +434,9 @@ public static LocalRelationshipCriterion buildRelationshipFieldCriterion(LocalRe /** * Create a soft deleted aspect with the given old value and timestamp. * + * TODO: Make oldValue @Nonnull once the Old Schema is completed deprecated OR if old schema write pathways are + * refactored to be able to process (retrieve and then write) the old value. + * * @param aspectClass the class of the aspect * @param oldValue the old value of the aspect * @param deletedTimestamp the timestamp of the old value From 72088c6244d73ab5452e67002681b02c7382e410 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Thu, 5 Jun 2025 23:44:42 -0700 Subject: [PATCH 08/12] nit --- .../java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java index 03fcc2109..9e86f1187 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java @@ -433,7 +433,7 @@ public static LocalRelationshipCriterion buildRelationshipFieldCriterion(LocalRe /** * Create a soft deleted aspect with the given old value and timestamp. - * + *

* TODO: Make oldValue @Nonnull once the Old Schema is completed deprecated OR if old schema write pathways are * refactored to be able to process (retrieve and then write) the old value. * From aa364431702e9f57689f7aef37734192671187c7 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Thu, 5 Jun 2025 23:49:40 -0700 Subject: [PATCH 09/12] final nit --- .../java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java index 9e86f1187..092b565d8 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java @@ -433,8 +433,8 @@ public static LocalRelationshipCriterion buildRelationshipFieldCriterion(LocalRe /** * Create a soft deleted aspect with the given old value and timestamp. - *

- * TODO: Make oldValue @Nonnull once the Old Schema is completed deprecated OR if old schema write pathways are + * + *

TODO: Make oldValue @Nonnull once the Old Schema is completed deprecated OR if old schema write pathways are * refactored to be able to process (retrieve and then write) the old value. * * @param aspectClass the class of the aspect From ca0d4b2de280997fdd43d6cd9382d70acc7290cb Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Tue, 10 Jun 2025 11:17:09 -0700 Subject: [PATCH 10/12] address comments --- .../java/com/linkedin/metadata/dao/EbeanLocalAccess.java | 4 ++-- .../java/com/linkedin/metadata/dao/EbeanLocalDAO.java | 6 +++--- .../com/linkedin/metadata/dao/utils/EBeanDAOUtils.java | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java index bc183eeee..c94e244f2 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java @@ -125,10 +125,10 @@ public int addWithOptimisticLocking( @Nullable Timestamp oldTimestamp, @Nullable IngestionTrackingContext ingestionTrackingContext, boolean isTestMode) { - return addWithOptimisticLocking(urn, null, newValue, aspectClass, auditStamp, oldTimestamp, ingestionTrackingContext, isTestMode); + return updateWithOptimisticLocking(urn, null, newValue, aspectClass, auditStamp, oldTimestamp, ingestionTrackingContext, isTestMode); } - private int addWithOptimisticLocking( + private int updateWithOptimisticLocking( @Nonnull URN urn, @Nullable ASPECT oldValue, @Nullable ASPECT newValue, diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java index 9bfdc8381..e7c973cd5 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalDAO.java @@ -634,7 +634,7 @@ protected long saveLatest(@Nonnull URN urn, @Non } } - insertWithOldValue(urn, oldValue, newValue, aspectClass, newAuditStamp, LATEST_VERSION, trackingContext, isTestMode); + insert(urn, oldValue, newValue, aspectClass, newAuditStamp, LATEST_VERSION, trackingContext, isTestMode); } // This method will handle relationship ingestions and soft-deletions @@ -905,7 +905,7 @@ protected void updateWithOptimisticLocking(@Nonn protected void insert(@Nonnull URN urn, @Nullable RecordTemplate value, @Nonnull Class aspectClass, @Nonnull AuditStamp auditStamp, long version, @Nullable IngestionTrackingContext trackingContext, boolean isTestMode) { - insertWithOldValue(urn, null, value, aspectClass, auditStamp, version, trackingContext, isTestMode); + insert(urn, null, value, aspectClass, auditStamp, version, trackingContext, isTestMode); } /** @@ -925,7 +925,7 @@ protected void insert(@Nonnull URN urn, @Nullabl * @param trackingContext tracking context * @param isTestMode test mode */ - protected void insertWithOldValue(@Nonnull URN urn, @Nullable RecordTemplate oldValue, + protected void insert(@Nonnull URN urn, @Nullable RecordTemplate oldValue, @Nullable RecordTemplate newValue, @Nonnull Class aspectClass, @Nonnull AuditStamp auditStamp, long version, @Nullable IngestionTrackingContext trackingContext, boolean isTestMode) { final EbeanMetadataAspect aspect = buildMetadataAspectBean(urn, newValue, aspectClass, auditStamp, version); diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java index 092b565d8..9cf703f4e 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java @@ -293,7 +293,7 @@ private static boolean isSoftDeletedAspect(@Nonnull String metadata) { try { SoftDeletedAspect softDeletedAspect = RecordUtils.toRecordTemplate(SoftDeletedAspect.class, metadata); ValidationUtils.validateAgainstSchema(softDeletedAspect); - return true; + return softDeletedAspect.hasGma_deleted() && softDeletedAspect.isGma_deleted(); } catch (Exception e) { return false; } @@ -446,14 +446,14 @@ public static LocalRelationshipCriterion buildRelationshipFieldCriterion(LocalRe @Nonnull public static SoftDeletedAspect createSoftDeletedAspect( @Nonnull Class aspectClass, @Nullable ASPECT oldValue, @Nonnull Timestamp deletedTimestamp, @Nonnull String deletedBy) { - SoftDeletedAspect sda = new SoftDeletedAspect().setGma_deleted(true) + SoftDeletedAspect softDeletedAspect = new SoftDeletedAspect().setGma_deleted(true) .setCanonicalName(aspectClass.getCanonicalName()) .setDeletedTimestamp(deletedTimestamp.toString()) .setDeletedBy(deletedBy); if (oldValue != null) { - sda.setDeletedContent(RecordUtils.toJsonString(oldValue)); + softDeletedAspect.setDeletedContent(RecordUtils.toJsonString(oldValue)); } - return sda; + return softDeletedAspect; } // Using the GmaAnnotationParser, extract the model type from the @gma.model annotation on any models. From 9861b51f555803300056b863c16a8b9147f60c11 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Tue, 10 Jun 2025 11:17:42 -0700 Subject: [PATCH 11/12] typo --- .../main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java index c94e244f2..0b62d636f 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanLocalAccess.java @@ -113,7 +113,7 @@ public int addWithOldValue( @Nonnull AuditStamp auditStamp, @Nullable IngestionTrackingContext ingestionTrackingContext, boolean isTestMode) { - return addWithOptimisticLocking(urn, oldValue, newValue, aspectClass, auditStamp, null, ingestionTrackingContext, isTestMode); + return updateWithOptimisticLocking(urn, oldValue, newValue, aspectClass, auditStamp, null, ingestionTrackingContext, isTestMode); } @Override From 18b58513a1133b2b1e6705303884b46c05f20004 Mon Sep 17 00:00:00 2001 From: Jonathan Hui Date: Tue, 10 Jun 2025 15:04:31 -0700 Subject: [PATCH 12/12] fix tests and desc --- .../java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java | 6 +++--- .../com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java | 6 ++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java index 9cf703f4e..244007930 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java @@ -270,9 +270,9 @@ private static EbeanMetadataAspect readSqlRow(Sq * In other words, there are no current use cases where we store a SoftDeletedAspect with the `gma_deleted` flag set to * anything other than "true". * - *

This validation approach is necessary because many usages of checking soft-deletion are followed by - * a deserialization call to {@link RecordUtils#toRecordTemplate(Class, String)}, which will fail if - * we try to deserialize a SoftDeletedAspect -- to another Aspect Type -- with the flag set to "false". + *

While the validation implemented additionally checks for the setting of the flag, NOTE that some usages of checking + * soft-deletion are followed by a deserialization call to {@link RecordUtils#toRecordTemplate(Class, String)}, which + * will fail when we try to deserialize a SoftDeletedAspect -- to another Aspect Type -- with the flag set to "false". * * @param sqlRow {@link SqlRow} result from MySQL server * @param columnName column name of entity table diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java index df2859f5b..c33fbc8f8 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java @@ -580,9 +580,8 @@ public void testIsSoftDeletedAspect() { when(sqlRow.getString("a_aspectbaz")).thenReturn("{\"random_value\": \"baz\"}"); assertFalse(EBeanDAOUtils.isSoftDeletedAspect(sqlRow, "a_aspectbaz")); - // NOTE how this is should return "true"; see the explanation in the method javadoc when(sqlRow.getString("a_aspectqux")).thenReturn("{\"gma_deleted\": false}"); - assertTrue(EBeanDAOUtils.isSoftDeletedAspect(sqlRow, "a_aspectqux")); + assertFalse(EBeanDAOUtils.isSoftDeletedAspect(sqlRow, "a_aspectqux")); when(sqlRow.getString("a_aspectbax")).thenReturn(SOFT_DELETED_ASPECT_WITH_DELETED_CONTENT); assertTrue(EBeanDAOUtils.isSoftDeletedAspect(sqlRow, "a_aspectbax")); @@ -602,9 +601,8 @@ public void testIsSoftDeletedAspectEbeanMetadataAspect() { ebeanMetadataAspect.setMetadata(SOFT_DELETED_ASPECT_WITH_DELETED_CONTENT); assertTrue(EBeanDAOUtils.isSoftDeletedAspect(ebeanMetadataAspect)); - // NOTE how this is should return "true"; see the explanation in the method javadoc ebeanMetadataAspect.setMetadata("{\"gma_deleted\": false}"); - assertTrue(EBeanDAOUtils.isSoftDeletedAspect(ebeanMetadataAspect)); + assertFalse(EBeanDAOUtils.isSoftDeletedAspect(ebeanMetadataAspect)); ebeanMetadataAspect.setMetadata("{\"aspect\": {\"value\": \"bar\"}, \"lastmodifiedby\": \"urn:li:tester\"}"); assertFalse(EBeanDAOUtils.isSoftDeletedAspect(ebeanMetadataAspect));