Changes to get entities and services count associated with label #123
Conversation
Codecov Report
@@ Coverage Diff @@
## main #123 +/- ##
=========================================
Coverage 22.98% 22.98%
Complexity 75 75
=========================================
Files 65 65
Lines 1679 1679
Branches 52 52
=========================================
Hits 386 386
Misses 1284 1284
Partials 9 9
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
...schema-impl/src/main/java/org/hypertrace/graphql/label/joiner/DefaultLabelJoinerBuilder.java
Outdated
Show resolved
Hide resolved
...graphql-labels-schema-api/src/main/java/org/hypertrace/graphql/label/joiner/LabelJoiner.java
Outdated
Show resolved
Hide resolved
...schema-impl/src/main/java/org/hypertrace/graphql/label/joiner/DefaultLabelJoinerBuilder.java
Outdated
Show resolved
Hide resolved
...aphql-labels-schema-api/src/main/java/org/hypertrace/graphql/label/fetcher/LabelFetcher.java
Outdated
Show resolved
Hide resolved
| return getUpdatedLabels(labelResultSet).flatMap(responseConverter::convert); | ||
| } | ||
|
|
||
| private Single<List<Label>> getUpdatedLabels(LabelResultSet labelResultSet) { |
There was a problem hiding this comment.
To my earlier point, using the joiner in place of the DAO is what leads to this strange fetch, build response then update pattern. Ideally, we're doing the entity joining before constructing the label response.
There was a problem hiding this comment.
Since dao returns a result set, you need to something similar isn't it? Even in entity joiner, entity dao returns entity result set which is then converted to a table and then back to appropriate result set.
There was a problem hiding this comment.
in the current model, some other dao (say, label), would use the joiner when constructing its objects (labels). The entity joiner calls the entity dao and presents the results. Right now, that means its unpacking the result set into a table, and in the version we have here you may need to unpack and repack the result sets. Ideally that's not necessary and we can use them as is, but I think that may be required to enforce limits that differ.
Edit: the unpack/repack will always be needed on entity results - I keep forgetting that we have a different entity schema here.
This comment has been minimized.
This comment has been minimized.
| } | ||
|
|
||
| @Override | ||
| public CompletableFuture<LabelResultSet> get(DataFetchingEnvironment environment) { | ||
| return this.requestBuilder | ||
| .build(environment.getContext(), environment.getSelectionSet()) | ||
| .flatMap(request -> request.joinLabelsWithEntities()) | ||
| .flatMap(request -> request.joinLabelsWithEntitiesAndRules()) |
There was a problem hiding this comment.
I guess it doesn't really matter, but this is that inversion discussed elsewhere - the joiner is wrapping the dao rather than the dao wrapping the joiner. Since the DAO is an abstraction over where the data comes from, joiner in my mind is an implementation detail.
...trace-graphql-labels-schema-api/src/main/java/org/hypertrace/graphql/label/schema/Label.java
Outdated
Show resolved
Hide resolved
...bels-schema-api/src/main/java/org/hypertrace/graphql/label/schema/LabelApplicationRules.java
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
No description provided.