Skip to content

Commit

Permalink
soft delete relationships when aspects are soft deleted (ASPECT_METAD…
Browse files Browse the repository at this point in the history
…ATA only) (#456)
  • Loading branch information
jsdonn authored Oct 24, 2024
1 parent efa96e0 commit e364ee2
Show file tree
Hide file tree
Showing 19 changed files with 249 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import com.linkedin.testing.SnapshotUnionAliasWithEntitySnapshotAliasOptionalFields;
import com.linkedin.testing.TyperefPizzaAspect;
import com.linkedin.testing.localrelationship.AspectFooBar;
import com.linkedin.testing.localrelationship.AspectFooBarBaz;
import com.linkedin.testing.namingedgecase.InternalEntitySnapshotNamingEdgeCase;
import com.linkedin.testing.urn.PizzaUrn;
import com.linkedin.testing.urn.BarUrn;
Expand Down Expand Up @@ -106,7 +107,7 @@ public void testGetValidAspectTypes() {
Set<Class<? extends RecordTemplate>> validTypes = ModelUtils.getValidAspectTypes(EntityAspectUnion.class);

assertEquals(validTypes,
ImmutableSet.of(AspectFoo.class, AspectBar.class, AspectFooBar.class, AspectAttributes.class));
ImmutableSet.of(AspectFoo.class, AspectBar.class, AspectFooBar.class, AspectFooBarBaz.class, AspectAttributes.class));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -601,21 +601,13 @@ protected <ASPECT extends RecordTemplate> long saveLatest(@Nonnull URN urn, @Non
@Nonnull AuditStamp newAuditStamp, boolean isSoftDeleted, @Nullable IngestionTrackingContext trackingContext,
boolean isTestMode) {
// First, check that if the aspect is going to be soft-deleted that it does not have any relationships derived from it.
// We currently don't support soft-deleting aspects from which local relationships are derived from.
if (newValue == null) {
if (_relationshipSource == RelationshipSource.RELATIONSHIP_BUILDERS
// We currently don't support soft-deleting aspects from which local relationships are derived via relationship builders.
if (newValue == null && _relationshipSource == RelationshipSource.RELATIONSHIP_BUILDERS
&& _localRelationshipBuilderRegistry != null
&& _localRelationshipBuilderRegistry.isRegistered(aspectClass)) {
throw new UnsupportedOperationException(
String.format("Aspect %s cannot be soft-deleted because it has a local relationship builder registered.",
aspectClass.getCanonicalName()));
}

if (_relationshipSource == RelationshipSource.ASPECT_METADATA) {
// TODO: not yet implemented -> add support for removing relationships when the aspect is to be soft-deleted
throw new UnsupportedOperationException("This method has not been implemented yet to support the "
+ "ASPECT_METADATA RelationshipSource type yet.");
}
throw new UnsupportedOperationException(
String.format("Aspect %s cannot be soft-deleted because it has a local relationship builder registered.",
aspectClass.getCanonicalName()));
}

// Save oldValue as the largest version + 1
Expand Down Expand Up @@ -656,8 +648,17 @@ protected <ASPECT extends RecordTemplate> long saveLatest(@Nonnull URN urn, @Non
insert(urn, newValue, aspectClass, newAuditStamp, LATEST_VERSION, trackingContext, isTestMode);
}

// Add any local relationships that are derived from the aspect.
addRelationshipsIfAny(urn, newValue, aspectClass, isTestMode);
// If the aspect is to be soft deleted and we are deriving relationships from aspect metadata, remove any relationships
// associated with the previous aspect value.
if (newValue == null && _relationshipSource == RelationshipSource.ASPECT_METADATA && oldValue != null) {
List<RecordTemplate> relationships = extractRelationshipsFromAspect(oldValue).stream()
.flatMap(List::stream)
.collect(Collectors.toList());
_localRelationshipWriterDAO.removeRelationships(relationships);
// Otherwise, add any local relationships that are derived from the aspect.
} else {
addRelationshipsIfAny(urn, newValue, aspectClass, isTestMode);
}

return largestVersion;
}
Expand Down Expand Up @@ -887,6 +888,9 @@ protected <ASPECT extends RecordTemplate> void insert(@Nonnull URN urn, @Nullabl
*/
public <ASPECT extends RecordTemplate, RELATIONSHIP extends RecordTemplate> List<LocalRelationshipUpdates> addRelationshipsIfAny(
@Nonnull URN urn, @Nullable ASPECT aspect, @Nonnull Class<ASPECT> aspectClass, boolean isTestMode) {
if (aspect == null) {
return Collections.emptyList();
}
List<LocalRelationshipUpdates> localRelationshipUpdates = Collections.emptyList();
if (_relationshipSource == RelationshipSource.ASPECT_METADATA) {
List<List<RELATIONSHIP>> allRelationships = EBeanDAOUtils.extractRelationshipsFromAspect(aspect);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ private static class CommonColumnName {
private static final String LAST_MODIFIED_ON = "lastmodifiedon";
private static final String LAST_MODIFIED_BY = "lastmodifiedby";
private static final String DELETED_TS = "deleted_ts";
private static final String ASPECT = "aspect";
}

public EbeanLocalRelationshipWriterDAO(EbeanServer server) {
Expand Down Expand Up @@ -73,7 +74,7 @@ public void clearRelationshipsByEntity(@Nonnull Urn urn,
return;
}
RelationshipValidator.validateRelationshipSchema(relationshipClass);
SqlUpdate deletionSQL = _server.createSqlUpdate(SQLStatementUtils.deleteLocaRelationshipSQL(
SqlUpdate deletionSQL = _server.createSqlUpdate(SQLStatementUtils.deleteLocalRelationshipSQL(
isTestMode ? SQLSchemaUtils.getTestRelationshipTableName(relationshipClass)
: SQLSchemaUtils.getRelationshipTableName(relationshipClass), removalOption));
if (removalOption == RemovalOption.REMOVE_ALL_EDGES_FROM_SOURCE) {
Expand Down Expand Up @@ -102,7 +103,13 @@ public <RELATIONSHIP extends RecordTemplate> void addRelationships(@Nonnull List

@Override
public <RELATIONSHIP extends RecordTemplate> void removeRelationships(@Nonnull List<RELATIONSHIP> relationships) {
throw new UnsupportedOperationException("Local relationship only supports adding relationships.");
for (RELATIONSHIP relationship : relationships) {
_server.createSqlUpdate(SQLStatementUtils.deleteLocalRelationshipSQL(SQLSchemaUtils.getRelationshipTableName(relationship),
RemovalOption.REMOVE_ALL_EDGES_FROM_SOURCE_TO_DESTINATION))
.setParameter(CommonColumnName.SOURCE, getSourceUrnFromRelationship(relationship).toString())
.setParameter(CommonColumnName.DESTINATION, getDestinationUrnFromRelationship(relationship).toString())
.execute();
}
}

@Override
Expand Down Expand Up @@ -156,7 +163,7 @@ private <RELATIONSHIP extends RecordTemplate> void processRemovalOption(String t
return;
}

SqlUpdate deletionSQL = _server.createSqlUpdate(SQLStatementUtils.deleteLocaRelationshipSQL(tableName, removalOption));
SqlUpdate deletionSQL = _server.createSqlUpdate(SQLStatementUtils.deleteLocalRelationshipSQL(tableName, removalOption));
Urn source = getSourceUrnFromRelationship(relationship);
Urn destination = getDestinationUrnFromRelationship(relationship);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ public static String escapeReservedCharInUrn(String strInSql) {

@Nonnull
@ParametersAreNonnullByDefault
public static String deleteLocaRelationshipSQL(final String tableName, final BaseGraphWriterDAO.RemovalOption removalOption) {
public static String deleteLocalRelationshipSQL(final String tableName, final BaseGraphWriterDAO.RemovalOption removalOption) {
if (removalOption == BaseGraphWriterDAO.RemovalOption.REMOVE_ALL_EDGES_FROM_SOURCE) {
return String.format(DELETE_BY_SOURCE, tableName);
} else if (removalOption == BaseGraphWriterDAO.RemovalOption.REMOVE_ALL_EDGES_FROM_SOURCE_TO_DESTINATION) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,11 @@
import com.linkedin.testing.FooSnapshot;
import com.linkedin.testing.MixedRecord;
import com.linkedin.testing.localrelationship.AspectFooBar;
import com.linkedin.testing.localrelationship.AspectFooBarBaz;
import com.linkedin.testing.localrelationship.BelongsTo;
import com.linkedin.testing.localrelationship.BelongsToArray;
import com.linkedin.testing.localrelationship.ReportsTo;
import com.linkedin.testing.localrelationship.ReportsToArray;
import com.linkedin.testing.urn.BarUrn;
import com.linkedin.testing.urn.BurgerUrn;
import com.linkedin.testing.urn.FooUrn;
Expand Down Expand Up @@ -2536,6 +2539,85 @@ public void testUndeleteSoftDeletedAspect() {
verifyNoMoreInteractions(_mockProducer);
}

@Test
public void testRemoveRelationshipsDuringAspectSoftDeletion() throws URISyntaxException {
EbeanLocalDAO<EntityAspectUnion, FooUrn> fooDao = createDao(FooUrn.class);
EbeanLocalDAO<EntityAspectUnion, BarUrn> barDao = createDao(BarUrn.class);

fooDao.setRelationshipSource(EbeanLocalDAO.RelationshipSource.ASPECT_METADATA);

// add an aspect (AspectFooBar) which includes BelongsTo relationships and ReportsTo relationships
FooUrn fooUrn = makeFooUrn(1);
BarUrn barUrn1 = BarUrn.createFromString("urn:li:bar:1");
BelongsTo belongsTo1 = new BelongsTo().setSource(barUrn1).setDestination(fooUrn);
BarUrn barUrn2 = BarUrn.createFromString("urn:li:bar:2");
BelongsTo belongsTo2 = new BelongsTo().setSource(barUrn2).setDestination(fooUrn);
BarUrn barUrn3 = BarUrn.createFromString("urn:li:bar:3");
BelongsTo belongsTo3 = new BelongsTo().setSource(barUrn3).setDestination(fooUrn);
BelongsToArray belongsToArray = new BelongsToArray(belongsTo1, belongsTo2, belongsTo3);
ReportsTo reportsTo = new ReportsTo().setSource(fooUrn).setDestination(barUrn1);
ReportsToArray reportsToArray = new ReportsToArray(reportsTo);
AspectFooBar aspectFooBar = new AspectFooBar()
.setBars(new BarUrnArray(barUrn1, barUrn2, barUrn3)).setBelongsTos(belongsToArray).setReportsTos(reportsToArray);
AuditStamp auditStamp = makeAuditStamp("foo", System.currentTimeMillis());

fooDao.add(fooUrn, aspectFooBar, auditStamp);
barDao.add(barUrn1, new AspectFoo().setValue("1"), auditStamp);
barDao.add(barUrn2, new AspectFoo().setValue("2"), auditStamp);
barDao.add(barUrn3, new AspectFoo().setValue("3"), auditStamp);

// add another aspect (AspectFooBarBaz) which includes BelongsTo relationship(s)
BarUrn barUrn4 = BarUrn.createFromString("urn:li:bar:4");
BelongsTo belongsTo4 = new BelongsTo().setSource(barUrn4).setDestination(fooUrn);
AspectFooBarBaz aspectFooBarBaz = new AspectFooBarBaz().setBars(new BarUrnArray(barUrn4)).setBelongsTos(new BelongsToArray(belongsTo4));
fooDao.add(fooUrn, aspectFooBarBaz, auditStamp);
barDao.add(barUrn4, new AspectFoo().setValue("4"), auditStamp);

// Verify local relationships and entities are added.
EbeanLocalRelationshipQueryDAO ebeanLocalRelationshipQueryDAO = new EbeanLocalRelationshipQueryDAO(_server);
ebeanLocalRelationshipQueryDAO.setSchemaConfig(_schemaConfig);

List<BelongsTo> resultBelongsTos =
ebeanLocalRelationshipQueryDAO.findRelationships(BarSnapshot.class, EMPTY_FILTER, FooSnapshot.class,
EMPTY_FILTER, BelongsTo.class, OUTGOING_FILTER, 0, 10);

assertEquals(resultBelongsTos.size(), 4);

List<ReportsTo> resultReportsTos =
ebeanLocalRelationshipQueryDAO.findRelationships(FooSnapshot.class, EMPTY_FILTER, BarSnapshot.class,
EMPTY_FILTER, ReportsTo.class, OUTGOING_FILTER, 0, 10);

assertEquals(resultReportsTos.size(), 1);

AspectKey<FooUrn, AspectFooBar> key = new AspectKey<>(AspectFooBar.class, fooUrn, 0L);
List<EbeanMetadataAspect> aspects = fooDao.batchGetHelper(Collections.singletonList(key), 1, 0);

assertEquals(aspects.size(), 1);

// soft delete the AspectFooBar aspect
fooDao.delete(fooUrn, AspectFooBar.class, _dummyAuditStamp);

// check that the belongsTo relationships 1, 2, & 3 were soft deleted
resultBelongsTos = ebeanLocalRelationshipQueryDAO.findRelationships(BarSnapshot.class, EMPTY_FILTER, FooSnapshot.class,
EMPTY_FILTER, BelongsTo.class, OUTGOING_FILTER, 0, 10);

// but ensure that belongsTo4 (from AspectFooBarBaz) was left untouched
assertEquals(resultBelongsTos.size(), 1);
assertEquals(resultBelongsTos.get(0).getSource(), barUrn4);
assertEquals(resultBelongsTos.get(0).getDestination(), fooUrn);

// check that the reportsTo relationship was soft deleted
resultReportsTos =
ebeanLocalRelationshipQueryDAO.findRelationships(FooSnapshot.class, EMPTY_FILTER, BarSnapshot.class,
EMPTY_FILTER, ReportsTo.class, OUTGOING_FILTER, 0, 10);

assertEquals(resultReportsTos.size(), 0);

// check that the AspectFooBar aspect was soft deleted
Optional<AspectWithExtraInfo<AspectFooBar>> optionalAspect = fooDao.getWithExtraInfo(AspectFooBar.class, fooUrn, 0L);
assertFalse(optionalAspect.isPresent());
}

@Test
public void testGetWithExtraInfoMultipleKeys() {
EbeanLocalDAO<EntityAspectUnion, FooUrn> dao = createDao(FooUrn.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ ReportsTo.class, new LocalRelationshipFilter().setCriteria(new LocalRelationship

// Soft (set delete_ts = now()) Delete Jack reports-to ALice relationship
SqlUpdate deletionSQL = _server.createSqlUpdate(
SQLStatementUtils.deleteLocaRelationshipSQL(SQLSchemaUtils.getRelationshipTableName(jackReportsToAlice),
SQLStatementUtils.deleteLocalRelationshipSQL(SQLSchemaUtils.getRelationshipTableName(jackReportsToAlice),
BaseGraphWriterDAO.RemovalOption.REMOVE_ALL_EDGES_FROM_SOURCE));
deletionSQL.setParameter("source", jack.toString());
deletionSQL.execute();
Expand Down Expand Up @@ -368,7 +368,7 @@ ReportsTo.class, new LocalRelationshipFilter().setCriteria(new LocalRelationship

// Soft (set delete_ts = now()) Delete Jack reports-to ALice relationship
SqlUpdate deletionSQL = _server.createSqlUpdate(
SQLStatementUtils.deleteLocaRelationshipSQL(SQLSchemaUtils.getRelationshipTableName(jackReportsToAlice),
SQLStatementUtils.deleteLocalRelationshipSQL(SQLSchemaUtils.getRelationshipTableName(jackReportsToAlice),
BaseGraphWriterDAO.RemovalOption.REMOVE_ALL_EDGES_FROM_SOURCE));
deletionSQL.setParameter("source", jack.toString());
deletionSQL.execute();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.io.IOException;
import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.List;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
Expand Down Expand Up @@ -258,6 +259,31 @@ public void testClearRelationshipsByEntityUrn() throws URISyntaxException {
_server.execute(Ebean.createSqlUpdate("truncate metadata_relationship_pairswith"));
}

@Test
public void testRemoveRelationships() throws URISyntaxException {
BarUrn barUrn = BarUrn.createFromString("urn:li:bar:123");
FooUrn fooUrn123 = FooUrn.createFromString("urn:li:foo:123");
FooUrn fooUrn456 = FooUrn.createFromString("urn:li:foo:456");
_server.execute(Ebean.createSqlUpdate(insertRelationships("metadata_relationship_pairswith", barUrn.toString(),
"bar", fooUrn123.toString(), "foo")));

_server.execute(Ebean.createSqlUpdate(insertRelationships("metadata_relationship_pairswith", barUrn.toString(),
"bar", fooUrn456.toString(), "foo")));

// Before processing
List<SqlRow> before = _server.createSqlQuery("select * from metadata_relationship_pairswith where deleted_ts is null").findList();
assertEquals(before.size(), 2);

PairsWith pairsWith = new PairsWith().setSource(barUrn).setDestination(fooUrn123);
_localRelationshipWriterDAO.removeRelationships(Collections.singletonList(pairsWith));

// After processing verification
List<SqlRow> all = _server.createSqlQuery("select * from metadata_relationship_pairswith where deleted_ts is null").findList();
assertEquals(all.size(), 1); // Total number of edges is 1
assertEquals(all.get(0).getString("source"), barUrn.toString());
assertEquals(all.get(0).getString("destination"), fooUrn456.toString());
}

private String insertRelationships(String table, String sourceUrn, String sourceType, String destinationUrn, String destinationType) {
String insertTemplate = "INSERT INTO %s (metadata, source, source_type, destination, destination_type, lastmodifiedon, lastmodifiedby)"
+ " VALUES ('{\"metadata\": true}', '%s', '%s', '%s', '%s', '1970-01-01 00:00:01', 'unknown')";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,30 @@ CREATE TABLE IF NOT EXISTS metadata_entity_burger (
);

CREATE TABLE IF NOT EXISTS metadata_relationship_belongsto (
id BIGINT NOT NULL AUTO_INCREMENT,
metadata LONGTEXT NOT NULL,
source VARCHAR(1000) NOT NULL,
id BIGINT NOT NULL AUTO_INCREMENT,
metadata LONGTEXT NOT NULL,
source VARCHAR(1000) NOT NULL,
source_type VARCHAR(100) NOT NULL,
destination VARCHAR(1000) NOT NULL,
destination_type VARCHAR(100) NOT NULL,
lastmodifiedon TIMESTAMP NOT NULL,
lastmodifiedby VARCHAR(255) NOT NULL,
deleted_ts DATETIME(6) DEFAULT NULL,
PRIMARY KEY (id)
);
);

CREATE TABLE IF NOT EXISTS metadata_relationship_reportsto (
id BIGINT NOT NULL AUTO_INCREMENT,
metadata LONGTEXT NOT NULL,
source VARCHAR(1000) NOT NULL,
source_type VARCHAR(100) NOT NULL,
destination VARCHAR(1000) NOT NULL,
destination_type VARCHAR(100) NOT NULL,
lastmodifiedon TIMESTAMP NOT NULL,
lastmodifiedby VARCHAR(255) NOT NULL,
deleted_ts DATETIME(6) DEFAULT NULL,
PRIMARY KEY (id)
);

CREATE TABLE metadata_id (
namespace VARCHAR(255) NOT NULL,
Expand Down Expand Up @@ -92,6 +105,9 @@ ALTER TABLE metadata_entity_foo_test ADD a_aspectbar JSON;
-- add foobar aspect to foo entity
ALTER TABLE metadata_entity_foo ADD a_aspectfoobar JSON;

-- add foobar aspect to foo entity
ALTER TABLE metadata_entity_foo ADD a_aspectfoobarbaz JSON;

-- add array aspect to foo entity
ALTER TABLE metadata_entity_foo ADD a_aspectattributes JSON;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,19 @@ CREATE TABLE IF NOT EXISTS metadata_relationship_belongsto (
PRIMARY KEY (id)
);

CREATE TABLE IF NOT EXISTS metadata_relationship_reportsto (
id BIGINT NOT NULL AUTO_INCREMENT,
metadata LONGTEXT NOT NULL,
source VARCHAR(1000) NOT NULL,
source_type VARCHAR(100) NOT NULL,
destination VARCHAR(1000) NOT NULL,
destination_type VARCHAR(100) NOT NULL,
lastmodifiedon TIMESTAMP NOT NULL,
lastmodifiedby VARCHAR(255) NOT NULL,
deleted_ts DATETIME(6) DEFAULT NULL,
PRIMARY KEY (id)
);

CREATE TABLE metadata_id (
namespace VARCHAR(255) NOT NULL,
id BIGINT NOT NULL,
Expand Down Expand Up @@ -92,6 +105,9 @@ ALTER TABLE metadata_entity_foo_test ADD a_aspectbar JSON;
-- add foobar aspect to foo entity
ALTER TABLE metadata_entity_foo ADD a_aspectfoobar JSON;

-- add foobar aspect to foo entity
ALTER TABLE metadata_entity_foo ADD a_aspectfoobarbaz JSON;

-- add array aspect to foo entity
ALTER TABLE metadata_entity_foo ADD a_aspectattributes JSON;

Expand Down
13 changes: 13 additions & 0 deletions dao-impl/ebean-dao/src/test/resources/gma-create-all.sql
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,16 @@ CREATE TABLE IF NOT EXISTS metadata_relationship_belongsto (
deleted_ts DATETIME(6) DEFAULT NULL,
PRIMARY KEY (id)
);

CREATE TABLE IF NOT EXISTS metadata_relationship_reportsto (
id BIGINT NOT NULL AUTO_INCREMENT,
metadata JSON NOT NULL,
source VARCHAR(500) NOT NULL,
source_type VARCHAR(100) NOT NULL,
destination VARCHAR(500) NOT NULL,
destination_type VARCHAR(100) NOT NULL,
lastmodifiedon DATETIME(6) NOT NULL,
lastmodifiedby VARCHAR(255) NOT NULL,
deleted_ts DATETIME(6) DEFAULT NULL,
PRIMARY KEY (id)
);
2 changes: 2 additions & 0 deletions dao-impl/ebean-dao/src/test/resources/gma-drop-all.sql
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,5 @@ drop table if exists metadata_aspect;
drop table if exists metadata_index;

drop table if exists metadata_relationship_belongsto;

drop table if exists metadata_relationship_reportsto;
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,14 @@ namespace com.linkedin.testing
/**
* For unit tests
*/
@gma.aspect.column.name = "annotatedaspectbarwithrelationshipfields"
@gma.model = "ASPECT"
@gma = {
"aspect": {
"column": {
"name": "annotatedaspectbarwithrelationshipfields"
}
},
"model": "ASPECT"
}
record AnnotatedAspectBarWithRelationshipFields {
/**
* For unit tests
Expand Down
Loading

0 comments on commit e364ee2

Please sign in to comment.