From 1c4c31eb0c12c74af9cc8fc087f368fd8859fd53 Mon Sep 17 00:00:00 2001 From: Ivan Bella <347158+ivakegg@users.noreply.github.com> Date: Fri, 6 Dec 2024 16:48:39 -0500 Subject: [PATCH] Suggested changes (#2656) * Suggested changes * Removed unused imports --- .../datawave/core/query/logic/QueryLogic.java | 9 +-- .../query/language/parser/QueryParser.java | 2 - .../jexl/JexlControlledQueryParser.java | 1 - .../query/tables/ShardQueryLogic.java | 12 +--- .../ShardQueryLogicQueryValidationTest.java | 14 +--- .../query/runner/QueryExecutorBean.java | 67 ++----------------- 6 files changed, 13 insertions(+), 92 deletions(-) diff --git a/core/query/src/main/java/datawave/core/query/logic/QueryLogic.java b/core/query/src/main/java/datawave/core/query/logic/QueryLogic.java index a0b6fee3999..1ae627770df 100644 --- a/core/query/src/main/java/datawave/core/query/logic/QueryLogic.java +++ b/core/query/src/main/java/datawave/core/query/logic/QueryLogic.java @@ -10,8 +10,6 @@ import org.apache.commons.collections4.Transformer; import org.apache.commons.collections4.iterators.TransformIterator; -import com.google.common.collect.HashMultimap; - import datawave.audit.SelectorExtractor; import datawave.core.common.connection.AccumuloConnectionFactory; import datawave.core.query.cache.ResultsPage; @@ -494,17 +492,14 @@ default void preInitialize(Query settings, Set<Authorizations> userAuthorization * the query settings (query, begin date, end date, etc.) * @param auths * the authorizations that have been calculated for this query based on the caller and server. - * @param expandFields - * @param expandValues * @return a list of messages detailing any issues found for the query */ - default Object validateQuery(AccumuloClient client, Query query, Set<Authorizations> auths, boolean expandFields, boolean expandValues) throws Exception { + default Object validateQuery(AccumuloClient client, Query query, Set<Authorizations> auths) throws Exception { throw new UnsupportedOperationException("Query validation not implemented"); } /** - * Return a transformer that will convert the result of {@link QueryLogic#validateQuery(AccumuloClient, Query, Set, boolean, boolean)} to a - * {@link QueryValidationResponse}. + * Return a transformer that will convert the result of {@link QueryLogic#validateQuery(AccumuloClient, Query, Set)} to a {@link QueryValidationResponse}. * * @return the transformer */ diff --git a/warehouse/query-core/src/main/java/datawave/query/language/parser/QueryParser.java b/warehouse/query-core/src/main/java/datawave/query/language/parser/QueryParser.java index 7793be11c5c..cb92641476d 100644 --- a/warehouse/query-core/src/main/java/datawave/query/language/parser/QueryParser.java +++ b/warehouse/query-core/src/main/java/datawave/query/language/parser/QueryParser.java @@ -1,7 +1,5 @@ package datawave.query.language.parser; -import org.apache.lucene.queryparser.flexible.core.parser.SyntaxParser; - import datawave.query.language.tree.QueryNode; public interface QueryParser { diff --git a/warehouse/query-core/src/main/java/datawave/query/language/parser/jexl/JexlControlledQueryParser.java b/warehouse/query-core/src/main/java/datawave/query/language/parser/jexl/JexlControlledQueryParser.java index f450a286839..6b21794f582 100644 --- a/warehouse/query-core/src/main/java/datawave/query/language/parser/jexl/JexlControlledQueryParser.java +++ b/warehouse/query-core/src/main/java/datawave/query/language/parser/jexl/JexlControlledQueryParser.java @@ -10,7 +10,6 @@ import org.apache.commons.jexl3.parser.ASTIdentifier; import org.apache.commons.jexl3.parser.JexlNode; import org.apache.commons.lang.StringUtils; -import org.apache.lucene.queryparser.flexible.core.parser.SyntaxParser; import datawave.query.jexl.JexlASTHelper; import datawave.query.language.parser.ParseException; diff --git a/warehouse/query-core/src/main/java/datawave/query/tables/ShardQueryLogic.java b/warehouse/query-core/src/main/java/datawave/query/tables/ShardQueryLogic.java index 6f148ee48c1..401836408da 100644 --- a/warehouse/query-core/src/main/java/datawave/query/tables/ShardQueryLogic.java +++ b/warehouse/query-core/src/main/java/datawave/query/tables/ShardQueryLogic.java @@ -1398,22 +1398,18 @@ public void close() { } @Override - public Object validateQuery(AccumuloClient client, Query settings, Set<Authorizations> auths, boolean expandFields, boolean expandValues) throws Exception { + public Object validateQuery(AccumuloClient client, Query settings, Set<Authorizations> auths) throws Exception { this.config = ShardQueryConfiguration.create(this, settings); if (log.isTraceEnabled()) { log.trace("Initializing ShardQueryLogic for query validation: " + System.identityHashCode(this) + '(' + (this.getSettings() == null ? "empty" : this.getSettings().getId()) + ')'); } - // todo - maybe unnecessary, we could just return early if no rules configured. + // delegate to the super class if no validation rules configured. if (validationRules == null || validationRules.isEmpty()) { - throw new IllegalStateException("Query validation rules not configured."); + super.validateQuery(client, settings, auths); } - // todo - verify if we should allow these to be configured, or always stick to an established default. - this.config.setExpandFields(expandFields); - this.config.setExpandValues(expandValues); - // Set the connector and authorizations for the config object. config.setClient(client); config.setAuthorizations(auths); @@ -1527,8 +1523,6 @@ public Object validateQuery(AccumuloClient client, Query settings, Set<Authoriza config.setQueryTree(ShardQueryUtils.applyQueryModel(config.getQueryTree(), config, metadataHelper.getAllFields(config.getDatatypeFilter()), this.queryModel)); - // todo - should any other normalization steps be applied? - // Update the configurations with the target syntax JEXL and the jexl query string. Execute any remaining rules that expect to run against a JEXL query. validationConfig.setParsedQuery(config.getQueryTree()); validationConfig.setQueryString(JexlStringBuildingVisitor.buildQuery(config.getQueryTree())); diff --git a/warehouse/query-core/src/test/java/datawave/query/tables/ShardQueryLogicQueryValidationTest.java b/warehouse/query-core/src/test/java/datawave/query/tables/ShardQueryLogicQueryValidationTest.java index e3e344f7be6..92f034228ad 100644 --- a/warehouse/query-core/src/test/java/datawave/query/tables/ShardQueryLogicQueryValidationTest.java +++ b/warehouse/query-core/src/test/java/datawave/query/tables/ShardQueryLogicQueryValidationTest.java @@ -80,8 +80,6 @@ public class ShardQueryLogicQueryValidationTest { private String query; private Date startDate; private Date endDate; - private boolean expandFields; - private boolean expandValues; private final List<QueryRuleResult> expectedRuleResults = new ArrayList<>(); private Class<? extends Throwable> expectedExceptionType; @@ -120,8 +118,6 @@ public void setup() throws ParseException { this.deserializer = new KryoDocumentDeserializer(); this.startDate = dateFormat.parse("20091231"); this.endDate = dateFormat.parse("20150101"); - this.expandFields = true; - this.expandValues = false; } @After @@ -366,14 +362,6 @@ private void givenQueryParameter(String parameter, String value) { this.queryParameters.put(parameter, value); } - private void givenExpandFields(boolean expandFields) { - this.expandFields = expandFields; - } - - private void givenExpandValues(boolean expandValues) { - this.expandValues = expandValues; - } - private void expectRuleResult(QueryRuleResult ruleResult) { this.expectedRuleResults.add(ruleResult); } @@ -386,7 +374,7 @@ private void expectExceptionInResult(Class<? extends Throwable> type, String mes private void assertResult() throws Exception { AccumuloClient client = createClient(); Query settings = createSettings(); - QueryValidationResult actualResult = (QueryValidationResult) logic.validateQuery(client, settings, authSet, expandFields, expandValues); + QueryValidationResult actualResult = (QueryValidationResult) logic.validateQuery(client, settings, authSet); Assertions.assertThat(actualResult.getRuleResults()).isEqualTo(expectedRuleResults); if (expectedExceptionType == null) { diff --git a/web-services/query/src/main/java/datawave/webservice/query/runner/QueryExecutorBean.java b/web-services/query/src/main/java/datawave/webservice/query/runner/QueryExecutorBean.java index c8fd38f18ea..7e6212f5948 100644 --- a/web-services/query/src/main/java/datawave/webservice/query/runner/QueryExecutorBean.java +++ b/web-services/query/src/main/java/datawave/webservice/query/runner/QueryExecutorBean.java @@ -457,7 +457,7 @@ private QueryData setUserData(Principal p, QueryData qd) { * the logic name * @return QueryData */ - private QueryData validateQuery(String queryLogicName, MultivaluedMap<String,String> queryParameters, HttpHeaders httpHeaders) { + private QueryData validateQueryParameters(String queryLogicName, MultivaluedMap<String,String> queryParameters, HttpHeaders httpHeaders) { // Parameter 'logicName' is required and passed in prior to this call. Add to the queryParameters now. if (!queryParameters.containsKey(QueryParameters.QUERY_LOGIC_NAME)) { @@ -625,7 +625,7 @@ public GenericResponse<String> defineQuery(@Required("logicName") @PathParam("lo MultivaluedMap<String,String> queryParameters, @Context HttpHeaders httpHeaders) { CreateQuerySessionIDFilter.QUERY_ID.set(null); - QueryData qd = validateQuery(queryLogicName, queryParameters, httpHeaders); + QueryData qd = validateQueryParameters(queryLogicName, queryParameters, httpHeaders); GenericResponse<String> response = new GenericResponse<>(); @@ -689,7 +689,7 @@ public GenericResponse<String> createQuery(@Required("logicName") @PathParam("lo MultivaluedMap<String,String> queryParameters, @Context HttpHeaders httpHeaders) { CreateQuerySessionIDFilter.QUERY_ID.set(null); - QueryData qd = validateQuery(queryLogicName, queryParameters, httpHeaders); + QueryData qd = validateQueryParameters(queryLogicName, queryParameters, httpHeaders); GenericResponse<String> response = new GenericResponse<>(); @@ -852,7 +852,7 @@ public GenericResponse<String> createQuery(@Required("logicName") @PathParam("lo @Timed(name = "dw.query.planQuery", absolute = true) public GenericResponse<String> planQuery(@Required("logicName") @PathParam("logicName") String queryLogicName, MultivaluedMap<String,String> queryParameters) { - QueryData qd = validateQuery(queryLogicName, queryParameters, null); + QueryData qd = validateQueryParameters(queryLogicName, queryParameters, null); GenericResponse<String> response = new GenericResponse<>(); @@ -1002,7 +1002,7 @@ public GenericResponse<String> predictQuery(@Required("logicName") @PathParam("l CreateQuerySessionIDFilter.QUERY_ID.set(null); - QueryData qd = validateQuery(queryLogicName, queryParameters, null); + QueryData qd = validateQueryParameters(queryLogicName, queryParameters, null); GenericResponse<String> response = new GenericResponse<>(); @@ -3014,68 +3014,15 @@ private void updateQueryParams(Query q, String queryLogicName, String query, Dat @Timed(name = "dw.query.validateQuery", absolute = true) public QueryValidationResponse validateQuery(@Required("logicName") @PathParam("logicName") String queryLogicName, MultivaluedMap<String,String> queryParameters) { - QueryData queryData = validateQuery(queryLogicName, queryParameters, null); + QueryData queryData = validateQueryParameters(queryLogicName, queryParameters, null); QueryValidationResponse response = new QueryValidationResponse(); Query query = null; AccumuloClient client = null; AccumuloConnectionFactory.Priority priority; - RunningQuery runningQuery = null; try { - // by default we will expand the fields but not the values. - boolean expandFields = true; - boolean expandValues = false; - if (queryParameters.containsKey(EXPAND_FIELDS)) { - expandFields = Boolean.valueOf(queryParameters.getFirst(EXPAND_FIELDS)); - } - if (queryParameters.containsKey(EXPAND_VALUES)) { - expandValues = Boolean.valueOf(queryParameters.getFirst(EXPAND_VALUES)); - } - - AuditType auditType = queryData.logic.getAuditType(); - try { - // The query should be transient. - qp.setPersistenceMode(QueryPersistence.TRANSIENT); - Map<String,List<String>> optionalQueryParameters = qp.getUnknownParameters(MapUtils.toMultivaluedMap(queryParameters)); - query = persister.create(queryData.userDn, queryData.dnList, marking, queryLogicName, qp, MapUtils.toMultivaluedMap(optionalQueryParameters)); - auditType = queryData.logic.getAuditType(); - } finally { - queryParameters.add(PrivateAuditConstants.AUDIT_TYPE, auditType.name()); - - if (!auditType.equals(AuditType.NONE)) { - // audit the query before its executed. - try { - try { - List<String> selectors = queryData.logic.getSelectors(query); - if (selectors != null && !selectors.isEmpty()) { - queryParameters.put(PrivateAuditConstants.SELECTORS, selectors); - } - } catch (Exception e) { - log.error("Error accessing query selector", e); - } - // if the user didn't set an audit id, use the query id - if (!queryParameters.containsKey(AuditParameters.AUDIT_ID) && query != null) { - queryParameters.putSingle(AuditParameters.AUDIT_ID, query.getId().toString()); - } - auditor.audit(MapUtils.toMultiValueMap(queryParameters)); - } catch (IllegalArgumentException e) { - log.error("Error validating audit parameters", e); - BadRequestQueryException qe = new BadRequestQueryException(DatawaveErrorCode.MISSING_REQUIRED_PARAMETER, e); - response = new QueryValidationResponse(); - response.addException(qe); - throw new BadRequestException(qe, response); - } catch (Exception e) { - log.error("Error auditing query", e); - QueryException qe = new QueryException(DatawaveErrorCode.QUERY_AUDITING_ERROR, e); - response = new QueryValidationResponse(); - response.addException(qe); - throw qe; - } - } - } - priority = queryData.logic.getConnectionPriority(); Map<String,String> trackingMap = connectionFactory.getTrackingMap(Thread.currentThread().getStackTrace()); query.populateTrackingMap(trackingMap); @@ -3100,7 +3047,7 @@ public QueryValidationResponse validateQuery(@Required("logicName") @PathParam(" Set<Authorizations> calculatedAuths = WSAuthorizationsUtil.getDowngradedAuthorizations(qp.getAuths(), overallPrincipal, queryPrincipal); // Validate the query. - Object validationResult = queryData.logic.validateQuery(client, query, calculatedAuths, expandFields, expandValues); + Object validationResult = queryData.logic.validateQuery(client, query, calculatedAuths); // Convert the validation results to a response. Transformer<Object,QueryValidationResponse> responseTransformer = queryData.logic.getQueryValidationResponseTransformer(); response = responseTransformer.transform(validationResult);