Skip to content

Refactor FieldCapabilities creation by adding a proper builder object #121310

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Mar 5, 2025
Merged

Refactor FieldCapabilities creation by adding a proper builder object #121310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ff82a3b
FieldCapabilitiesBuilder
GalLalouche Jan 30, 2025
8acb415
Revert change to non-test class
GalLalouche Jan 30, 2025
085ae87
Move builder to test framework module
GalLalouche Jan 30, 2025
f2ca180
Merge branch 'main' into tests/field_caps_refactor
GalLalouche Mar 2, 2025
a1c5eaf
Merge branch 'main' into tests/field_caps_refactor
GalLalouche Mar 3, 2025
923a924
Code review fixes.
GalLalouche Mar 2, 2025
d5bd523
Fix test, some more defensive copying.
GalLalouche Mar 3, 2025
04c3bf9
Fix test
GalLalouche Mar 3, 2025
2723656
Merge branch 'main' into tests/field_caps_refactor
GalLalouche Mar 5, 2025
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
26 changes: 9 additions & 17 deletions ...extras/src/test/java/org/elasticsearch/index/mapper/extras/FieldCapsRankFeatureTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
package org.elasticsearch.index.mapper.extras;

import org.elasticsearch.action.fieldcaps.FieldCapabilities;
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesBuilder;
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse;
import org.elasticsearch.action.support.ActiveShardCount;
import org.elasticsearch.plugins.Plugin;
Expand All @@ -19,7 +20,6 @@

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Map;

import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
Expand Down Expand Up @@ -59,10 +59,7 @@ public void testRankFeatureInIndex() {
Map<String, FieldCapabilities> fooRankField = response.getField("fooRank");
assertEquals(1, fooRankField.size());
assertThat(fooRankField, Matchers.hasKey("rank_feature"));
assertEquals(
new FieldCapabilities("fooRank", "rank_feature", false, true, false, null, null, null, Collections.emptyMap()),
fooRankField.get("rank_feature")
);
assertEquals(fieldCapabilities("fooRank"), fooRankField.get("rank_feature"));
}

