[7.x][Transform] fix count when matching exact ids(#56544) (#56582)

fix count in get and get stats if explicit ids are given and ids might be
duplicated when configuration are stored in different index (versions).

fixes #56196
This commit is contained in:
Hendrik Muhs 2020-05-12 14:23:13 +02:00 committed by GitHub
parent 575cafb8da
commit a9425a0240
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 58 additions and 3 deletions

View File

@ -120,10 +120,16 @@ public abstract class AbstractTransportGetResourcesAction<Resource extends ToXCo
requiredMatches.filterMatchedIds(foundResourceIds); requiredMatches.filterMatchedIds(foundResourceIds);
if (requiredMatches.hasUnmatchedIds()) { if (requiredMatches.hasUnmatchedIds()) {
listener.onFailure(notFoundException(requiredMatches.unmatchedIdsString())); listener.onFailure(notFoundException(requiredMatches.unmatchedIdsString()));
} else {
// if only exact ids have been given, take the count from docs to avoid potential duplicates
// in versioned indexes (like transform)
if (requiredMatches.isOnlyExact()) {
listener.onResponse(new QueryPage<>(docs, docs.size(), getResultsField()));
} else { } else {
listener.onResponse(new QueryPage<>(docs, totalHitCount, getResultsField())); listener.onResponse(new QueryPage<>(docs, totalHitCount, getResultsField()));
} }
} }
}
@Override @Override

View File

