[Generic DAO][Part 2] Implement backfill method #385
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #385 +/- ##
============================================
+ Coverage 67.73% 67.84% +0.10%
- Complexity 1375 1388 +13
============================================
Files 136 136
Lines 5412 5446 +34
Branches 565 572 +7
============================================
+ Hits 3666 3695 +29
+ Misses 1516 1514 -2
- Partials 230 237 +7 ☔ View full report in Codecov by Sentry. |
dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/EbeanGenericLocalDAO.java
Show resolved
Hide resolved
| public interface GenericMetadataProducer { | ||
|
|
||
| /** | ||
| * Produces an aspect specific Metadata Audit Event (MAE) after a metadata aspect is updated for an entity. |
There was a problem hiding this comment.
Can it be used in MCE producer as well?
There was a problem hiding this comment.
No, this producer is to replace the type-bounded BaseMetadataEventProducer and specifically used for MAE emission.
MCE producer is within ingestion-template (MP: metadata-graph-platformization)
There was a problem hiding this comment.
Thanks for clarification. I'm more from the requirement on "entity agnostic" aspect ingestion. Doesn't need to be addressed here.
| final AuditStamp oldAuditStamp = latest.get().getExtraInfo() == null ? null : latest.get().getExtraInfo().getAudit(); | ||
|
|
||
| final boolean isBackfillEvent = trackingContext != null && trackingContext.hasBackfill() && trackingContext.isBackfill(); | ||
| final boolean shouldBackfill = trackingContext != null && trackingContext.hasEmitTime() && oldAuditStamp != null && oldAuditStamp.hasTime() |
There was a problem hiding this comment.
this check is complicated, making it a function and covered with an unit test?
There was a problem hiding this comment.
agree, convert it to method and added some unit tests.
| final boolean shouldBackfill = trackingContext != null && trackingContext.hasEmitTime() && oldAuditStamp != null && oldAuditStamp.hasTime() | ||
| && trackingContext.getEmitTime() > oldAuditStamp.getTime(); | ||
|
|
||
| // Skip update if metadata is sent by backfill event but should not be backfilled. |
There was a problem hiding this comment.
nit: such as expired backfill? (any other scenarios? )
There was a problem hiding this comment.
right, it's basically to prevent expired backfill. Let me update the comment to make it more clear.
| @Nonnull BackfillMode mode, @Nonnull Map<Urn, Set<Class<? extends RecordTemplate>>> aspectClasses) { | ||
|
|
||
| if (aspectClasses.isEmpty()) { | ||
| return new HashMap<>(); |
There was a problem hiding this comment.
Collections.emptyMap()?
|
|
||
| Optional<GenericLocalDAO.MetadataWithExtraInfo> metadata = _genericLocalDAO.queryLatest(fooUrn, AspectFoo.class); | ||
|
|
||
| // {"value":"bar"} is inserted later so it is the latest metadata. | ||
| assertEquals(metadata.get().getAspect(), RecordUtils.toJsonString(aspectFoo2)); | ||
| } | ||
|
|
||
| @Test | ||
| public void testBackfill() throws URISyntaxException { |
There was a problem hiding this comment.
nit: adding [when / given / expect] would help for readability.
There was a problem hiding this comment.
sounds good, added [when / expects]
Summary
Add backfill method into Generic DAO.
Testing Done
Unit tests
Checklist