Skip to content

Commit

Permalink
refactor(criterion): refactor criterion construction
Browse files Browse the repository at this point in the history
* consolidate all criterion builders to util
* remove deprecated getValue()/setValue()
* work around pdl contraints on non-null getValue()
  • Loading branch information
david-leifker committed Sep 26, 2024
1 parent 3366cdf commit f5737aa
Show file tree
Hide file tree
Showing 74 changed files with 786 additions and 1,509 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static com.linkedin.metadata.Constants.CORP_USER_ENTITY_NAME;
import static com.linkedin.metadata.Constants.CORP_USER_STATUS_LAST_MODIFIED_FIELD_NAME;
import static com.linkedin.metadata.utils.CriterionUtils.buildCriterion;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -30,7 +31,6 @@
import com.linkedin.metadata.query.filter.Condition;
import com.linkedin.metadata.query.filter.ConjunctiveCriterion;
import com.linkedin.metadata.query.filter.ConjunctiveCriterionArray;
import com.linkedin.metadata.query.filter.Criterion;
import com.linkedin.metadata.query.filter.CriterionArray;
import com.linkedin.metadata.query.filter.Filter;
import com.linkedin.metadata.query.filter.SortCriterion;
Expand Down Expand Up @@ -153,12 +153,11 @@ private SearchResult searchForNewUsers(@Nonnull final OperationContext opContext
.setAnd(
new CriterionArray(
ImmutableList.of(
new Criterion()
.setField(CORP_USER_STATUS_LAST_MODIFIED_FIELD_NAME)
.setCondition(Condition.GREATER_THAN)
.setValue(
String.valueOf(
trailingMonthDateRange.getStart())))))))),
buildCriterion(
CORP_USER_STATUS_LAST_MODIFIED_FIELD_NAME,
Condition.GREATER_THAN,
String.valueOf(
trailingMonthDateRange.getStart())))))))),
Collections.singletonList(
new SortCriterion()
.setField(CORP_USER_STATUS_LAST_MODIFIED_FIELD_NAME)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@

import static com.linkedin.datahub.graphql.resolvers.search.SearchUtils.*;
import static com.linkedin.metadata.Constants.*;
import static com.linkedin.metadata.utils.CriterionUtils.buildCriterion;

import com.datahub.authentication.Authentication;
import com.fasterxml.jackson.core.StreamReadConstraints;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.linkedin.common.urn.UrnUtils;
import com.linkedin.data.template.StringArray;
import com.linkedin.datahub.graphql.QueryContext;
import com.linkedin.datahub.graphql.exception.ValidationException;
import com.linkedin.datahub.graphql.generated.AndFilterInput;
Expand Down Expand Up @@ -154,38 +154,22 @@ public static Filter buildFilter(

// Translates a FacetFilterInput (graphql input class) into Criterion (our internal model)
public static Criterion criterionFromFilter(final FacetFilterInput filter) {
Criterion result = new Criterion();
result.setField(filter.getField());

// `value` is deprecated in place of `values`- this is to support old query patterns. If values
// is provided,
// this statement will be skipped
if (filter.getValues() == null && filter.getValue() != null) {
result.setValues(new StringArray(filter.getValue()));
result.setValue(filter.getValue());
} else if (filter.getValues() != null) {
result.setValues(new StringArray(filter.getValues()));
if (!filter.getValues().isEmpty()) {
result.setValue(filter.getValues().get(0));
} else {
result.setValue("");
}
} else {
result.setValues(new StringArray());
result.setValue("");
}

final Condition condition;
if (filter.getCondition() != null) {
result.setCondition(Condition.valueOf(filter.getCondition().toString()));
condition = Condition.valueOf(filter.getCondition().toString());
} else {
result.setCondition(Condition.EQUAL);
condition = Condition.EQUAL;
}

if (filter.getNegated() != null) {
result.setNegated(filter.getNegated());
final List<String> values;
if (filter.getValues() == null && filter.getValue() != null) {
values = Collections.singletonList(filter.getValue());
} else {
values = filter.getValues();
}

return result;
return buildCriterion(filter.getField(), condition, filter.getNegated(), values);
}

public static Filter viewFilter(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
package com.linkedin.datahub.graphql.resolvers.auth;

import static com.linkedin.metadata.Constants.*;
import static com.linkedin.metadata.utils.CriterionUtils.buildCriterion;

import com.google.common.collect.ImmutableSet;
import com.linkedin.common.EntityRelationship;
import com.linkedin.common.EntityRelationships;
import com.linkedin.common.urn.Urn;
import com.linkedin.data.template.StringArray;
import com.linkedin.datahub.graphql.QueryContext;
import com.linkedin.datahub.graphql.authorization.AuthorizationUtils;
import com.linkedin.datahub.graphql.concurrency.GraphQLConcurrencyUtils;
Expand All @@ -19,7 +19,6 @@
import com.linkedin.metadata.query.filter.Condition;
import com.linkedin.metadata.query.filter.ConjunctiveCriterion;
import com.linkedin.metadata.query.filter.ConjunctiveCriterionArray;
import com.linkedin.metadata.query.filter.Criterion;
import com.linkedin.metadata.query.filter.CriterionArray;
import com.linkedin.metadata.query.filter.Filter;
import com.linkedin.metadata.query.filter.RelationshipDirection;
Expand Down Expand Up @@ -199,41 +198,28 @@ private Filter buildFilterToGetPolicies(
ConjunctiveCriterionArray conjunctiveCriteria = new ConjunctiveCriterionArray();

final CriterionArray allUsersAndArray = new CriterionArray();
allUsersAndArray.add(
new Criterion().setField("allUsers").setValue("true").setCondition(Condition.EQUAL));
allUsersAndArray.add(buildCriterion("allUsers", Condition.EQUAL, "true"));
conjunctiveCriteria.add(new ConjunctiveCriterion().setAnd(allUsersAndArray));

final CriterionArray allGroupsAndArray = new CriterionArray();
allGroupsAndArray.add(
new Criterion().setField("allGroups").setValue("true").setCondition(Condition.EQUAL));
allGroupsAndArray.add(buildCriterion("allGroups", Condition.EQUAL, "true"));
conjunctiveCriteria.add(new ConjunctiveCriterion().setAnd(allGroupsAndArray));

if (user != null && !user.isEmpty()) {
final CriterionArray userAndArray = new CriterionArray();
userAndArray.add(
new Criterion().setField("users").setValue(user).setCondition(Condition.EQUAL));
userAndArray.add(buildCriterion("users", Condition.EQUAL, user));
conjunctiveCriteria.add(new ConjunctiveCriterion().setAnd(userAndArray));
}

if (groups != null && !groups.isEmpty()) {
final CriterionArray groupsAndArray = new CriterionArray();
groupsAndArray.add(
new Criterion()
.setField("groups")
.setValue("")
.setValues(new StringArray(groups))
.setCondition(Condition.EQUAL));
groupsAndArray.add(buildCriterion("groups", Condition.EQUAL, groups));
conjunctiveCriteria.add(new ConjunctiveCriterion().setAnd(groupsAndArray));
}

if (roles != null && !roles.isEmpty()) {
final CriterionArray rolesAndArray = new CriterionArray();
rolesAndArray.add(
new Criterion()
.setField("roles")
.setValue("")
.setValues(new StringArray(roles))
.setCondition(Condition.EQUAL));
rolesAndArray.add(buildCriterion("roles", Condition.EQUAL, roles));
conjunctiveCriteria.add(new ConjunctiveCriterion().setAnd(rolesAndArray));
}
return new Filter().setOr(conjunctiveCriteria);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.linkedin.datahub.graphql.resolvers.container;

import static com.linkedin.datahub.graphql.resolvers.ResolverUtils.*;
import static com.linkedin.metadata.utils.CriterionUtils.buildCriterion;

import com.google.common.collect.ImmutableList;
import com.linkedin.datahub.graphql.QueryContext;
Expand Down Expand Up @@ -74,10 +75,7 @@ public CompletableFuture<SearchResults> get(final DataFetchingEnvironment enviro
try {

final Criterion filterCriterion =
new Criterion()
.setField(CONTAINER_FIELD_NAME + ".keyword")
.setCondition(Condition.EQUAL)
.setValue(urn);
buildCriterion(CONTAINER_FIELD_NAME + ".keyword", Condition.EQUAL, urn);

return UrnSearchResultsMapper.map(
context,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.linkedin.datahub.graphql.resolvers.dashboard;

import static com.linkedin.datahub.graphql.resolvers.dashboard.DashboardUsageStatsUtils.*;
import static com.linkedin.metadata.utils.CriterionUtils.buildIsNullCriterion;

import com.google.common.collect.ImmutableList;
import com.linkedin.common.urn.Urn;
Expand All @@ -14,7 +15,6 @@
import com.linkedin.datahub.graphql.types.dashboard.mappers.DashboardUsageMetricMapper;
import com.linkedin.metadata.Constants;
import com.linkedin.metadata.aspect.EnvelopedAspect;
import com.linkedin.metadata.query.filter.Condition;
import com.linkedin.metadata.query.filter.ConjunctiveCriterion;
import com.linkedin.metadata.query.filter.ConjunctiveCriterionArray;
import com.linkedin.metadata.query.filter.Criterion;
Expand Down Expand Up @@ -102,11 +102,7 @@ private List<DashboardUsageMetrics> getDashboardUsageMetrics(
final ArrayList<Criterion> criteria = new ArrayList<>();

// Add filter for absence of eventGranularity - only consider absolute stats
Criterion excludeTimeBucketsCriterion =
new Criterion()
.setField(ES_FIELD_EVENT_GRANULARITY)
.setCondition(Condition.IS_NULL)
.setValue("");
Criterion excludeTimeBucketsCriterion = buildIsNullCriterion(ES_FIELD_EVENT_GRANULARITY);
criteria.add(excludeTimeBucketsCriterion);
filter.setOr(
new ConjunctiveCriterionArray(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package com.linkedin.datahub.graphql.resolvers.dashboard;

import static com.linkedin.metadata.utils.CriterionUtils.buildCriterion;
import static com.linkedin.metadata.utils.CriterionUtils.buildIsNullCriterion;

import com.google.common.collect.ImmutableList;
import com.linkedin.common.urn.Urn;
import com.linkedin.data.template.StringArray;
Expand Down Expand Up @@ -319,27 +322,22 @@ public static Filter createUsageFilter(
final ArrayList<Criterion> criteria = new ArrayList<>();

// Add filter for urn == dashboardUrn
Criterion dashboardUrnCriterion =
new Criterion().setField(ES_FIELD_URN).setCondition(Condition.EQUAL).setValue(dashboardUrn);
Criterion dashboardUrnCriterion = buildCriterion(ES_FIELD_URN, Condition.EQUAL, dashboardUrn);
criteria.add(dashboardUrnCriterion);

if (startTime != null) {
// Add filter for start time
Criterion startTimeCriterion =
new Criterion()
.setField(ES_FIELD_TIMESTAMP)
.setCondition(Condition.GREATER_THAN_OR_EQUAL_TO)
.setValue(Long.toString(startTime));
buildCriterion(
ES_FIELD_TIMESTAMP, Condition.GREATER_THAN_OR_EQUAL_TO, Long.toString(startTime));
criteria.add(startTimeCriterion);
}

if (endTime != null) {
// Add filter for end time
Criterion endTimeCriterion =
new Criterion()
.setField(ES_FIELD_TIMESTAMP)
.setCondition(Condition.LESS_THAN_OR_EQUAL_TO)
.setValue(Long.toString(endTime));
buildCriterion(
ES_FIELD_TIMESTAMP, Condition.LESS_THAN_OR_EQUAL_TO, Long.toString(endTime));
criteria.add(endTimeCriterion);
}

Expand All @@ -348,18 +346,11 @@ public static Filter createUsageFilter(
// stats
// since unit is mandatory, we assume if eventGranularity contains unit, then it is not null
Criterion onlyTimeBucketsCriterion =
new Criterion()
.setField(ES_FIELD_EVENT_GRANULARITY)
.setCondition(Condition.CONTAIN)
.setValue("unit");
buildCriterion(ES_FIELD_EVENT_GRANULARITY, Condition.CONTAIN, "unit");
criteria.add(onlyTimeBucketsCriterion);
} else {
// Add filter for absence of eventGranularity - only consider absolute stats
Criterion excludeTimeBucketsCriterion =
new Criterion()
.setField(ES_FIELD_EVENT_GRANULARITY)
.setCondition(Condition.IS_NULL)
.setValue("");
Criterion excludeTimeBucketsCriterion = buildIsNullCriterion(ES_FIELD_EVENT_GRANULARITY);
criteria.add(excludeTimeBucketsCriterion);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package com.linkedin.datahub.graphql.resolvers.dataset;

import static com.linkedin.metadata.Constants.ASSERTION_RUN_EVENT_STATUS_COMPLETE;
import static com.linkedin.metadata.utils.CriterionUtils.buildCriterion;

import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -197,16 +200,12 @@ private Filter createAssertionsFilter(final String datasetUrn) {
final ArrayList<Criterion> criteria = new ArrayList<>();

// Add filter for asserteeUrn == datasetUrn
Criterion datasetUrnCriterion =
new Criterion().setField("asserteeUrn").setCondition(Condition.EQUAL).setValue(datasetUrn);
Criterion datasetUrnCriterion = buildCriterion("asserteeUrn", Condition.EQUAL, datasetUrn);
criteria.add(datasetUrnCriterion);

// Add filter for result == result
Criterion startTimeCriterion =
new Criterion()
.setField("status")
.setCondition(Condition.EQUAL)
.setValue(Constants.ASSERTION_RUN_EVENT_STATUS_COMPLETE);
buildCriterion("status", Condition.EQUAL, ASSERTION_RUN_EVENT_STATUS_COMPLETE);
criteria.add(startTimeCriterion);

filter.setOr(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static com.linkedin.datahub.graphql.resolvers.ResolverUtils.*;
import static com.linkedin.datahub.graphql.resolvers.search.SearchUtils.*;
import static com.linkedin.metadata.utils.CriterionUtils.buildCriterion;

import com.linkedin.datahub.graphql.QueryContext;
import com.linkedin.datahub.graphql.concurrency.GraphQLConcurrencyUtils;
Expand Down Expand Up @@ -69,10 +70,7 @@ public CompletableFuture<SearchResults> get(final DataFetchingEnvironment enviro

final CriterionArray criteria = new CriterionArray();
final Criterion filterCriterion =
new Criterion()
.setField(DOMAINS_FIELD_NAME + ".keyword")
.setCondition(Condition.EQUAL)
.setValue(urn);
buildCriterion(DOMAINS_FIELD_NAME + ".keyword", Condition.EQUAL, urn);
criteria.add(filterCriterion);
if (input.getFilters() != null) {
input
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.linkedin.datahub.graphql.resolvers.glossary;

import static com.linkedin.datahub.graphql.resolvers.ResolverUtils.bindArgument;
import static com.linkedin.metadata.utils.CriterionUtils.buildCriterion;

import com.google.common.collect.ImmutableList;
import com.linkedin.common.urn.Urn;
Expand All @@ -15,7 +16,6 @@
import com.linkedin.metadata.query.filter.Condition;
import com.linkedin.metadata.query.filter.ConjunctiveCriterion;
import com.linkedin.metadata.query.filter.ConjunctiveCriterionArray;
import com.linkedin.metadata.query.filter.Criterion;
import com.linkedin.metadata.query.filter.CriterionArray;
import com.linkedin.metadata.query.filter.Filter;
import com.linkedin.metadata.search.SearchEntity;
Expand Down Expand Up @@ -84,11 +84,7 @@ public CompletableFuture<GetRootGlossaryNodesResult> get(
private Filter buildGlossaryEntitiesFilter() {
CriterionArray array =
new CriterionArray(
ImmutableList.of(
new Criterion()
.setField("hasParentNode")
.setValue("false")
.setCondition(Condition.EQUAL)));
ImmutableList.of(buildCriterion("hasParentNode", Condition.EQUAL, "false")));
final Filter filter = new Filter();
filter.setOr(
new ConjunctiveCriterionArray(ImmutableList.of(new ConjunctiveCriterion().setAnd(array))));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.linkedin.datahub.graphql.resolvers.glossary;

import static com.linkedin.datahub.graphql.resolvers.ResolverUtils.bindArgument;
import static com.linkedin.metadata.utils.CriterionUtils.buildCriterion;

import com.google.common.collect.ImmutableList;
import com.linkedin.common.urn.Urn;
Expand All @@ -15,7 +16,6 @@
import com.linkedin.metadata.query.filter.Condition;
import com.linkedin.metadata.query.filter.ConjunctiveCriterion;
import com.linkedin.metadata.query.filter.ConjunctiveCriterionArray;
import com.linkedin.metadata.query.filter.Criterion;
import com.linkedin.metadata.query.filter.CriterionArray;
import com.linkedin.metadata.query.filter.Filter;
import com.linkedin.metadata.search.SearchEntity;
Expand Down Expand Up @@ -84,11 +84,7 @@ public CompletableFuture<GetRootGlossaryTermsResult> get(
private Filter buildGlossaryEntitiesFilter() {
CriterionArray array =
new CriterionArray(
ImmutableList.of(
new Criterion()
.setField("hasParentNode")
.setValue("false")
.setCondition(Condition.EQUAL)));
ImmutableList.of(buildCriterion("hasParentNode", Condition.EQUAL, "false")));
final Filter filter = new Filter();
filter.setOr(
new ConjunctiveCriterionArray(ImmutableList.of(new ConjunctiveCriterion().setAnd(array))));
Expand Down
Loading

0 comments on commit f5737aa

Please sign in to comment.