@ -43,6 +43,7 @@ public final class ExpandedIdsMatcher {
} }
private final LinkedList<IdMatcher> requiredMatches; private final LinkedList<IdMatcher> requiredMatches;
private final boolean onlyExact;
/** /**
* Generate the list of required matches from the expressions in {@code tokens} * Generate the list of required matches from the expressions in {@code tokens}
@ -65,14 +66,19 @@ public final class ExpandedIdsMatcher {
// require something, anything to match // require something, anything to match
requiredMatches.add(new WildcardMatcher("*")); requiredMatches.add(new WildcardMatcher("*"));
} }
onlyExact = false;
return; return;
} }
boolean atLeastOneWildcard = false;
if (allowNoMatchForWildcards) { if (allowNoMatchForWildcards) {
// matches are not required for wildcards but // matches are not required for wildcards but
// specific job Ids are // specific job Ids are
for (String token : tokens) { for (String token : tokens) {
if (Regex.isSimpleMatchPattern(token) == false) { if (Regex.isSimpleMatchPattern(token)) {
atLeastOneWildcard = true;
} else {
requiredMatches.add(new EqualsIdMatcher(token)); requiredMatches.add(new EqualsIdMatcher(token));
} }
} }
@ -81,11 +87,13 @@ public final class ExpandedIdsMatcher {
for (String token : tokens) { for (String token : tokens) {
if (Regex.isSimpleMatchPattern(token)) { if (Regex.isSimpleMatchPattern(token)) {
requiredMatches.add(new WildcardMatcher(token)); requiredMatches.add(new WildcardMatcher(token));
atLeastOneWildcard = true;
} else { } else {
requiredMatches.add(new EqualsIdMatcher(token)); requiredMatches.add(new EqualsIdMatcher(token));
} }
} }
} }
onlyExact = atLeastOneWildcard == false;
} }
/** /**
@ -119,6 +127,14 @@ public final class ExpandedIdsMatcher {
return requiredMatches.stream().map(IdMatcher::getId).collect(Collectors.joining(",")); return requiredMatches.stream().map(IdMatcher::getId).collect(Collectors.joining(","));
} }
/**
* Whether ids are based on exact matchers or at least one contains a wildcard.
*
* @return true if only exact matches, false if at least one id contains a wildcard
*/
public boolean isOnlyExact() {
return onlyExact;
}
private abstract static class IdMatcher { private abstract static class IdMatcher {
protected final String id; protected final String id;

View File

@ -25,49 +25,58 @@ public class ExpandedIdsMatcherTests extends ESTestCase {
requiredMatches.filterMatchedIds(Collections.singletonList("foo")); requiredMatches.filterMatchedIds(Collections.singletonList("foo"));
assertFalse(requiredMatches.hasUnmatchedIds()); assertFalse(requiredMatches.hasUnmatchedIds());
assertThat(requiredMatches.unmatchedIds(), empty()); assertThat(requiredMatches.unmatchedIds(), empty());
assertFalse(requiredMatches.isOnlyExact());
requiredMatches = new ExpandedIdsMatcher(ExpandedIdsMatcher.tokenizeExpression(""), false); requiredMatches = new ExpandedIdsMatcher(ExpandedIdsMatcher.tokenizeExpression(""), false);
assertThat(requiredMatches.unmatchedIds(), hasSize(1)); assertThat(requiredMatches.unmatchedIds(), hasSize(1));
requiredMatches.filterMatchedIds(Collections.singletonList("foo")); requiredMatches.filterMatchedIds(Collections.singletonList("foo"));
assertThat(requiredMatches.unmatchedIds(), empty()); assertThat(requiredMatches.unmatchedIds(), empty());
assertFalse(requiredMatches.isOnlyExact());
requiredMatches = new ExpandedIdsMatcher(ExpandedIdsMatcher.tokenizeExpression(null), false); requiredMatches = new ExpandedIdsMatcher(ExpandedIdsMatcher.tokenizeExpression(null), false);
assertThat(requiredMatches.unmatchedIds(), hasSize(1)); assertThat(requiredMatches.unmatchedIds(), hasSize(1));
requiredMatches.filterMatchedIds(Collections.singletonList("foo")); requiredMatches.filterMatchedIds(Collections.singletonList("foo"));
assertThat(requiredMatches.unmatchedIds(), empty()); assertThat(requiredMatches.unmatchedIds(), empty());
assertFalse(requiredMatches.isOnlyExact());
requiredMatches = new ExpandedIdsMatcher(ExpandedIdsMatcher.tokenizeExpression(null), false); requiredMatches = new ExpandedIdsMatcher(ExpandedIdsMatcher.tokenizeExpression(null), false);
assertThat(requiredMatches.unmatchedIds(), hasSize(1)); assertThat(requiredMatches.unmatchedIds(), hasSize(1));
requiredMatches.filterMatchedIds(Collections.emptyList()); requiredMatches.filterMatchedIds(Collections.emptyList());
assertThat(requiredMatches.unmatchedIds(), hasSize(1)); assertThat(requiredMatches.unmatchedIds(), hasSize(1));
assertThat(requiredMatches.unmatchedIds().get(0), equalTo("*")); assertThat(requiredMatches.unmatchedIds().get(0), equalTo("*"));
assertFalse(requiredMatches.isOnlyExact());
requiredMatches = new ExpandedIdsMatcher(ExpandedIdsMatcher.tokenizeExpression("_all"), false); requiredMatches = new ExpandedIdsMatcher(ExpandedIdsMatcher.tokenizeExpression("_all"), false);
assertThat(requiredMatches.unmatchedIds(), hasSize(1)); assertThat(requiredMatches.unmatchedIds(), hasSize(1));
requiredMatches.filterMatchedIds(Collections.singletonList("foo")); requiredMatches.filterMatchedIds(Collections.singletonList("foo"));
assertThat(requiredMatches.unmatchedIds(), empty()); assertThat(requiredMatches.unmatchedIds(), empty());
assertFalse(requiredMatches.isOnlyExact());
requiredMatches = new ExpandedIdsMatcher(new String[] {"foo*"}, false); requiredMatches = new ExpandedIdsMatcher(new String[] {"foo*"}, false);
assertThat(requiredMatches.unmatchedIds(), hasSize(1)); assertThat(requiredMatches.unmatchedIds(), hasSize(1));
requiredMatches.filterMatchedIds(Arrays.asList("foo1","foo2")); requiredMatches.filterMatchedIds(Arrays.asList("foo1","foo2"));
assertThat(requiredMatches.unmatchedIds(), empty()); assertThat(requiredMatches.unmatchedIds(), empty());
assertFalse(requiredMatches.isOnlyExact());
requiredMatches = new ExpandedIdsMatcher(new String[] {"foo*","bar"}, false); requiredMatches = new ExpandedIdsMatcher(new String[] {"foo*","bar"}, false);
assertThat(requiredMatches.unmatchedIds(), hasSize(2)); assertThat(requiredMatches.unmatchedIds(), hasSize(2));
requiredMatches.filterMatchedIds(Arrays.asList("foo1","foo2")); requiredMatches.filterMatchedIds(Arrays.asList("foo1","foo2"));
assertThat(requiredMatches.unmatchedIds(), hasSize(1)); assertThat(requiredMatches.unmatchedIds(), hasSize(1));
assertEquals("bar", requiredMatches.unmatchedIds().get(0)); assertEquals("bar", requiredMatches.unmatchedIds().get(0));
assertFalse(requiredMatches.isOnlyExact());
requiredMatches = new ExpandedIdsMatcher(new String[] {"foo*","bar"}, false); requiredMatches = new ExpandedIdsMatcher(new String[] {"foo*","bar"}, false);
assertThat(requiredMatches.unmatchedIds(), hasSize(2)); assertThat(requiredMatches.unmatchedIds(), hasSize(2));
requiredMatches.filterMatchedIds(Arrays.asList("foo1","bar")); requiredMatches.filterMatchedIds(Arrays.asList("foo1","bar"));
assertFalse(requiredMatches.hasUnmatchedIds()); assertFalse(requiredMatches.hasUnmatchedIds());
assertFalse(requiredMatches.isOnlyExact());
requiredMatches = new ExpandedIdsMatcher(new String[] {"foo*","bar"}, false); requiredMatches = new ExpandedIdsMatcher(new String[] {"foo*","bar"}, false);
assertThat(requiredMatches.unmatchedIds(), hasSize(2)); assertThat(requiredMatches.unmatchedIds(), hasSize(2));
requiredMatches.filterMatchedIds(Collections.singletonList("bar")); requiredMatches.filterMatchedIds(Collections.singletonList("bar"));
assertThat(requiredMatches.unmatchedIds(), hasSize(1)); assertThat(requiredMatches.unmatchedIds(), hasSize(1));
assertEquals("foo*", requiredMatches.unmatchedIds().get(0)); assertEquals("foo*", requiredMatches.unmatchedIds().get(0));
assertFalse(requiredMatches.isOnlyExact());
requiredMatches = new ExpandedIdsMatcher(ExpandedIdsMatcher.tokenizeExpression("foo,bar,baz,wild*"), false); requiredMatches = new ExpandedIdsMatcher(ExpandedIdsMatcher.tokenizeExpression("foo,bar,baz,wild*"), false);
assertThat(requiredMatches.unmatchedIds(), hasSize(4)); assertThat(requiredMatches.unmatchedIds(), hasSize(4));
@ -75,6 +84,14 @@ public class ExpandedIdsMatcherTests extends ESTestCase {
assertThat(requiredMatches.unmatchedIds(), hasSize(2)); assertThat(requiredMatches.unmatchedIds(), hasSize(2));
assertThat(requiredMatches.unmatchedIds().get(0), is(oneOf("bar", "wild*"))); assertThat(requiredMatches.unmatchedIds().get(0), is(oneOf("bar", "wild*")));
assertThat(requiredMatches.unmatchedIds().get(1), is(oneOf("bar", "wild*"))); assertThat(requiredMatches.unmatchedIds().get(1), is(oneOf("bar", "wild*")));
assertFalse(requiredMatches.isOnlyExact());
requiredMatches = new ExpandedIdsMatcher(new String[] {"foo","bar"}, false);
assertThat(requiredMatches.unmatchedIds(), hasSize(2));
requiredMatches.filterMatchedIds(Collections.singletonList("bar"));
assertThat(requiredMatches.unmatchedIds(), hasSize(1));
assertEquals("foo", requiredMatches.unmatchedIds().get(0));
assertTrue(requiredMatches.isOnlyExact());
} }
public void testMatchingResourceIds_allowNoMatch() { public void testMatchingResourceIds_allowNoMatch() {
@ -84,6 +101,7 @@ public class ExpandedIdsMatcherTests extends ESTestCase {
requiredMatches.filterMatchedIds(Collections.emptyList()); requiredMatches.filterMatchedIds(Collections.emptyList());
assertThat(requiredMatches.unmatchedIds(), empty()); assertThat(requiredMatches.unmatchedIds(), empty());
assertFalse(requiredMatches.hasUnmatchedIds()); assertFalse(requiredMatches.hasUnmatchedIds());
assertFalse(requiredMatches.isOnlyExact());
requiredMatches = new ExpandedIdsMatcher(new String[] {"foo*","bar"}, true); requiredMatches = new ExpandedIdsMatcher(new String[] {"foo*","bar"}, true);
assertThat(requiredMatches.unmatchedIds(), hasSize(1)); assertThat(requiredMatches.unmatchedIds(), hasSize(1));
@ -91,11 +109,20 @@ public class ExpandedIdsMatcherTests extends ESTestCase {
requiredMatches.filterMatchedIds(Collections.singletonList("bar")); requiredMatches.filterMatchedIds(Collections.singletonList("bar"));
assertThat(requiredMatches.unmatchedIds(), empty()); assertThat(requiredMatches.unmatchedIds(), empty());
assertFalse(requiredMatches.hasUnmatchedIds()); assertFalse(requiredMatches.hasUnmatchedIds());
assertFalse(requiredMatches.isOnlyExact());
requiredMatches = new ExpandedIdsMatcher(new String[] {"foo*","bar"}, true); requiredMatches = new ExpandedIdsMatcher(new String[] {"foo*","bar"}, true);
assertThat(requiredMatches.unmatchedIds(), hasSize(1)); assertThat(requiredMatches.unmatchedIds(), hasSize(1));
requiredMatches.filterMatchedIds(Collections.emptyList()); requiredMatches.filterMatchedIds(Collections.emptyList());
assertThat(requiredMatches.unmatchedIds(), hasSize(1)); assertThat(requiredMatches.unmatchedIds(), hasSize(1));
assertEquals("bar", requiredMatches.unmatchedIds().get(0)); assertEquals("bar", requiredMatches.unmatchedIds().get(0));
assertFalse(requiredMatches.isOnlyExact());
requiredMatches = new ExpandedIdsMatcher(new String[] {"foo","bar"}, true);
assertThat(requiredMatches.unmatchedIds(), hasSize(2));
requiredMatches.filterMatchedIds(Collections.singletonList("bar"));
assertThat(requiredMatches.unmatchedIds(), hasSize(1));
assertEquals("foo", requiredMatches.unmatchedIds().get(0));
assertTrue(requiredMatches.isOnlyExact());
} }
} }

View File

@ -453,7 +453,13 @@ public class IndexBasedTransformConfigManager implements TransformConfigManager
); );
return; return;
} }
// if only exact ids have been given, take the count from docs to avoid potential duplicates
// in versioned indexes (like transform)
if (requiredMatches.isOnlyExact()) {
foundIdsListener.onResponse(new Tuple<>((long) ids.size(), new ArrayList<>(ids)));
} else {
foundIdsListener.onResponse(new Tuple<>(totalHits, new ArrayList<>(ids))); foundIdsListener.onResponse(new Tuple<>(totalHits, new ArrayList<>(ids)));
}
}, foundIdsListener::onFailure), }, foundIdsListener::onFailure),
client::search client::search
); );