Skip to content

Commit

Permalink
Another review iteration
Browse files Browse the repository at this point in the history
  • Loading branch information
Christoph Büscher committed Sep 16, 2020
1 parent 67df1d3 commit ce51484
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 66 deletions.
2 changes: 1 addition & 1 deletion ...lugin/versionfield/src/main/java/org/elasticsearch/xpack/versionfield/VersionEncoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class VersionEncoder {
private static final char PRERELEASE_SEPARATOR = '-';
private static final char DOT_SEPARATOR = '.';
private static final char BUILD_SEPARATOR = '+';
private static final String ENCODED_EMPTY_STRING = new String(new String(new byte[] { NO_PRERELEASE_SEPARATOR_BYTE }, Charsets.UTF_8));
private static final String ENCODED_EMPTY_STRING = new String(new byte[] { NO_PRERELEASE_SEPARATOR_BYTE }, Charsets.UTF_8);

// Regex to test relaxed Semver Main Version validity. Allows for more or less than three main version parts
private static Pattern LEGAL_MAIN_VERSION_SEMVER = Pattern.compile("(0|[1-9]\\d*)(\\.(0|[1-9]\\d*))*");
Expand Down
22 changes: 14 additions & 8 deletions ...onfield/src/main/java/org/elasticsearch/xpack/versionfield/VersionFieldWildcardQuery.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,17 @@
*/
class VersionFieldWildcardQuery extends AutomatonQuery {

private static final Automaton OPTIONAL_NUMERIC_CHARPREFIX = Operations.optional(
Operations.concatenate(Automata.makeChar(VersionEncoder.NUMERIC_MARKER_BYTE), Automata.makeCharRange(0x80, 0xFF))
);

private static final Automaton OPTIONAL_RELEASE_SEPARATOR = Operations.optional(
Operations.union(
Automata.makeChar(VersionEncoder.PRERELEASE_SEPARATOR_BYTE),
Automata.makeChar(VersionEncoder.NO_PRERELEASE_SEPARATOR_BYTE)
)
);

private static final byte WILDCARD_STRING = '*';

private static final byte WILDCARD_CHAR = '?';
Expand All @@ -46,7 +57,8 @@ private static Automaton toAutomaton(Term wildcardquery) {
break;
case WILDCARD_CHAR:
// this should also match leading digits, which have optional leading numeric marker and length bytes
automata.add(optionalNumericCharPrefix());
automata.add(OPTIONAL_NUMERIC_CHARPREFIX);
automata.add(OPTIONAL_RELEASE_SEPARATOR);
automata.add(Automata.makeAnyChar());
break;

Expand Down Expand Up @@ -79,7 +91,7 @@ private static Automaton toAutomaton(Term wildcardquery) {
firstDigitInGroup = false;
}
if (firstDigitInGroup) {
automata.add(optionalNumericCharPrefix());
automata.add(OPTIONAL_NUMERIC_CHARPREFIX);
}
automata.add(Automata.makeChar(c));
break;
Expand All @@ -95,12 +107,6 @@ private static Automaton toAutomaton(Term wildcardquery) {
return Operations.concatenate(automata);
}

private static Automaton optionalNumericCharPrefix() {
return Operations.optional(
Operations.concatenate(Automata.makeChar(VersionEncoder.NUMERIC_MARKER_BYTE), Automata.makeCharRange(0x80, 0xFF))
);
}

@Override
public String toString(String field) {
StringBuilder buffer = new StringBuilder();
Expand Down
107 changes: 50 additions & 57 deletions ...sionfield/src/test/java/org/elasticsearch/xpack/versionfield/VersionStringFieldTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,30 +158,38 @@ public void testPrefixQuery() throws IOException {

public void testSort() throws IOException {
String indexName = setUpIndex("test");
// also adding some invalid versions that should be sorted after legal ones
client().prepareIndex(indexName).setSource(jsonBuilder().startObject().field("version", "1.2.3alpha").endObject()).get();
client().prepareIndex(indexName).setSource(jsonBuilder().startObject().field("version", "1.3.567#12").endObject()).get();
client().admin().indices().prepareRefresh(indexName).get();

// sort based on version field
SearchResponse response = client().prepareSearch(indexName)
.setQuery(QueryBuilders.matchAllQuery())
.addSort("version", SortOrder.DESC)
.get();
assertEquals(6, response.getHits().getTotalHits().value);
assertEquals(8, response.getHits().getTotalHits().value);
SearchHit[] hits = response.getHits().getHits();
assertEquals("21.11.0", hits[0].getSortValues()[0]);
assertEquals("11.1.0", hits[1].getSortValues()[0]);
assertEquals("2.1.0", hits[2].getSortValues()[0]);
assertEquals("2.1.0-alpha.beta", hits[3].getSortValues()[0]);
assertEquals("1.3.0+build.1234567", hits[4].getSortValues()[0]);
assertEquals("1.0.0", hits[5].getSortValues()[0]);
assertEquals("1.3.567#12", hits[0].getSortValues()[0]);
assertEquals("1.2.3alpha", hits[1].getSortValues()[0]);
assertEquals("21.11.0", hits[2].getSortValues()[0]);
assertEquals("11.1.0", hits[3].getSortValues()[0]);
assertEquals("2.1.0", hits[4].getSortValues()[0]);
assertEquals("2.1.0-alpha.beta", hits[5].getSortValues()[0]);
assertEquals("1.3.0+build.1234567", hits[6].getSortValues()[0]);
assertEquals("1.0.0", hits[7].getSortValues()[0]);

response = client().prepareSearch(indexName).setQuery(QueryBuilders.matchAllQuery()).addSort("version", SortOrder.ASC).get();
assertEquals(6, response.getHits().getTotalHits().value);
assertEquals(8, response.getHits().getTotalHits().value);
hits = response.getHits().getHits();
assertEquals("1.0.0", hits[0].getSortValues()[0]);
assertEquals("1.3.0+build.1234567", hits[1].getSortValues()[0]);
assertEquals("2.1.0-alpha.beta", hits[2].getSortValues()[0]);
assertEquals("2.1.0", hits[3].getSortValues()[0]);
assertEquals("11.1.0", hits[4].getSortValues()[0]);
assertEquals("21.11.0", hits[5].getSortValues()[0]);
assertEquals("1.2.3alpha", hits[6].getSortValues()[0]);
assertEquals("1.3.567#12", hits[7].getSortValues()[0]);
}

public void testRegexQuery() throws Exception {
Expand Down Expand Up @@ -280,58 +288,43 @@ public void testWildcardQuery() throws Exception {
);
ensureGreen(indexName);

client().prepareIndex(indexName)
.setId("1")
.setSource(jsonBuilder().startObject().field("version", "1.0.0-alpha.2.1.0-rc.1").endObject())
.get();
client().prepareIndex(indexName)
.setId("2")
.setSource(jsonBuilder().startObject().field("version", "1.3.0+build.1234567").endObject())
.get();
client().prepareIndex(indexName)
.setId("3")
.setSource(jsonBuilder().startObject().field("version", "2.1.0-alpha.beta").endObject())
.get();
client().prepareIndex(indexName).setId("4").setSource(jsonBuilder().startObject().field("version", "2.1.0").endObject()).get();
client().prepareIndex(indexName).setId("5").setSource(jsonBuilder().startObject().field("version", "2.33.0").endObject()).get();
for (String version : List.of(
"1.0.0-alpha.2.1.0-rc.1",
"1.3.0+build.1234567",
"2.1.0-alpha.beta",
"2.1.0",
"2.33.0",
"3.1.1-a",
"3.1.1+b",
"3.1.123"
)) {
client().prepareIndex(indexName).setSource(jsonBuilder().startObject().field("version", version).endObject()).get();
}
client().admin().indices().prepareRefresh(indexName).get();

// wildcard
SearchResponse response = client().prepareSearch(indexName).setQuery(QueryBuilders.wildcardQuery("version", "*alpha*")).get();
assertEquals(2, response.getHits().getTotalHits().value);

response = client().prepareSearch(indexName).setQuery(QueryBuilders.wildcardQuery("version", "*b*")).get();
assertEquals(2, response.getHits().getTotalHits().value);

response = client().prepareSearch(indexName).setQuery(QueryBuilders.wildcardQuery("version", "*bet*")).get();
assertEquals(1, response.getHits().getTotalHits().value);

response = client().prepareSearch(indexName).setQuery(QueryBuilders.wildcardQuery("version", "2.1*")).get();
assertEquals(2, response.getHits().getTotalHits().value);

response = client().prepareSearch(indexName).setQuery(QueryBuilders.wildcardQuery("version", "2.1.0-*")).get();
assertEquals(1, response.getHits().getTotalHits().value);

response = client().prepareSearch(indexName).setQuery(QueryBuilders.wildcardQuery("version", "*2.1.0-*")).get();
assertEquals(2, response.getHits().getTotalHits().value);

response = client().prepareSearch(indexName).setQuery(QueryBuilders.wildcardQuery("version", "*2.1.0*")).get();
assertEquals(3, response.getHits().getTotalHits().value);

response = client().prepareSearch(indexName).setQuery(QueryBuilders.wildcardQuery("version", "*2.?.0*")).get();
assertEquals(3, response.getHits().getTotalHits().value);

response = client().prepareSearch(indexName).setQuery(QueryBuilders.wildcardQuery("version", "*2.??.0*")).get();
assertEquals(1, response.getHits().getTotalHits().value);

response = client().prepareSearch(indexName).setQuery(QueryBuilders.wildcardQuery("version", "?.1.0")).get();
assertEquals(1, response.getHits().getTotalHits().value);

response = client().prepareSearch(indexName).setQuery(QueryBuilders.wildcardQuery("version", "*-*")).get();
assertEquals(2, response.getHits().getTotalHits().value);
checkWildcardQuery(indexName, "*alpha*", new String[] { "1.0.0-alpha.2.1.0-rc.1", "2.1.0-alpha.beta" });
checkWildcardQuery(indexName, "*b*", new String[] { "1.3.0+build.1234567", "2.1.0-alpha.beta", "3.1.1+b" });
checkWildcardQuery(indexName, "*bet*", new String[] { "2.1.0-alpha.beta" });
checkWildcardQuery(indexName, "2.1*", new String[] { "2.1.0-alpha.beta", "2.1.0" });
checkWildcardQuery(indexName, "2.1.0-*", new String[] { "2.1.0-alpha.beta" });
checkWildcardQuery(indexName, "*2.1.0-*", new String[] { "1.0.0-alpha.2.1.0-rc.1", "2.1.0-alpha.beta" });
checkWildcardQuery(indexName, "*2.1.0*", new String[] { "1.0.0-alpha.2.1.0-rc.1", "2.1.0-alpha.beta", "2.1.0" });
checkWildcardQuery(indexName, "*2.?.0*", new String[] { "1.0.0-alpha.2.1.0-rc.1", "2.1.0-alpha.beta", "2.1.0" });
checkWildcardQuery(indexName, "*2.??.0*", new String[] { "2.33.0" });
checkWildcardQuery(indexName, "?.1.0", new String[] { "2.1.0" });
checkWildcardQuery(indexName, "*-*", new String[] { "1.0.0-alpha.2.1.0-rc.1", "2.1.0-alpha.beta", "3.1.1-a" });
checkWildcardQuery(indexName, "1.3.0+b*", new String[] { "1.3.0+build.1234567" });
checkWildcardQuery(indexName, "3.1.1??", new String[] { "3.1.1-a", "3.1.1+b", "3.1.123" });
}

response = client().prepareSearch(indexName).setQuery(QueryBuilders.wildcardQuery("version", "1.3.0+b*")).get();
assertEquals(1, response.getHits().getTotalHits().value);
private void checkWildcardQuery(String indexName, String query, String... expectedResults) {
SearchResponse response = client().prepareSearch(indexName).setQuery(QueryBuilders.wildcardQuery("version", query)).get();
assertEquals(expectedResults.length, response.getHits().getTotalHits().value);
for (int i = 0; i < expectedResults.length; i++) {
String expected = expectedResults[i];
Object actual = response.getHits().getHits()[i].getSourceAsMap().get("version");
assertEquals("expected " + expected + " in position " + i + " but found " + actual, expected, actual);
}
}

/**
Expand Down

0 comments on commit ce51484

Please sign in to comment.