Skip to content
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

Fix lineage for FacetCheck #956

Merged
merged 3 commits into from
Oct 30, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
Expand Up @@ -76,11 +76,13 @@ public Object visit(ASTJexlScript node, Object data) {
// Only add that node if it actually has children
if (0 < newChild.jjtGetNumChildren()) {
newNode.jjtAddChild(newChild, newIndex);
newChild.jjtSetParent(newNode);
newIndex++;
}
} else {
// Otherwise, we want to add the child regardless
newNode.jjtAddChild(newChild, newIndex);
newChild.jjtSetParent(newNode);
newIndex++;
}
}
Expand Down Expand Up @@ -110,11 +112,13 @@ public Object visit(ASTOrNode node, Object data) {
// Only add that node if it actually has children
if (0 < newChild.jjtGetNumChildren()) {
newNode.jjtAddChild(newChild, newIndex);
newChild.jjtSetParent(newNode);
newIndex++;
}
} else {
// Otherwise, we want to add the child regardless
newNode.jjtAddChild(newChild, newIndex);
newChild.jjtSetParent(newNode);
newIndex++;
}
}
Expand Down Expand Up @@ -147,11 +151,13 @@ public Object visit(ASTAndNode node, Object data) {
// Only add that node if it actually has children
if (0 < newChild.jjtGetNumChildren()) {
newNode.jjtAddChild(newChild, newIndex);
newChild.jjtSetParent(newNode);
newIndex++;
}
} else {
// Otherwise, we want to add the child regardless
newNode.jjtAddChild(newChild, newIndex);
newChild.jjtSetParent(newNode);
newIndex++;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,32 @@
package datawave.query.tables.facets;

import java.text.MessageFormat;
import java.util.NoSuchElementException;

import datawave.query.jexl.JexlASTHelper;
import datawave.query.jexl.visitors.PrintingVisitor;
import com.google.common.collect.Multimap;
import datawave.query.Constants;
import datawave.query.config.ShardQueryConfiguration;
import datawave.query.exceptions.InvalidFieldIndexQueryFatalQueryException;
import datawave.query.jexl.JexlASTHelper;
import datawave.query.jexl.visitors.AllTermsIndexedVisitor;
import datawave.query.jexl.visitors.PrintingVisitor;
import datawave.query.util.MetadataHelper;
import datawave.webservice.query.exception.DatawaveErrorCode;
import datawave.webservice.query.exception.PreConditionFailedQueryException;

import org.apache.accumulo.core.client.TableNotFoundException;
import org.apache.commons.jexl2.parser.JexlNode;

import com.google.common.collect.Multimap;
import java.text.MessageFormat;
import java.util.NoSuchElementException;

/**
* This visitor examines a JEXL tree and verifies that it satisfies all of the following conditions:
* <ul>
* <li>The query contains no binary equality nodes that are comparing two literals.</li>
* <li>All terms are indexed, with the exception of {@value Constants#ANY_FIELD} or {@value Constants#NO_FIELD} terms.</li>
* <li>The query has at least one indexed term if it has {@value Constants#ANY_FIELD} or {@value Constants#NO_FIELD} terms.</li>
* <li>The query does not contain assignments, functions, or any of the following operators: &lt;, &lt;=, &gt;, &gt;=, =~, !~.</li>
* </ul>
*
* In the case where the tree fails to meet a condition, an exception will be thrown.
*/
public class FacetCheck extends AllTermsIndexedVisitor {

Multimap<String,String> facetMultimap;
Expand All @@ -33,14 +42,16 @@ public FacetCheck(ShardQueryConfiguration config, FacetedConfiguration facetConf

/**
* Determine, for a binary equality node, if the field name is indexed
*
*
* @param node
* the node to verify the indexed status of
* @param data
* @return
* the node data
* @return a copy of the node
*/
@Override
protected JexlNode equalityVisitor(JexlNode node, Object data) {
String fieldName = null;
String fieldName;
try {
fieldName = JexlASTHelper.getIdentifier(node);
} catch (NoSuchElementException e) {
Expand All @@ -61,7 +72,6 @@ protected JexlNode equalityVisitor(JexlNode node, Object data) {
}

return copy(node);

}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@
package datawave.query.tables.facets;

import com.google.common.collect.HashMultimap;
import com.google.common.collect.Multimap;
import com.google.common.collect.Sets;
import datawave.query.Constants;
import datawave.query.config.ShardQueryConfiguration;
import datawave.query.exceptions.DatawaveFatalQueryException;
import datawave.query.exceptions.EmptyUnfieldedTermExpansionException;
import datawave.query.jexl.JexlASTHelper;
import datawave.query.util.MetadataHelper;
import org.apache.accumulo.core.client.TableNotFoundException;
import org.apache.commons.jexl2.parser.ASTJexlScript;
import org.apache.commons.jexl2.parser.JexlNode;
import org.apache.commons.jexl2.parser.ParseException;
import org.junit.Before;
import org.junit.Test;

import java.util.Collections;
import java.util.Set;

import static org.easymock.EasyMock.anyObject;
import static org.easymock.EasyMock.anyString;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.getCurrentArguments;
import static org.easymock.EasyMock.mock;
import static org.easymock.EasyMock.replay;
import static org.junit.Assert.assertTrue;

public class FacetCheckTest {

private static final Set<String> indexedFields = Collections.unmodifiableSet(Sets.newHashSet("FOO", "FOO2", "FOO3"));

private FacetCheck facetCheck;

@Before
public void before() throws TableNotFoundException, IllegalAccessException, InstantiationException {
Multimap<String,String> facets = HashMultimap.create();
facets.put("FACET1", "VALUE");
facets.put("FACET2", "VALUE");
facets.put("FACET3", "VALUE");

ShardQueryConfiguration shardQueryConfiguration = mock(ShardQueryConfiguration.class);
FacetedConfiguration facetedConfiguration = mock(FacetedConfiguration.class);
expect(facetedConfiguration.getFacetMetadataTableName()).andReturn("facetMetadata");
MetadataHelper metadataHelper = mock(MetadataHelper.class);
expect(metadataHelper.isIndexed(anyString(), anyObject())).andAnswer(() -> indexedFields.contains(getCurrentArguments()[0]));
expect(metadataHelper.getFacets("facetMetadata")).andReturn(facets);

replay(shardQueryConfiguration, facetedConfiguration, metadataHelper);

facetCheck = new FacetCheck(shardQueryConfiguration, facetedConfiguration, metadataHelper);
}

@Test(expected = EmptyUnfieldedTermExpansionException.class)
public void testAnyFieldSingleTerm() throws ParseException {
String query = Constants.ANY_FIELD + " == 'bar'";
testVisitor(query);
}

@Test(expected = EmptyUnfieldedTermExpansionException.class)
public void testNoFieldSingleTerm() throws ParseException {
String query = Constants.NO_FIELD + " == 'bar'";
testVisitor(query);
}

@Test
public void testFacetedSingleTerm() throws ParseException {
String query = "FACET1 == 'bar'";
testVisitor(query);
}

@Test
public void testFacetedConjunction() throws ParseException {
String query = "FACET1 == 'bar' || FACET2 == 'bar'";
testVisitor(query);
}

@Test
public void testFacetedDisjunction() throws ParseException {
String query = "FACET1 == 'bar' && FACET2 == 'bar'";
testVisitor(query);
}

@Test(expected = EmptyUnfieldedTermExpansionException.class)
public void testAnyFieldsTerm() throws ParseException {
String query = Constants.ANY_FIELD + " == 'bar'";
testVisitor(query);
}

@Test(expected = EmptyUnfieldedTermExpansionException.class)
public void testNoFieldsTerm() throws ParseException {
String query = Constants.NO_FIELD + " == 'bar'";
testVisitor(query);
}

@Test
public void testAnyFieldAndFacetDisjunction() throws ParseException {
String query = Constants.ANY_FIELD + " == 'bar' && FACET2 == 'bar'";
testVisitor(query);
}

@Test
public void testNoFieldAndFacetDisjunction() throws ParseException {
String query = Constants.NO_FIELD + " == 'bar' && FACET2 == 'bar'";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testNonFacetedSingleTerm() throws ParseException {
String query = "BAR == 'foo'";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testNonFacetedConjunction() throws ParseException {
String query = "BAR == 'foo' && BAR2 == 'foo'";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testNonFacetedDisjunction() throws ParseException {
String query = "FOO == 'bar' || BAR == 'foo'";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testNonFacetedDoubleDisjunction() throws ParseException {
String query = "BAR == 'foo' || BAR2 == 'foo'";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testNonFacetedTripleDisjunction() throws ParseException {
String query = "FOO == 'bar' || BAR == 'foo' || BAR2 == 'foo'";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testFunction() throws ParseException {
String query = "FOO == 'bar' || content:phrase(termOffsetMap, 'bar', 'too')";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testOnlyFunction() throws ParseException {
String query = "content:phrase(termOffsetMap, 'bar', 'too')";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testFunctionOverIndexedField() throws ParseException {
String query = "content:phrase(termOffsetMap, FOO, 'bar', 'too')";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testRegex() throws ParseException {
String query = "FOO =~ 'bar.*'";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testRegexWithExtraTerm() throws ParseException {
String query = "FOO =~ 'bar.*' || FOO == 'bar'";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testNegatedRegex() throws ParseException {
String query = "FOO !~ 'bar.*'";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testNegatedRegexWithExtraTerm() throws ParseException {
String query = "FOO !~ 'bar.*' || FOO == 'bar'";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testLessThan() throws ParseException {
String query = "FOO < '+aE1'";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testLessThanWithExtraTerm() throws ParseException {
String query = "FOO < '+aE1' || FOO == 'bar'";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testLessThanEquals() throws ParseException {
String query = "FOO <= '+aE1'";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testLessThanEqualsWithExtraTerm() throws ParseException {
String query = "FOO <= '+aE1' || FOO == 'bar'";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testGreaterThan() throws ParseException {
String query = "FOO > '+aE1'";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testGreaterThanWithExtraTerm() throws ParseException {
String query = "FOO > '+aE1' || FOO == 'bar'";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testGreaterThanEquals() throws ParseException {
String query = "FOO >= '+aE1'";
testVisitor(query);
}

@Test(expected = DatawaveFatalQueryException.class)
public void testGreaterThanEqualsWithExtraTerm() throws ParseException {
String query = "FOO >= '+aE1' || FOO == 'bar'";
testVisitor(query);
}

private void testVisitor(String query) throws ParseException {
ASTJexlScript script = JexlASTHelper.parseJexlQuery(query);
JexlNode result = (JexlNode) script.jjtAccept(facetCheck, null);
assertTrue(JexlASTHelper.validateLineage(result, true));
}
}