feat(model 2.0): implement extractRelationshipsFromAspect util method #449
Conversation
dao-impl/ebean-dao/src/main/java/com/linkedin/metadata/dao/utils/EBeanDAOUtils.java
Show resolved
Hide resolved
dao-impl/ebean-dao/src/test/java/com/linkedin/metadata/dao/utils/EBeanDAOUtilsTest.java
Outdated
Show resolved
Hide resolved
...a-annotations-test-models/src/main/pegasus/com/linkedin/testing/AnnotatedRelationshipBar.pdl
Show resolved
Hide resolved
There was a problem hiding this comment.
LGTM. Please try to get a review from @yangyangv2 as well since he has more context on the latest discussion. Thanks.
| try { | ||
| Method getMethod = clazz.getMethod("get" + StringUtils.capitalize(fieldName)); // getFieldName | ||
| Object obj = getMethod.invoke(aspect); // invoke getFieldName | ||
| // all relationship fields will be represented as Arrays so filter out any non-lists, empty lists, and lists that don't contain RecordTemplates |
There was a problem hiding this comment.
is this a hard requirement? can't a relationship be a single value field in an aspect?
There was a problem hiding this comment.
Updated in recent PR to support single value field case: #452
| List<RecordTemplate> relationshipsList = (List<RecordTemplate>) obj; | ||
| ModelType modelType = parseModelTypeFromGmaAnnotation(relationshipsList.get(0)); | ||
| if (modelType == ModelType.RELATIONSHIP) { | ||
| log.debug(String.format("Found {%d} relationships of type {%s} for field {%s} of aspect class {%s}.", |
There was a problem hiding this comment.
nit: a more efficient way to write this log is
log.isDebugEnabled() {
log.debug(...)
}
saves the string formatter parsing.
There was a problem hiding this comment.
Could you clarify how this gets rid of the string formatting?
| * field in the aspect. An empty list means that there is no non-null relationship metadata attached to the given aspect. | ||
| */ | ||
| @Nonnull | ||
| public static <RELATIONSHIP extends RecordTemplate, ASPECT extends RecordTemplate> List<List<RELATIONSHIP>> extractRelationshipsFromAspect(ASPECT aspect) { |
There was a problem hiding this comment.
does this mean an Aspect can contain contain one relationship type?
There was a problem hiding this comment.
a few thoughts:
-
have a cache for each Aspect type, and the relationship type and path, since these information are deterministic through the application life cycle. This cache maintains
<Aspect.class, List<<Relationship.class, List<field_path>>>>
-
the method could be List extractRelationshipFromAspect(Aspect aspect, Class relationshipType)
The implementation of 2 can be extended with recursive lookup or get List<field_path> for relationship type in an Aspect for fast access.
What's more important over the performance improvement is we can remove the 1:1 mapping from relationship to aspect.
There was a problem hiding this comment.
A cache is a good suggestion, although a caveat is that some of the relationship fields of an aspect can be not present when we allow an aspect to have multiple relationships. For example, aspect with model:
record AspectFoo {
relationshipBars = optional array[RelationshipBar]
relationshipFooBars = optional array[RelationshipFooBar]
}
On the first ingestion of this aspect, the relationshipBars is present but relationshipFooBars may not be present. so the cache will not register that field. So on the next ingestion of this aspect, we need to check every field again. I will leave this optimization for the future if we see that this logic has noticeable performance impact.
| AnnotatedAspectFooWithRelationshipField fooWithRelationshipField = new AnnotatedAspectFooWithRelationshipField() | ||
| .setRelationshipFoo(relationshipFoos); | ||
|
|
||
| List<List<RecordTemplate>> results = EBeanDAOUtils.extractRelationshipsFromAspect(fooWithRelationshipField); |
There was a problem hiding this comment.
if EBeanDAOUtils.extractRelationshipsFromAspect returns List<List>, what's the point for the generic type RELATIONSHIP
Summary
Implement a util method to extract any relationship fields out of aspects. In a later PR, this util method will be used during relationship ingestion to retrieve relationships to ingest in place of relationship builders.
The logic relies on the assumption that all PDL relationship models are properly annotated with the
@gma.model = RELATIONSHIPannotation.Testing Done
added a unit test.
Checklist