ElasticQueryBuilder: Search for empty yet present repeatable field value

Make sure to add such a change field with an empty string value to the
UpdateRequest (towards the Elasticsearch index) in the first place.

Elasticsearch does support fields with empty string values. The below
Issue was considering the opposite as a possibility. There is therefore
no need anymore to consider using a sentinel value as proposed in the
original Issue (below). This change aims at permanently solving that.

Remove the no longer needed FileWithNoExtensionInElasticPredicate class.
The latter, as well as the removed AbstractQueryChangesTest::byDirectory
test method implementation parts, lead to reverting to the previous
implementation for these tests. -Issue has the references and context.

Bug: Issue 10889
Change-Id: Iec126e4c2456e68d141eb0b8b55f03ede01dcb95
This commit is contained in:
Marco Miller
2019-10-08 17:56:01 -04:00
parent 83432a974e
commit b54caf7da5
5 changed files with 2 additions and 56 deletions

View File

@@ -145,7 +145,7 @@ public class ElasticQueryBuilder {
String name = p.getField().getName();
String value = p.getValue();
if (value.isEmpty()) {
if (!p.getField().isRepeatable() && value.isEmpty()) {
return new BoolQueryBuilder().mustNot(QueryBuilders.existsQuery(name));
} else if (p instanceof RegexPredicate) {
if (value.startsWith("^")) {

View File

@@ -40,11 +40,7 @@ public class UpdateRequest<V> extends BulkRequest {
for (Values<V> values : schema.buildFields(v)) {
String name = values.getField().getName();
if (values.getField().isRepeatable()) {
builder.field(
name,
Streams.stream(values.getValues())
.filter(e -> shouldAddElement(e))
.collect(toList()));
builder.field(name, Streams.stream(values.getValues()).collect(toList()));
} else {
Object element = Iterables.getOnlyElement(values.getValues(), "");
if (shouldAddElement(element)) {

View File

@@ -52,10 +52,6 @@ public class IndexType {
return type.equals(ELASTICSEARCH);
}
public static boolean isElasticsearch(String type) {
return type.toLowerCase().equals(ELASTICSEARCH);
}
@Override
public String toString() {
return type;

View File

@@ -33,7 +33,6 @@ import com.google.gerrit.exceptions.NotSignedInException;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.IndexType;
import com.google.gerrit.index.Schema;
import com.google.gerrit.index.SchemaUtil;
import com.google.gerrit.index.query.LimitPredicate;
@@ -736,9 +735,6 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
@Operator
public Predicate<ChangeData> extension(String ext) throws QueryParseException {
if (args.getSchema().hasField(ChangeField.EXTENSION)) {
if (ext.isEmpty() && IndexType.isElasticsearch(args.indexConfig.type())) {
return new FileWithNoExtensionInElasticPredicate();
}
return new FileExtensionPredicate(ext);
}
throw new QueryParseException("'extension' operator is not supported by change index version");
@@ -777,11 +773,6 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil
if (directory.startsWith("^")) {
return new RegexDirectoryPredicate(directory);
}
if (IndexType.isElasticsearch(args.indexConfig.type())
&& (directory.isEmpty() || directory.equals("/"))) {
return Predicate.any();
}
return new DirectoryPredicate(directory);
}
throw new QueryParseException("'directory' operator is not supported by change index version");

View File

@@ -1,37 +0,0 @@
// Copyright (C) 2019 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.query.change;
import com.google.gerrit.index.query.PostFilterPredicate;
import com.google.gerrit.server.index.change.ChangeField;
public class FileWithNoExtensionInElasticPredicate extends PostFilterPredicate<ChangeData> {
private static final String NO_EXT = "";
public FileWithNoExtensionInElasticPredicate() {
super(ChangeField.EXTENSION.getName(), NO_EXT);
}
@Override
public boolean match(ChangeData cd) {
return ChangeField.getExtensions(cd).contains(NO_EXT);
}
@Override
public int getCost() {
return 1;
}
}