From 22e2a55367320a5082649308641b18fdb7c61b53 Mon Sep 17 00:00:00 2001 From: John Plaisted Date: Tue, 3 Nov 2020 15:38:38 -0800 Subject: [PATCH 1/2] fix: enable werror for ebean dao. --- build.gradle | 5 +- .../producer/DummyMetadataEventProducer.java | 8 +- .../DummyMetadataEventProducerTest.java | 23 --- dao-impl/ebean-dao/build.gradle | 1 - .../linkedin/metadata/dao/EbeanLocalDAO.java | 90 ++++++------ .../metadata/dao/ImmutableLocalDAO.java | 39 +++-- .../metadata/dao/EbeanLocalDAOTest.java | 138 ++++++++++++++---- .../metadata/dao/ImmutableLocalDAOTest.java | 15 +- .../dao/utils/BarUrnPathExtractor.java | 2 +- .../dao/utils/BazUrnPathExtractor.java | 2 +- .../dao/utils/FooUrnPathExtractor.java | 2 +- 11 files changed, 204 insertions(+), 121 deletions(-) delete mode 100644 dao-api/src/test/java/com/linkedin/metadata/dao/producer/DummyMetadataEventProducerTest.java diff --git a/build.gradle b/build.gradle index 475e57b36..366e6551b 100644 --- a/build.gradle +++ b/build.gradle @@ -46,10 +46,13 @@ project.ext.externalDependency = [ 'jacksonCore': 'com.fasterxml.jackson.core:jackson-core:2.9.7', 'jacksonDataBind': 'com.fasterxml.jackson.core:jackson-databind:2.9.7', 'javatuples': 'org.javatuples:javatuples:1.2', +<<<<<<< HEAD 'jsonSimple': 'com.googlecode.json-simple:json-simple:1.1.1', 'junitJupiterApi': "org.junit.jupiter:junit-jupiter-api:$junitJupiterVersion", 'junitJupiterParams': "org.junit.jupiter:junit-jupiter-params:$junitJupiterVersion", 'junitJupiterEngine': "org.junit.jupiter:junit-jupiter-engine:$junitJupiterVersion", +======= +>>>>>>> 82e47552 (fix: enable werror for ebean dao.) 'lombok': 'org.projectlombok:lombok:1.18.12', 'mockito': 'org.mockito:mockito-core:3.0.0', 'mockitoInline': 'org.mockito:mockito-inline:3.0.0', @@ -71,7 +74,7 @@ apply from: "./gradle/release.gradle" def wErrorProjects = [ project(':core-models'), project(':dao-api'), - // project(':dao-impl:ebean-dao'), + project(':dao-impl:ebean-dao'), // project(':dao-impl:elasticsearch-dao'), // project(':dao-impl:neo4j-dao'), project(':restli-resources'), diff --git a/dao-api/src/main/java/com/linkedin/metadata/dao/producer/DummyMetadataEventProducer.java b/dao-api/src/main/java/com/linkedin/metadata/dao/producer/DummyMetadataEventProducer.java index 1904147c3..1a0005e1a 100644 --- a/dao-api/src/main/java/com/linkedin/metadata/dao/producer/DummyMetadataEventProducer.java +++ b/dao-api/src/main/java/com/linkedin/metadata/dao/producer/DummyMetadataEventProducer.java @@ -2,6 +2,7 @@ import com.linkedin.common.urn.Urn; import com.linkedin.data.template.RecordTemplate; +import com.linkedin.data.template.UnionTemplate; import com.linkedin.metadata.dummy.DummyAspect; import com.linkedin.metadata.dummy.DummySnapshot; import javax.annotation.Nonnull; @@ -11,11 +12,12 @@ /** * A dummy metadata event producer that doesn't actually produce any events. */ -public class DummyMetadataEventProducer - extends BaseMetadataEventProducer { +public class DummyMetadataEventProducer + extends BaseMetadataEventProducer { + @SuppressWarnings("unchecked") public DummyMetadataEventProducer() { - super(DummySnapshot.class, DummyAspect.class); + super((Class) DummySnapshot.class, (Class) DummyAspect.class); } @Override diff --git a/dao-api/src/test/java/com/linkedin/metadata/dao/producer/DummyMetadataEventProducerTest.java b/dao-api/src/test/java/com/linkedin/metadata/dao/producer/DummyMetadataEventProducerTest.java deleted file mode 100644 index 88ea91cb2..000000000 --- a/dao-api/src/test/java/com/linkedin/metadata/dao/producer/DummyMetadataEventProducerTest.java +++ /dev/null @@ -1,23 +0,0 @@ -package com.linkedin.metadata.dao.producer; - -import com.linkedin.common.urn.Urn; -import com.linkedin.testing.AspectFoo; -import org.testng.annotations.Test; - -import static com.linkedin.testing.TestUtils.*; - - -public class DummyMetadataEventProducerTest { - - // ensure the producer can be created since it has default snapshot and aspect validation - @Test - public void testCreateDummyMetadataEventProducer() { - DummyMetadataEventProducer producer = new DummyMetadataEventProducer<>(); - Urn urn = makeUrn(1); - AspectFoo oldValue = new AspectFoo().setValue("old"); - AspectFoo newValue = new AspectFoo().setValue("new"); - - producer.produceMetadataAuditEvent(urn, oldValue, newValue); - producer.produceAspectSpecificMetadataAuditEvent(urn, oldValue, newValue); - } -} diff --git a/dao-impl/ebean-dao/build.gradle b/dao-impl/ebean-dao/build.gradle index 4f3526d14..276479267 100644 --- a/dao-impl/ebean-dao/build.gradle +++ b/dao-impl/ebean-dao/build.gradle @@ -10,7 +10,6 @@ dependencies { compile project(':core-models-utils') compile project(':dao-api') compile externalDependency.ebean - compile externalDependency.jsonSimple compile externalDependency.guava compileOnly externalDependency.ebeanAgent 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 84519df73..ee7cf0f92 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 @@ -89,8 +89,9 @@ static class GMAIndexPair { }); @VisibleForTesting - EbeanLocalDAO(@Nonnull Class aspectUnionClass, @Nonnull BaseMetadataEventProducer producer, - @Nonnull EbeanServer server, @Nonnull Class urnClass) { + EbeanLocalDAO(@Nonnull Class aspectUnionClass, + @Nonnull BaseMetadataEventProducer producer, @Nonnull EbeanServer server, + @Nonnull Class urnClass) { super(aspectUnionClass, producer); _server = server; _urnClass = urnClass; @@ -105,13 +106,14 @@ static class GMAIndexPair { * @param serverConfig {@link ServerConfig} that defines the configuration of EbeanServer instances * @param urnClass Class of the entity URN */ - public EbeanLocalDAO(@Nonnull Class aspectUnionClass, @Nonnull BaseMetadataEventProducer producer, - @Nonnull ServerConfig serverConfig, @Nonnull Class urnClass) { + public EbeanLocalDAO(@Nonnull Class aspectUnionClass, + @Nonnull BaseMetadataEventProducer producer, @Nonnull ServerConfig serverConfig, + @Nonnull Class urnClass) { this(aspectUnionClass, producer, createServer(serverConfig), urnClass); } @VisibleForTesting - EbeanLocalDAO(@Nonnull BaseMetadataEventProducer producer, @Nonnull EbeanServer server, + EbeanLocalDAO(@Nonnull BaseMetadataEventProducer producer, @Nonnull EbeanServer server, @Nonnull LocalDAOStorageConfig storageConfig, @Nonnull Class urnClass, @Nonnull UrnPathExtractor urnPathExtractor) { super(producer, storageConfig); @@ -121,7 +123,7 @@ public EbeanLocalDAO(@Nonnull Class aspectUnionClass, @Nonnull Bas } @VisibleForTesting - EbeanLocalDAO(@Nonnull BaseMetadataEventProducer producer, @Nonnull EbeanServer server, + EbeanLocalDAO(@Nonnull BaseMetadataEventProducer producer, @Nonnull EbeanServer server, @Nonnull LocalDAOStorageConfig storageConfig, @Nonnull Class urnClass) { this(producer, server, storageConfig, urnClass, new EmptyPathExtractor<>()); } @@ -135,8 +137,8 @@ public EbeanLocalDAO(@Nonnull Class aspectUnionClass, @Nonnull Bas * @param urnClass class of the entity URN * @param urnPathExtractor path extractor to index parts of URNs to the secondary index */ - public EbeanLocalDAO(@Nonnull BaseMetadataEventProducer producer, @Nonnull ServerConfig serverConfig, - @Nonnull LocalDAOStorageConfig storageConfig, @Nonnull Class urnClass, + public EbeanLocalDAO(@Nonnull BaseMetadataEventProducer producer, + @Nonnull ServerConfig serverConfig, @Nonnull LocalDAOStorageConfig storageConfig, @Nonnull Class urnClass, @Nonnull UrnPathExtractor urnPathExtractor) { this(producer, createServer(serverConfig), storageConfig, urnClass, urnPathExtractor); } @@ -149,8 +151,8 @@ public EbeanLocalDAO(@Nonnull BaseMetadataEventProducer producer, @Nonnull Serve * @param storageConfig {@link LocalDAOStorageConfig} containing storage config of full list of supported aspects * @param urnClass class of the entity URN */ - public EbeanLocalDAO(@Nonnull BaseMetadataEventProducer producer, @Nonnull ServerConfig serverConfig, - @Nonnull LocalDAOStorageConfig storageConfig, @Nonnull Class urnClass) { + public EbeanLocalDAO(@Nonnull BaseMetadataEventProducer producer, + @Nonnull ServerConfig serverConfig, @Nonnull LocalDAOStorageConfig storageConfig, @Nonnull Class urnClass) { this(producer, createServer(serverConfig), storageConfig, urnClass, new EmptyPathExtractor<>()); } @@ -245,7 +247,8 @@ protected T runInTransactionWithRetry(@Nonnull Supplier block, int maxTra @Override protected long saveLatest(@Nonnull URN urn, @Nonnull Class aspectClass, - @Nullable ASPECT oldValue, @Nullable AuditStamp oldAuditStamp, @Nonnull ASPECT newValue, @Nonnull AuditStamp newAuditStamp) { + @Nullable ASPECT oldValue, @Nullable AuditStamp oldAuditStamp, @Nonnull ASPECT newValue, + @Nonnull AuditStamp newAuditStamp) { // Save oldValue as the largest version + 1 long largestVersion = 0; if (oldValue != null && oldAuditStamp != null) { @@ -259,8 +262,8 @@ protected long saveLatest(@Nonnull URN urn, @Non } @Override - protected void updateLocalIndex(@Nonnull URN urn, - @Nonnull ASPECT newValue, long version) { + protected void updateLocalIndex(@Nonnull URN urn, @Nonnull ASPECT newValue, + long version) { if (!isLocalSecondaryIndexEnabled()) { throw new UnsupportedOperationException("Local secondary index isn't supported"); } @@ -310,13 +313,10 @@ protected void save(@Nonnull URN urn, @Nonnull RecordTemplate value, @Nonnull Au } } - protected long saveSingleRecordToLocalIndex(@Nonnull URN urn, @Nonnull String aspect, - @Nonnull String path, @Nonnull Object value) { + protected long saveSingleRecordToLocalIndex(@Nonnull URN urn, @Nonnull String aspect, @Nonnull String path, + @Nonnull Object value) { - final EbeanMetadataIndex record = new EbeanMetadataIndex() - .setUrn(urn.toString()) - .setAspect(aspect) - .setPath(path); + final EbeanMetadataIndex record = new EbeanMetadataIndex().setUrn(urn.toString()).setAspect(aspect).setPath(path); if (value instanceof Integer || value instanceof Long) { record.setLongVal(Long.valueOf(value.toString())); } else if (value instanceof Float || value instanceof Double) { @@ -341,15 +341,13 @@ private void updateUrnInLocalIndex(@Nonnull URN urn) { final Map pathValueMap = _urnPathExtractor.extractPaths(urn); pathValueMap.forEach( - (path, value) -> saveSingleRecordToLocalIndex(urn, urn.getClass().getCanonicalName(), path, value) - ); + (path, value) -> saveSingleRecordToLocalIndex(urn, urn.getClass().getCanonicalName(), path, value)); } private void updateAspectInLocalIndex(@Nonnull URN urn, @Nonnull ASPECT newValue) { if (!_storageConfig.getAspectStorageConfigMap().containsKey(newValue.getClass()) - || _storageConfig.getAspectStorageConfigMap().get(newValue.getClass()) == null - ) { + || _storageConfig.getAspectStorageConfigMap().get(newValue.getClass()) == null) { return; } // step1: remove all rows from the index table corresponding to pair @@ -453,11 +451,8 @@ protected void applyTimeBasedRetention(@Nonnull return result; } - public boolean existsInLocalIndex(@Nonnull URN urn) { - return _server.find(EbeanMetadataIndex.class) - .where().eq(URN_COLUMN, urn.toString()) - .exists(); + return _server.find(EbeanMetadataIndex.class).where().eq(URN_COLUMN, urn.toString()).exists(); } // Will be migrated to use {@link #batchGet(Set>, int)} @@ -498,7 +493,8 @@ public void setQueryKeysCount(int keysCount) { * @param keysCount the max number of keys for each sub query */ @Nonnull - private List batchGet(@Nonnull Set> keys, int keysCount) { + private List batchGet(@Nonnull Set> keys, + int keysCount) { int position = 0; final int totalPageCount = QueryUtils.getTotalPageCount(keys.size(), keysCount); @@ -513,7 +509,8 @@ private List batchGet(@Nonnull Set batchGetHelper(@Nonnull List> keys, int keysCount, int position) { + private List batchGetHelper(@Nonnull List> keys, + int keysCount, int position) { ExpressionList query = _server.find(EbeanMetadataAspect.class).select(ALL_COLUMNS).where(); // add or if it is not the last element @@ -583,7 +580,8 @@ public ListResult listUrns(@Nonnull Class urns = pagedList.getList().stream().map(entry -> getUrn(entry.getKey().getUrn())).collect(Collectors.toList()); + final List urns = + pagedList.getList().stream().map(entry -> getUrn(entry.getKey().getUrn())).collect(Collectors.toList()); return toListResult(urns, null, pagedList, start); } @@ -607,8 +605,8 @@ public ListResult list(@Nonnull Class aspects = pagedList.getList().stream().map(a -> toRecordTemplate(aspectClass, a)).collect(Collectors.toList()); - final ListResultMetadata listResultMetadata = - makeListResultMetadata(pagedList.getList().stream().map(EbeanLocalDAO::toExtraInfo).collect(Collectors.toList())); + final ListResultMetadata listResultMetadata = makeListResultMetadata( + pagedList.getList().stream().map(EbeanLocalDAO::toExtraInfo).collect(Collectors.toList())); return toListResult(aspects, listResultMetadata, pagedList, start); } @@ -632,8 +630,8 @@ public ListResult list(@Nonnull Class aspects = pagedList.getList().stream().map(a -> toRecordTemplate(aspectClass, a)).collect(Collectors.toList()); - final ListResultMetadata listResultMetadata = - makeListResultMetadata(pagedList.getList().stream().map(EbeanLocalDAO::toExtraInfo).collect(Collectors.toList())); + final ListResultMetadata listResultMetadata = makeListResultMetadata( + pagedList.getList().stream().map(EbeanLocalDAO::toExtraInfo).collect(Collectors.toList())); return toListResult(aspects, listResultMetadata, pagedList, start); } @@ -670,7 +668,8 @@ private static AspectWithExtraInfo toRec @Nonnull private ListResult toListResult(@Nonnull List values, @Nullable ListResultMetadata listResultMetadata, @Nonnull PagedList pagedList, @Nullable Integer start) { - final int nextStart = (start != null && pagedList.hasNext()) ? start.intValue() + pagedList.getList().size() : ListResult.INVALID_NEXT_START; + final int nextStart = (start != null && pagedList.hasNext()) ? start.intValue() + pagedList.getList().size() + : ListResult.INVALID_NEXT_START; return ListResult.builder() // Format .values(values) @@ -744,7 +743,7 @@ static GMAIndexPair getGMAIndexPair(@Nonnull IndexValue indexValue) { if (indexValue.isBoolean()) { object = indexValue.getBoolean().toString(); return new GMAIndexPair(EbeanMetadataIndex.STRING_COLUMN, object); - } else if (indexValue.isDouble()) { + } else if (indexValue.isDouble()) { object = indexValue.getDouble(); return new GMAIndexPair(EbeanMetadataIndex.DOUBLE_COLUMN, object); } else if (indexValue.isFloat()) { @@ -774,8 +773,8 @@ static GMAIndexPair getGMAIndexPair(@Nonnull IndexValue indexValue) { * @param lastUrn string representation of the urn whose value is used to set the last urn parameter in index query * @param pageSize maximum number of distinct urns to return which is essentially the LIMIT clause of SQL query */ - private static void setParameters(@Nonnull IndexCriterionArray indexCriterionArray, @Nonnull Query indexQuery, - @Nonnull String lastUrn, int pageSize) { + private static void setParameters(@Nonnull IndexCriterionArray indexCriterionArray, + @Nonnull Query indexQuery, @Nonnull String lastUrn, int pageSize) { indexQuery.setParameter(1, lastUrn); int pos = 2; for (IndexCriterion criterion : indexCriterionArray) { @@ -791,7 +790,8 @@ private static void setParameters(@Nonnull IndexCriterionArray indexCriterionArr @Nonnull private static String getStringForOperator(@Nonnull Condition condition) { if (!CONDITION_STRING_MAP.containsKey(condition)) { - throw new UnsupportedOperationException(condition.toString() + " condition is not supported in local secondary index"); + throw new UnsupportedOperationException( + condition.toString() + " condition is not supported in local secondary index"); } return CONDITION_STRING_MAP.get(condition); } @@ -806,8 +806,9 @@ private static String getStringForOperator(@Nonnull Condition condition) { @Nonnull private static String constructSQLQuery(@Nonnull IndexCriterionArray indexCriterionArray) { String selectClause = "SELECT DISTINCT(t0.urn) FROM metadata_index t0"; - selectClause += IntStream.range(1, indexCriterionArray.size()).mapToObj(i -> " INNER JOIN metadata_index " + "t" - + i + " ON t0.urn = " + "t" + i + ".urn").collect(Collectors.joining("")); + selectClause += IntStream.range(1, indexCriterionArray.size()) + .mapToObj(i -> " INNER JOIN metadata_index " + "t" + i + " ON t0.urn = " + "t" + i + ".urn") + .collect(Collectors.joining("")); final StringBuilder whereClause = new StringBuilder("WHERE t0.urn > ?"); IntStream.range(0, indexCriterionArray.size()).forEach(i -> { final IndexCriterion criterion = indexCriterionArray.get(i); @@ -859,7 +860,8 @@ public List listUrns(@Nonnull IndexFilter indexFilter, @Nullable URN lastUr throw new UnsupportedOperationException("Empty Index Filter is not supported by EbeanLocalDAO"); } if (indexCriterionArray.size() > 10) { - throw new UnsupportedOperationException("Currently more than 10 filter conditions is not supported by EbeanLocalDAO"); + throw new UnsupportedOperationException( + "Currently more than 10 filter conditions is not supported by EbeanLocalDAO"); } addEntityTypeFilter(indexFilter); @@ -871,8 +873,6 @@ public List listUrns(@Nonnull IndexFilter indexFilter, @Nullable URN lastUr final List pagedList = query.findList(); - return pagedList.stream() - .map(entry -> getUrn(entry.getUrn())) - .collect(Collectors.toList()); + return pagedList.stream().map(entry -> getUrn(entry.getUrn())).collect(Collectors.toList()); } } diff --git a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/ImmutableLocalDAO.java b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/ImmutableLocalDAO.java index d07c2f23c..75172cfe3 100644 --- a/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/ImmutableLocalDAO.java +++ b/dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/ImmutableLocalDAO.java @@ -5,6 +5,8 @@ import com.linkedin.common.AuditStamp; import com.linkedin.common.urn.Urn; import com.linkedin.common.urn.Urns; +import com.linkedin.data.DataMap; +import com.linkedin.data.codec.JacksonDataCodec; import com.linkedin.data.template.RecordTemplate; import com.linkedin.data.template.UnionTemplate; import com.linkedin.metadata.dao.producer.DummyMetadataEventProducer; @@ -18,20 +20,17 @@ import java.util.HashMap; import java.util.Map; import java.util.Optional; -import java.util.Set; import java.util.function.Function; import javax.annotation.Nonnull; -import org.json.simple.JSONObject; -import org.json.simple.parser.JSONParser; -import org.json.simple.parser.ParseException; /** * An immutable implementation of {@link BaseLocalDAO}. Suitable for serving statically declared metadata. */ -public class ImmutableLocalDAO extends EbeanLocalDAO { +public class ImmutableLocalDAO + extends EbeanLocalDAO { - private static final JSONParser JSON_PARSER = new JSONParser(); + private static final JacksonDataCodec JSON_CODEC = new JacksonDataCodec(); private static final AuditStamp DUMMY_AUDIT_STAMP = new AuditStamp().setActor(Urns.createFromTypeSpecificString("dummy", "unknown")).setTime(0L); @@ -44,7 +43,7 @@ public class ImmutableLocalDAO aspectUnionClass, @Nonnull Map urnAspectMap, @Nonnull Class urnClass) { - super(aspectUnionClass, new DummyMetadataEventProducer(), + super(aspectUnionClass, new DummyMetadataEventProducer<>(), createProductionH2ServerConfig(aspectUnionClass.getCanonicalName()), urnClass); _server.execute(Ebean.createSqlUpdate(readSQLfromFile(GMA_CREATE_ALL_SQL))); urnAspectMap.forEach((key, value) -> super.save(key, value, DUMMY_AUDIT_STAMP, LATEST_VERSION, true)); @@ -54,7 +53,7 @@ public ImmutableLocalDAO(@Nonnull Class aspectUnionClass, public ImmutableLocalDAO(@Nonnull Class aspectUnionClass, @Nonnull Map urnAspectMap, boolean ddlGenerate, @Nonnull Class urnClass) { - super(aspectUnionClass, new DummyMetadataEventProducer(), createTestingH2ServerConfig(), urnClass); + super(aspectUnionClass, new DummyMetadataEventProducer<>(), createTestingH2ServerConfig(), urnClass); urnAspectMap.forEach((key, value) -> super.save(key, value, DUMMY_AUDIT_STAMP, LATEST_VERSION, true)); } @@ -65,16 +64,24 @@ public ImmutableLocalDAO(@Nonnull Class aspectUnionClass, * specific type of metadata aspect. */ @Nonnull - public static Map loadAspects( - @Nonnull Class aspectClass, @Nonnull InputStream inputStream) - throws IOException, ParseException, URISyntaxException { + public static Map loadAspects(@Nonnull Class aspectClass, + @Nonnull InputStream inputStream) throws IOException, URISyntaxException { - final Map aspects = new HashMap<>(); + final Map aspects = new HashMap<>(); try (InputStreamReader reader = new InputStreamReader(inputStream, Charset.defaultCharset())) { - JSONObject root = (JSONObject) JSON_PARSER.parse(reader); - for (Map.Entry entry : (Set) root.entrySet()) { - URN urn = (URN) Urn.createFromString((String) entry.getKey()); - ASPECT aspect = RecordUtils.toRecordTemplate(aspectClass, entry.getValue().toString()); + final DataMap dataMap = JSON_CODEC.readMap(reader); + for (Map.Entry entry : dataMap.entrySet()) { + final Urn urn = Urn.createFromString(entry.getKey()); + final Object value = entry.getValue(); + + if (!DataMap.class.isAssignableFrom(value.getClass())) { + throw new IllegalArgumentException( + String.format("Failed to parse value for urn `%s`. Expected `DataMap` but got `%s`.", entry.getKey(), + value.getClass())); + } + + final DataMap dataMapValue = (DataMap) value; + final ASPECT aspect = RecordUtils.toRecordTemplate(aspectClass, dataMapValue); aspects.put(urn, aspect); } } 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 5704c57c6..7c1675ce3 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 @@ -13,6 +13,7 @@ import com.linkedin.metadata.dao.exception.InvalidMetadataType; import com.linkedin.metadata.dao.exception.RetryLimitReached; import com.linkedin.metadata.dao.producer.BaseMetadataEventProducer; +import com.linkedin.metadata.dao.producer.DummyMetadataEventProducer; import com.linkedin.metadata.dao.retention.TimeBasedRetention; import com.linkedin.metadata.dao.retention.VersionBasedRetention; import com.linkedin.metadata.dao.storage.LocalDAOStorageConfig; @@ -69,10 +70,11 @@ public class EbeanLocalDAOTest { private EbeanServer _server; - private BaseMetadataEventProducer _mockProducer; + private BaseMetadataEventProducer _mockProducer; private AuditStamp _dummyAuditStamp; @BeforeMethod + @SuppressWarnings("unchecked") public void setupTest() { _server = EbeanServerFactory.create(EbeanLocalDAO.createTestingH2ServerConfig()); _mockProducer = mock(BaseMetadataEventProducer.class); @@ -291,7 +293,7 @@ public void testGetNonExisting() { @Test public void testGetCapsSensitivity() { final EbeanLocalDAO dao = - new EbeanLocalDAO<>(EntityAspectUnion.class, _mockProducer, _server, Urn.class); + new EbeanLocalDAO<>(EntityAspectUnion.class, new DummyMetadataEventProducer<>(), _server, Urn.class); final Urn urnCaps = makeUrn("Dataset"); final Urn urnLower = makeUrn("dataset"); @@ -439,7 +441,7 @@ public void testLocalSecondaryIndexBackfill() { } @Test - public void testBackfillWithUrns() { + public void testBackfillSingleAspectWithSingleUrn() { EbeanLocalDAO dao = new EbeanLocalDAO<>(EntityAspectUnion.class, _mockProducer, _server, FooUrn.class); List urns = ImmutableList.of(makeFooUrn(1), makeFooUrn(2), makeFooUrn(3)); @@ -457,26 +459,57 @@ public void testBackfillWithUrns() { // Backfill single aspect for set of urns Map, Optional>> backfilledAspects = dao.backfill(Collections.singleton(AspectFoo.class), new HashSet<>(urns)); - for (Urn urn : urns) { + for (FooUrn urn : urns) { RecordTemplate aspect = aspects.get(urn).get(AspectFoo.class); assertEquals(backfilledAspects.get(urn).get(AspectFoo.class).get(), aspect); verify(_mockProducer, times(1)).produceMetadataAuditEvent(urn, aspect, aspect); } - clearInvocations(_mockProducer); + } + + public void testBackfillMultipleAspectsWithSingleUrn() { + EbeanLocalDAO dao = + new EbeanLocalDAO<>(EntityAspectUnion.class, _mockProducer, _server, FooUrn.class); + List urns = ImmutableList.of(makeFooUrn(1), makeFooUrn(2), makeFooUrn(3)); + + Map, RecordTemplate>> aspects = new HashMap<>(); + + urns.forEach(urn -> { + AspectFoo aspectFoo = new AspectFoo().setValue("foo"); + AspectBar aspectBar = new AspectBar().setValue("bar"); + aspects.put(urn, ImmutableMap.of(AspectFoo.class, aspectFoo, AspectBar.class, aspectBar)); + addMetadata(urn, AspectFoo.class.getCanonicalName(), 0, aspectFoo); + addMetadata(urn, AspectBar.class.getCanonicalName(), 0, aspectBar); + }); // Backfill set of aspects for a single urn - backfilledAspects = + Map, Optional>> backfilledAspects = dao.backfill(ImmutableSet.of(AspectFoo.class, AspectBar.class), Collections.singleton(urns.get(0))); for (Class clazz : aspects.get(urns.get(0)).keySet()) { RecordTemplate aspect = aspects.get(urns.get(0)).get(clazz); assertEquals(backfilledAspects.get(urns.get(0)).get(clazz).get(), aspect); verify(_mockProducer, times(1)).produceMetadataAuditEvent(urns.get(0), aspect, aspect); } - clearInvocations(_mockProducer); + } + + public void testBackfillMultipleAspectsForMultipleUrns() { + EbeanLocalDAO dao = + new EbeanLocalDAO<>(EntityAspectUnion.class, _mockProducer, _server, FooUrn.class); + List urns = ImmutableList.of(makeFooUrn(1), makeFooUrn(2), makeFooUrn(3)); + + Map, RecordTemplate>> aspects = new HashMap<>(); + + urns.forEach(urn -> { + AspectFoo aspectFoo = new AspectFoo().setValue("foo"); + AspectBar aspectBar = new AspectBar().setValue("bar"); + aspects.put(urn, ImmutableMap.of(AspectFoo.class, aspectFoo, AspectBar.class, aspectBar)); + addMetadata(urn, AspectFoo.class.getCanonicalName(), 0, aspectFoo); + addMetadata(urn, AspectBar.class.getCanonicalName(), 0, aspectBar); + }); // Backfill set of aspects for set of urns - backfilledAspects = dao.backfill(ImmutableSet.of(AspectFoo.class, AspectBar.class), new HashSet<>(urns)); - for (Urn urn : urns) { + Map, Optional>> backfilledAspects = + dao.backfill(ImmutableSet.of(AspectFoo.class, AspectBar.class), new HashSet<>(urns)); + for (FooUrn urn : urns) { for (Class clazz : aspects.get(urn).keySet()) { RecordTemplate aspect = aspects.get(urn).get(clazz); assertEquals(backfilledAspects.get(urn).get(clazz).get(), aspect); @@ -487,7 +520,7 @@ public void testBackfillWithUrns() { } @Test - public void testBackfillUsingSCSI() { + public void testBackfillUsingSCSIOnly() { LocalDAOStorageConfig storageConfig = makeLocalDAOStorageConfig(AspectFoo.class, Collections.singletonList("/value"), AspectBar.class, Collections.singletonList("/value")); @@ -516,7 +549,7 @@ public void testBackfillUsingSCSI() { Map, Optional>> backfilledAspects = dao.backfill(BackfillMode.SCSI_ONLY, Collections.singleton(AspectFoo.class), FooUrn.class, null, 3); for (int index = 0; index < 3; index++) { - Urn urn = urns.get(index); + FooUrn urn = urns.get(index); RecordTemplate aspect = aspects.get(urn).get(AspectFoo.class); assertEquals(backfilledAspects.get(urn).get(AspectFoo.class).get(), aspect); verify(_mockProducer, times(0)).produceMetadataAuditEvent(urn, aspect, aspect); @@ -524,27 +557,80 @@ public void testBackfillUsingSCSI() { IndexFilter indexFilter = new IndexFilter().setCriteria( new IndexCriterionArray(new IndexCriterion().setAspect(AspectFoo.class.getCanonicalName()))); assertEquals(dao.listUrns(indexFilter, null, 3).size(), 3); + } + + @Test + public void testBackfillUsingMaeOnly() { + LocalDAOStorageConfig storageConfig = + makeLocalDAOStorageConfig(AspectFoo.class, Collections.singletonList("/value"), AspectBar.class, + Collections.singletonList("/value")); + EbeanLocalDAO dao = + new EbeanLocalDAO(_mockProducer, _server, storageConfig, FooUrn.class); + dao.enableLocalSecondaryIndex(true); + + List urns = ImmutableList.of(makeFooUrn(1), makeFooUrn(2), makeFooUrn(3)); + + Map, RecordTemplate>> aspects = new HashMap<>(); + + urns.forEach(urn -> { + AspectFoo aspectFoo = new AspectFoo().setValue("foo"); + AspectBar aspectBar = new AspectBar().setValue("bar"); + + // update metadata_aspects table + aspects.put(urn, ImmutableMap.of(AspectFoo.class, aspectFoo, AspectBar.class, aspectBar)); + addMetadata(urn, AspectFoo.class.getCanonicalName(), 0, aspectFoo); + addMetadata(urn, AspectBar.class.getCanonicalName(), 0, aspectBar); + + // only index urn + addIndex(urn, FooUrn.class.getCanonicalName(), "/fooId", urn.getFooIdEntity()); + }); // Backfill in MAE_ONLY mode - backfilledAspects = + Map, Optional>> backfilledAspects = dao.backfill(BackfillMode.MAE_ONLY, Collections.singleton(AspectBar.class), FooUrn.class, null, 3); for (int index = 0; index < 3; index++) { - Urn urn = urns.get(index); + FooUrn urn = urns.get(index); RecordTemplate aspect = aspects.get(urn).get(AspectBar.class); assertEquals(backfilledAspects.get(urn).get(AspectBar.class).get(), aspect); verify(_mockProducer, times(1)).produceMetadataAuditEvent(urn, aspect, aspect); } - clearInvocations(_mockProducer); + } + + @Test + public void testBackfillUsingBackfillAll() { + LocalDAOStorageConfig storageConfig = + makeLocalDAOStorageConfig(AspectFoo.class, Collections.singletonList("/value"), AspectBar.class, + Collections.singletonList("/value")); + EbeanLocalDAO dao = + new EbeanLocalDAO(_mockProducer, _server, storageConfig, FooUrn.class); + dao.enableLocalSecondaryIndex(true); + + List urns = ImmutableList.of(makeFooUrn(1), makeFooUrn(2), makeFooUrn(3)); + + Map, RecordTemplate>> aspects = new HashMap<>(); + + urns.forEach(urn -> { + AspectFoo aspectFoo = new AspectFoo().setValue("foo"); + AspectBar aspectBar = new AspectBar().setValue("bar"); + + // update metadata_aspects table + aspects.put(urn, ImmutableMap.of(AspectFoo.class, aspectFoo, AspectBar.class, aspectBar)); + addMetadata(urn, AspectFoo.class.getCanonicalName(), 0, aspectFoo); + addMetadata(urn, AspectBar.class.getCanonicalName(), 0, aspectBar); - indexFilter = new IndexFilter().setCriteria( + // only index urn + addIndex(urn, FooUrn.class.getCanonicalName(), "/fooId", urn.getFooIdEntity()); + }); + + IndexFilter indexFilter = new IndexFilter().setCriteria( new IndexCriterionArray(new IndexCriterion().setAspect(AspectBar.class.getCanonicalName()))); assertEquals(dao.listUrns(indexFilter, null, 3).size(), 0); // Backfill in BACKFILL_ALL mode - backfilledAspects = + Map, Optional>> backfilledAspects = dao.backfill(BackfillMode.BACKFILL_ALL, ImmutableSet.of(AspectBar.class), FooUrn.class, null, 3); for (int index = 0; index < 3; index++) { - Urn urn = urns.get(index); + FooUrn urn = urns.get(index); RecordTemplate aspect = aspects.get(urn).get(AspectBar.class); assertEquals(backfilledAspects.get(urn).get(AspectBar.class).get(), aspect); verify(_mockProducer, times(1)).produceMetadataAuditEvent(urn, aspect, aspect); @@ -855,8 +941,8 @@ public void testList() { assertNotNull(results.getMetadata()); List expectedVersions = Arrays.asList(0L, 1L, 2L, 3L, 4L); List expectedUrns = Arrays.asList(makeFooUrn(0), makeFooUrn(1), makeFooUrn(2), makeFooUrn(3), makeFooUrn(4)); - assertVersionMetadata(results.getMetadata(), expectedVersions, expectedUrns, 1234L, Urns.createFromTypeSpecificString("test", "foo"), - Urns.createFromTypeSpecificString("test", "bar")); + assertVersionMetadata(results.getMetadata(), expectedVersions, expectedUrns, 1234L, + Urns.createFromTypeSpecificString("test", "foo"), Urns.createFromTypeSpecificString("test", "bar")); // List next page results = dao.list(AspectFoo.class, urn0, 5, 9); @@ -999,7 +1085,7 @@ void testNewNumericId() { @Test void testSaveSingleEntryToLocalIndex() { EbeanLocalDAO dao = - new EbeanLocalDAO<>(EntityAspectUnion.class, _mockProducer, _server, BarUrn.class); + new EbeanLocalDAO<>(EntityAspectUnion.class, new DummyMetadataEventProducer<>(), _server, BarUrn.class); BarUrn urn = makeBarUrn(0); // Test indexing integer typed value @@ -1060,7 +1146,7 @@ record = getRecordFromLocalIndex(recordId); @Test void testExistsInLocalIndex() { EbeanLocalDAO dao = - new EbeanLocalDAO<>(EntityAspectUnion.class, _mockProducer, _server, BarUrn.class); + new EbeanLocalDAO<>(EntityAspectUnion.class, new DummyMetadataEventProducer<>(), _server, BarUrn.class); BarUrn urn = makeBarUrn(0); assertFalse(dao.existsInLocalIndex(urn)); @@ -1073,11 +1159,11 @@ void testExistsInLocalIndex() { void testUpdateUrnInLocalIndex() { // only urn will be updated since storage config has not been provided EbeanLocalDAO dao1 = - new EbeanLocalDAO(EntityAspectUnion.class, _mockProducer, _server, BarUrn.class); + new EbeanLocalDAO(EntityAspectUnion.class, new DummyMetadataEventProducer<>(), _server, BarUrn.class); dao1.enableLocalSecondaryIndex(true); dao1.setUrnPathExtractor(new BarUrnPathExtractor()); EbeanLocalDAO dao2 = - new EbeanLocalDAO(EntityAspectUnion.class, _mockProducer, _server, BazUrn.class); + new EbeanLocalDAO(EntityAspectUnion.class, new DummyMetadataEventProducer<>(), _server, BazUrn.class); dao2.enableLocalSecondaryIndex(true); dao2.setUrnPathExtractor(new BazUrnPathExtractor()); @@ -1235,7 +1321,7 @@ void testUpdateUrnAndAspectInLocalIndex() { @Test void testUpdateLocalIndex() { EbeanLocalDAO dao = - new EbeanLocalDAO<>(EntityAspectUnion.class, _mockProducer, _server, BarUrn.class); + new EbeanLocalDAO<>(EntityAspectUnion.class, new DummyMetadataEventProducer<>(), _server, BarUrn.class); dao.enableLocalSecondaryIndex(true); dao.setUrnPathExtractor(new BarUrnPathExtractor()); @@ -1418,9 +1504,9 @@ void testAddEntityTypeFilter() { @Test void testListUrnsFromIndexForAnEntity() { EbeanLocalDAO dao1 = - new EbeanLocalDAO<>(EntityAspectUnion.class, _mockProducer, _server, FooUrn.class); + new EbeanLocalDAO<>(EntityAspectUnion.class, new DummyMetadataEventProducer<>(), _server, FooUrn.class); EbeanLocalDAO dao2 = - new EbeanLocalDAO<>(EntityAspectUnion.class, _mockProducer, _server, BarUrn.class); + new EbeanLocalDAO<>(EntityAspectUnion.class, new DummyMetadataEventProducer<>(), _server, BarUrn.class); dao1.enableLocalSecondaryIndex(true); dao2.enableLocalSecondaryIndex(true); dao1.setUrnPathExtractor(new FooUrnPathExtractor()); diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/ImmutableLocalDAOTest.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/ImmutableLocalDAOTest.java index 3a8b38194..a93fef7cc 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/ImmutableLocalDAOTest.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/ImmutableLocalDAOTest.java @@ -8,7 +8,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Optional; -import org.json.simple.parser.ParseException; +import java.util.stream.Collectors; import org.testng.annotations.Test; import static com.linkedin.common.AuditStamps.*; @@ -57,8 +57,17 @@ public void testNewNumericId() { private Map loadAspectsFromResource(String name) { try { - return ImmutableLocalDAO.loadAspects(AspectFoo.class, getClass().getClassLoader().getResourceAsStream(name)); - } catch (ParseException | IOException | URISyntaxException e) { + return ImmutableLocalDAO.loadAspects(AspectFoo.class, getClass().getClassLoader().getResourceAsStream(name)) + .entrySet() + .stream() + .collect(Collectors.toMap(e -> { + try { + return FooUrn.createFromString(e.getKey().toString()); + } catch (URISyntaxException ex) { + throw new RuntimeException(ex); + } + }, Map.Entry::getValue)); + } catch (IOException | URISyntaxException e) { throw new RuntimeException(e); } } diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/BarUrnPathExtractor.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/BarUrnPathExtractor.java index 7c23d381d..1e04673e0 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/BarUrnPathExtractor.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/BarUrnPathExtractor.java @@ -11,7 +11,7 @@ public class BarUrnPathExtractor implements UrnPathExtractor { @Override public Map extractPaths(@Nonnull BarUrn urn) { - return Collections.unmodifiableMap(new HashMap() { + return Collections.unmodifiableMap(new HashMap() { { put("/barId", urn.getBarIdEntity()); } diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/BazUrnPathExtractor.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/BazUrnPathExtractor.java index 2fbb01d28..52762f293 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/BazUrnPathExtractor.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/BazUrnPathExtractor.java @@ -11,7 +11,7 @@ public class BazUrnPathExtractor implements UrnPathExtractor { @Override public Map extractPaths(@Nonnull BazUrn urn) { - return Collections.unmodifiableMap(new HashMap() { + return Collections.unmodifiableMap(new HashMap() { { put("/bazId", urn.getBazIdEntity()); } diff --git a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/FooUrnPathExtractor.java b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/FooUrnPathExtractor.java index e95d88d3f..653a75986 100644 --- a/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/FooUrnPathExtractor.java +++ b/dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/FooUrnPathExtractor.java @@ -11,7 +11,7 @@ public class FooUrnPathExtractor implements UrnPathExtractor { @Override public Map extractPaths(@Nonnull FooUrn urn) { - return Collections.unmodifiableMap(new HashMap() { + return Collections.unmodifiableMap(new HashMap() { { put("/fooId", urn.getFooIdEntity()); } From 9520f1be316f681088aaaad9cbe9a9835f9176c9 Mon Sep 17 00:00:00 2001 From: John Plaisted Date: Tue, 1 Dec 2020 16:16:18 -0800 Subject: [PATCH 2/2] remove git conflict markers --- build.gradle | 3 --- 1 file changed, 3 deletions(-) diff --git a/build.gradle b/build.gradle index 366e6551b..145c79465 100644 --- a/build.gradle +++ b/build.gradle @@ -46,13 +46,10 @@ project.ext.externalDependency = [ 'jacksonCore': 'com.fasterxml.jackson.core:jackson-core:2.9.7', 'jacksonDataBind': 'com.fasterxml.jackson.core:jackson-databind:2.9.7', 'javatuples': 'org.javatuples:javatuples:1.2', -<<<<<<< HEAD 'jsonSimple': 'com.googlecode.json-simple:json-simple:1.1.1', 'junitJupiterApi': "org.junit.jupiter:junit-jupiter-api:$junitJupiterVersion", 'junitJupiterParams': "org.junit.jupiter:junit-jupiter-params:$junitJupiterVersion", 'junitJupiterEngine': "org.junit.jupiter:junit-jupiter-engine:$junitJupiterVersion", -======= ->>>>>>> 82e47552 (fix: enable werror for ebean dao.) 'lombok': 'org.projectlombok:lombok:1.18.12', 'mockito': 'org.mockito:mockito-core:3.0.0', 'mockitoInline': 'org.mockito:mockito-inline:3.0.0',