Skip to content

Schema changes to get entities count for labels #122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package org.hypertrace.graphql.label.request;

import org.hypertrace.core.graphql.common.request.ContextualRequest;
import org.hypertrace.graphql.label.schema.Label;
import org.hypertrace.graphql.label.schema.mutation.UpdateLabel;

public interface LabelUpdateRequest extends ContextualRequest {
Label label();
UpdateLabel label();
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import org.hypertrace.core.graphql.common.schema.id.Identifiable;

@GraphQLName(Label.TYPE_NAME)
public interface Label extends Identifiable, LabelData {
public interface Label extends Identifiable, LabelData, LabeledEntities {
String TYPE_NAME = "Label";
String ARGUMENT_NAME = "label";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package org.hypertrace.graphql.label.schema;

import graphql.annotations.annotationTypes.GraphQLField;
import graphql.annotations.annotationTypes.GraphQLName;
import graphql.annotations.annotationTypes.GraphQLNonNull;
import org.hypertrace.core.graphql.common.schema.results.arguments.page.LimitArgument;

@GraphQLName(LabeledEntities.TYPE_NAME)
public interface LabeledEntities {
String TYPE_NAME = "LabeledEntities";
String LABELED_ENTITIES_QUERY_NAME = "labeledEntities";
String ENTITY_TYPE_ARGUMENT_NAME = "type";

@GraphQLField
@GraphQLNonNull
@GraphQLName(LABELED_ENTITIES_QUERY_NAME)
LabeledEntityResultSet labeledEntities(
@GraphQLNonNull @GraphQLName(ENTITY_TYPE_ARGUMENT_NAME) String entityType,
@GraphQLName(LimitArgument.ARGUMENT_NAME) int limit);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package org.hypertrace.graphql.label.schema;

import graphql.annotations.annotationTypes.GraphQLName;
import org.hypertrace.core.graphql.common.schema.attributes.AttributeQueryable;
import org.hypertrace.core.graphql.common.schema.id.Identifiable;

@GraphQLName(LabeledEntity.TYPE_NAME)
public interface LabeledEntity extends AttributeQueryable, Identifiable {
String TYPE_NAME = "LabeledEntity";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package org.hypertrace.graphql.label.schema;

import graphql.annotations.annotationTypes.GraphQLField;
import graphql.annotations.annotationTypes.GraphQLName;
import graphql.annotations.annotationTypes.GraphQLNonNull;
import java.util.List;
import org.hypertrace.core.graphql.common.schema.results.ResultSet;

@GraphQLName(LabeledEntityResultSet.TYPE_NAME)
public interface LabeledEntityResultSet extends ResultSet<LabeledEntity> {
String TYPE_NAME = "LabeledEntityResultSet";

@Override
@GraphQLField
@GraphQLNonNull
@GraphQLName(RESULT_SET_RESULTS_NAME)
List<LabeledEntity> results();
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,5 @@ public interface LabelMutationSchema {
@GraphQLNonNull
@GraphQLName(UPDATE_LABEL)
@GraphQLDataFetcher(LabelUpdateMutator.class)
Label updateLabel(@GraphQLNonNull @GraphQLName(Label.ARGUMENT_NAME) Label label);
Label updateLabel(@GraphQLNonNull @GraphQLName(Label.ARGUMENT_NAME) UpdateLabel label);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package org.hypertrace.graphql.label.schema.mutation;

import graphql.annotations.annotationTypes.GraphQLName;
import org.hypertrace.core.graphql.common.schema.id.Identifiable;
import org.hypertrace.graphql.label.schema.LabelData;

@GraphQLName(UpdateLabel.TYPE_NAME)
public interface UpdateLabel extends Identifiable, LabelData {
String TYPE_NAME = "UpdateLabel";
String ARGUMENT_NAME = "label";
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
package org.hypertrace.graphql.label.dao;

import io.reactivex.rxjava3.core.Single;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;
import lombok.Value;
import lombok.experimental.Accessors;
import org.hypertrace.graphql.label.schema.Label;
import org.hypertrace.graphql.label.schema.LabelResultSet;
import org.hypertrace.graphql.label.schema.LabeledEntity;
import org.hypertrace.graphql.label.schema.LabeledEntityResultSet;
import org.hypertrace.label.config.service.v1.CreateLabelResponse;
import org.hypertrace.label.config.service.v1.GetLabelsResponse;
import org.hypertrace.label.config.service.v1.UpdateLabelResponse;
Expand Down Expand Up @@ -42,7 +47,8 @@ private Label convertLabel(org.hypertrace.label.config.service.v1.Label label) {
label.getId(),
label.getData().getKey(),
label.getData().hasColor() ? label.getData().getColor() : null,
label.getData().hasDescription() ? label.getData().getDescription() : null);
label.getData().hasDescription() ? label.getData().getDescription() : null,
Collections.emptyMap());
}

@Value
Expand All @@ -60,5 +66,36 @@ private static class DefaultLabel implements Label {
String key;
String color;
String description;
Map<String, List<LabeledEntity>> labeledEntitiesMap;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would make sense to hold a Map<String, LabeledEntityResultSet> I think

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure


@Override
public LabeledEntityResultSet labeledEntities(String entityType, int limit) {
List<LabeledEntity> labeledEntities =
Optional.ofNullable(labeledEntitiesMap.get(entityType)).orElse(Collections.emptyList());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're doing the work on the request side, this should be unreachable - can assume the map contains any requested entity types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to return empty values till implementation is done

List<LabeledEntity> labeledEntitiesWithLimit =
labeledEntities.size() > limit ? labeledEntities.subList(0, limit) : labeledEntities;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

limits should be enforced on the request side

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An entity may hold multiple labels. I was planning to fetch entities along with labels, and reverse map from labels to entities here. Let me know if there is a better approach.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking we'd just create a query for entities filtered by label. Using the label results, you have a list of label IDs which can become an IN filter on a single entity query (per requested type). You're likely right about the limit being enforced on the response side though, now that I think about it. We can either

  • Make a request per [label, type] pair and enforce it per request
  • Make a request per type, and enforce it in memory when demuxing the result
    The second option seems to me like it will be significantly more performant, even if I don't love the idea that we're limiting in memory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going for second option as well

return new DefaultLabeledEntityResultSet(
labeledEntitiesWithLimit, labeledEntitiesWithLimit.size(), labeledEntities.size());
}
}

@Value
@Accessors(fluent = true)
private static class DefaultLabeledEntityResultSet implements LabeledEntityResultSet {
List<LabeledEntity> results;
long count;
long total;
}

@Value
@Accessors(fluent = true)
private static class DefaultLabeledEntity implements LabeledEntity {
String id;
Map<String, Object> attributeValues;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah crap, I have an enormous change (oops) locally that changes how attributes work

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me know what changes are coming, and I will update accordingly

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The relevant part here is that AttributeQueryable changes.

Object attribute(@GraphQLName(AttributeExpression.ARGUMENT_NAME) AttributeExpression expression);

and most of the impls are now holding a

