Skip to content

Commit

Permalink
Suggested changes (#2656)
Browse files Browse the repository at this point in the history 
* Suggested changes

* Removed unused imports
  • Loading branch information
@ivakegg
ivakegg authored Dec 6, 2024
1 parent c64b9d2 commit 1c4c31e
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 92 deletions.
9 changes: 2 additions & 7 deletions core/query/src/main/java/datawave/core/query/logic/QueryLogic.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
*/
Expand Down
2 changes: 0 additions & 2 deletions warehouse/query-core/src/main/java/datawave/query/language/parser/QueryParser.java
View file
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
1 change: 0 additions & 1 deletion ...ery-core/src/main/java/datawave/query/language/parser/jexl/JexlControlledQueryParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 3 additions & 9 deletions warehouse/query-core/src/main/java/datawave/query/tables/ShardQueryLogic.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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()));
Expand Down
14 changes: 1 addition & 13 deletions ...se/query-core/src/test/java/datawave/query/tables/ShardQueryLogicQueryValidationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand All @@ -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) {
Expand Down
67 changes: 7 additions & 60 deletions web-services/query/src/main/java/datawave/webservice/query/runner/QueryExecutorBean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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<>();

Expand Down Expand Up @@ -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<>();

Expand Down Expand Up @@ -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<>();

Expand Down Expand Up @@ -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<>();

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit 1c4c31e

Please sign in to comment.