public void testRankFeatureInIndexAfterRestart() throws Exception {
Expand All @@ -79,10 +76,7 @@ public void testRankFeatureInIndexAfterRestart() throws Exception {
Map<String, FieldCapabilities> fooRankField = response.getField("fooRank");
assertEquals(1, fooRankField.size());
assertThat(fooRankField, Matchers.hasKey("rank_feature"));
assertEquals(
new FieldCapabilities("fooRank", "rank_feature", false, true, false, null, null, null, Collections.emptyMap()),
fooRankField.get("rank_feature")
);
assertEquals(fieldCapabilities("fooRank"), fooRankField.get("rank_feature"));
}

public void testAllRankFeatureReturnedIfOneIsPresent() {
Expand All @@ -98,18 +92,16 @@ public void testAllRankFeatureReturnedIfOneIsPresent() {
Map<String, FieldCapabilities> fooRankField = response.getField("fooRank");
assertEquals(1, fooRankField.size());
assertThat(fooRankField, Matchers.hasKey("rank_feature"));
assertEquals(
new FieldCapabilities("fooRank", "rank_feature", false, true, false, null, null, null, Collections.emptyMap()),
fooRankField.get("rank_feature")
);
assertEquals(fieldCapabilities("fooRank"), fooRankField.get("rank_feature"));
assertThat(response.get(), Matchers.hasKey("barRank"));
// Check the capabilities for the 'barRank' field.
Map<String, FieldCapabilities> barRankField = response.getField("barRank");
assertEquals(1, barRankField.size());
assertThat(barRankField, Matchers.hasKey("rank_feature"));
assertEquals(
new FieldCapabilities("barRank", "rank_feature", false, true, false, null, null, null, Collections.emptyMap()),
barRankField.get("rank_feature")
);
assertEquals(fieldCapabilities("barRank"), barRankField.get("rank_feature"));
}

private static FieldCapabilities fieldCapabilities(String fieldName) {
return new FieldCapabilitiesBuilder(fieldName, "rank_feature").isAggregatable(false).build();
}
}
52 changes: 9 additions & 43 deletions .../src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.action.admin.indices.close.CloseIndexRequest;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
import org.elasticsearch.action.fieldcaps.FieldCapabilities;
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesBuilder;
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesFailure;
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesRequest;
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse;
Expand Down Expand Up @@ -219,24 +220,11 @@ public void testFieldAlias() {
assertEquals(2, distance.size());

assertTrue(distance.containsKey("double"));
assertEquals(
new FieldCapabilities(
"distance",
"double",
false,
true,
true,
new String[] { "old_index" },
null,
null,
Collections.emptyMap()
),
distance.get("double")
);
assertEquals(new FieldCapabilitiesBuilder("distance", "double").indices("old_index").build(), distance.get("double"));

assertTrue(distance.containsKey("text"));
assertEquals(
new FieldCapabilities("distance", "text", false, true, false, new String[] { "new_index" }, null, null, Collections.emptyMap()),
new FieldCapabilitiesBuilder("distance", "text").isAggregatable(false).indices("new_index").build(),
distance.get("text")
);

Expand All @@ -245,10 +233,7 @@ public void testFieldAlias() {
assertEquals(1, routeLength.size());

assertTrue(routeLength.containsKey("double"));
assertEquals(
new FieldCapabilities("route_length_miles", "double", false, true, true, null, null, null, Collections.emptyMap()),
routeLength.get("double")
);
assertEquals(new FieldCapabilitiesBuilder("route_length_miles", "double").build(), routeLength.get("double"));
}

public void testFieldAliasWithWildcard() {
Expand Down Expand Up @@ -284,35 +269,19 @@ public void testWithUnmapped() {
assertEquals(2, oldField.size());

assertTrue(oldField.containsKey("long"));
assertEquals(
new FieldCapabilities("old_field", "long", false, true, true, new String[] { "old_index" }, null, null, Collections.emptyMap()),
oldField.get("long")
);
assertEquals(new FieldCapabilitiesBuilder("old_field", "long").indices("old_index").build(), oldField.get("long"));

assertTrue(oldField.containsKey("unmapped"));
assertEquals(
new FieldCapabilities(
"old_field",
"unmapped",
false,
false,
false,
new String[] { "new_index" },
null,
null,
Collections.emptyMap()
),
new FieldCapabilitiesBuilder("old_field", "unmapped").isSearchable(false).isAggregatable(false).indices("new_index").build(),
oldField.get("unmapped")
);

Map<String, FieldCapabilities> newField = response.getField("new_field");
assertEquals(1, newField.size());

assertTrue(newField.containsKey("long"));
assertEquals(
new FieldCapabilities("new_field", "long", false, true, true, null, null, null, Collections.emptyMap()),
newField.get("long")
);
assertEquals(new FieldCapabilitiesBuilder("new_field", "long").build(), newField.get("long"));
}

public void testWithIndexAlias() {
Expand Down Expand Up @@ -431,18 +400,15 @@ public void testMetadataFields() {

assertTrue(idField.containsKey("_id"));
assertEquals(
new FieldCapabilities("_id", "_id", true, true, false, null, null, null, Collections.emptyMap()),
new FieldCapabilitiesBuilder("_id", "_id").isMetadataField(true).isAggregatable(false).build(),
idField.get("_id")
);

Map<String, FieldCapabilities> testField = response.getField("_test");
assertEquals(1, testField.size());

assertTrue(testField.containsKey("keyword"));
assertEquals(
new FieldCapabilities("_test", "keyword", true, true, true, null, null, null, Collections.emptyMap()),
testField.get("keyword")
);
assertEquals(new FieldCapabilitiesBuilder("_test", "keyword").isMetadataField(true).build(), testField.get("keyword"));
}
}

Expand Down
89 changes: 21 additions & 68 deletions ...c/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapsHasValueTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import org.elasticsearch.action.DocWriteResponse;
import org.elasticsearch.action.fieldcaps.FieldCapabilities;
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesBuilder;
import org.elasticsearch.action.fieldcaps.FieldCapabilitiesResponse;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.support.ActiveShardCount;
Expand Down Expand Up @@ -76,10 +77,7 @@ public void testOnlyFieldsWithValueInIndex() {
Map<String, FieldCapabilities> fooField = response.getField("foo");
assertEquals(1, fooField.size());
assertThat(fooField, Matchers.hasKey("text"));
assertEquals(
new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
fooField.get("text")
);
assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
}

public void testOnlyFieldsWithValueInAlias() {
Expand All @@ -94,10 +92,7 @@ public void testOnlyFieldsWithValueInAlias() {
Map<String, FieldCapabilities> fooField = response.getField("foo");
assertEquals(1, fooField.size());
assertThat(fooField, Matchers.hasKey("text"));
assertEquals(
new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
fooField.get("text")
);
assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
}

public void testOnlyFieldsWithValueInSpecifiedIndex() {
Expand All @@ -112,10 +107,7 @@ public void testOnlyFieldsWithValueInSpecifiedIndex() {
Map<String, FieldCapabilities> fooField = response.getField("foo");
assertEquals(1, fooField.size());
assertThat(fooField, Matchers.hasKey("text"));
assertEquals(
new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
fooField.get("text")
);
assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
}

public void testOnlyFieldsWithValueInSpecifiedAlias() {
Expand All @@ -130,10 +122,7 @@ public void testOnlyFieldsWithValueInSpecifiedAlias() {
Map<String, FieldCapabilities> fooField = response.getField("foo");
assertEquals(1, fooField.size());
assertThat(fooField, Matchers.hasKey("text"));
assertEquals(
new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
fooField.get("text")
);
assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
}

public void testFieldsWithValueAfterUpdate() {
Expand All @@ -150,18 +139,12 @@ public void testFieldsWithValueAfterUpdate() {
Map<String, FieldCapabilities> fooField = response.getField("foo");
assertEquals(1, fooField.size());
assertThat(fooField, Matchers.hasKey("text"));
assertEquals(
new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
fooField.get("text")
);
assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
// Check the capabilities for the 'bar' field.
Map<String, FieldCapabilities> barField = response.getField("bar");
assertEquals(1, barField.size());
assertThat(barField, Matchers.hasKey("keyword"));
assertEquals(
new FieldCapabilities("bar", "keyword", false, true, true, null, null, null, Collections.emptyMap()),
barField.get("keyword")
);
assertEquals(new FieldCapabilitiesBuilder("bar", "keyword").build(), barField.get("keyword"));
}

public void testOnlyFieldsWithValueAfterNodesRestart() throws Exception {
Expand All @@ -177,10 +160,7 @@ public void testOnlyFieldsWithValueAfterNodesRestart() throws Exception {
Map<String, FieldCapabilities> fooField = response.getField("foo");
assertEquals(1, fooField.size());
assertThat(fooField, Matchers.hasKey("text"));
assertEquals(
new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
fooField.get("text")
);
assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
}

public void testFieldsAndAliasWithValue() {
Expand All @@ -198,26 +178,17 @@ public void testFieldsAndAliasWithValue() {
Map<String, FieldCapabilities> fooField = response.getField("foo");
assertEquals(1, fooField.size());
assertThat(fooField, Matchers.hasKey("text"));
assertEquals(
new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
fooField.get("text")
);
assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
// Check the capabilities for the 'bar' field.
Map<String, FieldCapabilities> barField = response.getField("bar");
assertEquals(1, barField.size());
assertThat(barField, Matchers.hasKey("keyword"));
assertEquals(
new FieldCapabilities("bar", "keyword", false, true, true, null, null, null, Collections.emptyMap()),
barField.get("keyword")
);
assertEquals(new FieldCapabilitiesBuilder("bar", "keyword").build(), barField.get("keyword"));
// Check the capabilities for the 'bar-alias' field.
Map<String, FieldCapabilities> barAlias = response.getField("bar-alias");
assertEquals(1, barAlias.size());
assertThat(barAlias, Matchers.hasKey("keyword"));
assertEquals(
new FieldCapabilities("bar-alias", "keyword", false, true, true, null, null, null, Collections.emptyMap()),
barAlias.get("keyword")
);
assertEquals(new FieldCapabilitiesBuilder("bar-alias", "keyword").build(), barAlias.get("keyword"));
}

public void testUnmappedFieldsWithValueAfterRestart() throws Exception {
Expand All @@ -238,7 +209,7 @@ public void testUnmappedFieldsWithValueAfterRestart() throws Exception {
assertEquals(2, unmappedField.size());
assertThat(unmappedField, Matchers.hasKey("text"));
assertEquals(
new FieldCapabilities("unmapped", "text", false, true, false, new String[] { INDEX1 }, null, null, Collections.emptyMap()),
new FieldCapabilitiesBuilder("unmapped", "text").isAggregatable(false).indices(INDEX1).build(),
unmappedField.get("text")
);
}
Expand All @@ -257,18 +228,12 @@ public void testTwoFieldsNameTwoIndices() {
Map<String, FieldCapabilities> fooField = response.getField("foo");
assertEquals(1, fooField.size());
assertThat(fooField, Matchers.hasKey("text"));
assertEquals(
new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
fooField.get("text")
);
assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
// Check the capabilities for the 'bar' field.
Map<String, FieldCapabilities> barField = response.getField("bar");
assertEquals(1, barField.size());
assertThat(barField, Matchers.hasKey("date"));
assertEquals(
new FieldCapabilities("bar", "date", false, true, true, null, null, null, Collections.emptyMap()),
barField.get("date")
);
assertEquals(new FieldCapabilitiesBuilder("bar", "date").build(), barField.get("date"));
}

public void testSameFieldNameTwoIndices() {
Expand All @@ -284,15 +249,9 @@ public void testSameFieldNameTwoIndices() {
Map<String, FieldCapabilities> barField = response.getField("bar");
assertEquals(2, barField.size());
assertThat(barField, Matchers.hasKey("keyword"));
assertEquals(
new FieldCapabilities("bar", "keyword", false, true, true, new String[] { INDEX1 }, null, null, Collections.emptyMap()),
barField.get("keyword")
);
assertEquals(new FieldCapabilitiesBuilder("bar", "keyword").indices(INDEX1).build(), barField.get("keyword"));
assertThat(barField, Matchers.hasKey("date"));
assertEquals(
new FieldCapabilities("bar", "date", false, true, true, new String[] { INDEX2 }, null, null, Collections.emptyMap()),
barField.get("date")
);
assertEquals(new FieldCapabilitiesBuilder("bar", "date").indices(INDEX2).build(), barField.get("date"));
}

public void testDeletedDocsReturned() {
Expand All @@ -311,10 +270,7 @@ public void testDeletedDocsReturned() {
Map<String, FieldCapabilities> fooField = response.getField("foo");
assertEquals(1, fooField.size());
assertThat(fooField, Matchers.hasKey("text"));
assertEquals(
new FieldCapabilities("foo", "text", false, true, false, null, null, null, Collections.emptyMap()),
fooField.get("text")
);
assertEquals(new FieldCapabilitiesBuilder("foo", "text").isAggregatable(false).build(), fooField.get("text"));
}

public void testNoNestedFieldsInEmptyIndex() {
Expand All @@ -339,15 +295,15 @@ public void testNestedFields() {
assertEquals(1, nestedTypeField.size());
assertThat(nestedTypeField, Matchers.hasKey("nested"));
assertEquals(
new FieldCapabilities("nested_type", "nested", false, false, false, null, null, null, Collections.emptyMap()),
new FieldCapabilitiesBuilder("nested_type", "nested").isSearchable(false).isAggregatable(false).build(),
nestedTypeField.get("nested")
);
// Check the capabilities for the 'nested_type.nested_field' field.
Map<String, FieldCapabilities> nestedTypeNestedField = response.getField("nested_type.nested_field");
assertEquals(1, nestedTypeNestedField.size());
assertThat(nestedTypeNestedField, Matchers.hasKey("text"));
assertEquals(
new FieldCapabilities("nested_type.nested_field", "text", false, true, false, null, null, null, Collections.emptyMap()),
new FieldCapabilitiesBuilder("nested_type.nested_field", "text").isAggregatable(false).build(),
nestedTypeNestedField.get("text")
);
}
Expand All @@ -374,17 +330,14 @@ public void testObjectFields() {
assertEquals(1, objectTypeField.size());
assertThat(objectTypeField, Matchers.hasKey("object"));
assertEquals(
new FieldCapabilities("object", "object", false, false, false, null, null, null, Collections.emptyMap()),
new FieldCapabilitiesBuilder("object", "object").isSearchable(false).isAggregatable(false).build(),
objectTypeField.get("object")
);
// Check the capabilities for the 'object.sub_field' field.
Map<String, FieldCapabilities> objectSubfield = response.getField("object.sub_field");
assertEquals(1, objectSubfield.size());
assertThat(objectSubfield, Matchers.hasKey("keyword"));
assertEquals(
new FieldCapabilities("object.sub_field", "keyword", false, true, true, null, null, null, Collections.emptyMap()),
objectSubfield.get("keyword")
);
assertEquals(new FieldCapabilitiesBuilder("object.sub_field", "keyword").build(), objectSubfield.get("keyword"));
}

public void testWithIndexFilter() throws InterruptedException {
Expand Down
Loading