    Map<AttributeExpression, Object> attributeValues;

Unless I'm able to get stuff merged (and it won't be today), not much you can do to prepare for it, since don't have the new types.


@Override
public Object attribute(String key) {
return this.attributeValues.get(key);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ protected void configure() {
Multibinder.newSetBinder(binder(), ArgumentDeserializationConfig.class);

deserializationConfigBinder.addBinding().to(CreateLabelDeserializationConfig.class);
deserializationConfigBinder.addBinding().to(LabelDeserializationConfig.class);
deserializationConfigBinder.addBinding().to(UpdateLabelDeserializationConfig.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,30 @@
import lombok.Value;
import lombok.experimental.Accessors;
import org.hypertrace.core.graphql.deserialization.ArgumentDeserializationConfig;
import org.hypertrace.graphql.label.schema.Label;
import org.hypertrace.graphql.label.schema.mutation.UpdateLabel;

public class LabelDeserializationConfig implements ArgumentDeserializationConfig {
public class UpdateLabelDeserializationConfig implements ArgumentDeserializationConfig {

@Override
public String getArgumentKey() {
return Label.ARGUMENT_NAME;
return UpdateLabel.ARGUMENT_NAME;
}

@Override
public Class<?> getArgumentSchema() {
return Label.class;
return UpdateLabel.class;
}

@Override
public List<Module> jacksonModules() {
return List.of(new SimpleModule().addAbstractTypeMapping(Label.class, LabelArgument.class));
return List.of(
new SimpleModule().addAbstractTypeMapping(UpdateLabel.class, UpdateLabelArgument.class));
}

@Value
@Accessors(fluent = true)
@NoArgsConstructor(force = true)
private static class LabelArgument implements Label {
private static class UpdateLabelArgument implements UpdateLabel {
@JsonProperty(IDENTITY_FIELD_NAME)
String id;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
import lombok.experimental.Accessors;
import org.hypertrace.core.graphql.context.GraphQlRequestContext;
import org.hypertrace.core.graphql.deserialization.ArgumentDeserializer;
import org.hypertrace.graphql.label.schema.Label;
import org.hypertrace.graphql.label.schema.mutation.CreateLabel;
import org.hypertrace.graphql.label.schema.mutation.UpdateLabel;

public class LabelRequestBuilderImpl implements LabelRequestBuilder {
private final ArgumentDeserializer argumentDeserializer;
Expand All @@ -30,7 +30,7 @@ public LabelUpdateRequest buildUpdateRequest(
GraphQlRequestContext requestContext, Map<String, Object> arguments) {
return new LabelUpdateRequestImpl(
requestContext,
this.argumentDeserializer.deserializeObject(arguments, Label.class).orElseThrow());
this.argumentDeserializer.deserializeObject(arguments, UpdateLabel.class).orElseThrow());
}

@Value
Expand All @@ -44,6 +44,6 @@ private static class LabelCreateRequestImpl implements LabelCreateRequest {
@Accessors(fluent = true)
private static class LabelUpdateRequestImpl implements LabelUpdateRequest {
GraphQlRequestContext context;
Label label;
UpdateLabel label;
}
}