Delegate toXContent logic from ClusterState to its member classes #48218 #52743
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -73,6 +73,7 @@ | |
| import java.util.Set; | ||
| import java.util.function.Function; | ||
|
|
||
| import static org.elasticsearch.cluster.metadata.MetaData.CONTEXT_MODE_PARAM; | ||
| import static org.elasticsearch.cluster.node.DiscoveryNodeFilters.IP_VALIDATOR; | ||
| import static org.elasticsearch.cluster.node.DiscoveryNodeFilters.OpType.AND; | ||
| import static org.elasticsearch.cluster.node.DiscoveryNodeFilters.OpType.OR; | ||
|
|
@@ -1186,48 +1187,85 @@ public IndexMetaData build() { | |
| } | ||
|
|
||
| public static void toXContent(IndexMetaData indexMetaData, XContentBuilder builder, ToXContent.Params params) throws IOException { | ||
| MetaData.XContentContext context = MetaData.XContentContext.valueOf(params.param(CONTEXT_MODE_PARAM, "API")); | ||
|
|
||
| builder.startObject(indexMetaData.getIndex().getName()); | ||
|
|
||
| builder.field(KEY_VERSION, indexMetaData.getVersion()); | ||
| builder.field(KEY_MAPPING_VERSION, indexMetaData.getMappingVersion()); | ||
| builder.field(KEY_SETTINGS_VERSION, indexMetaData.getSettingsVersion()); | ||
| builder.field(KEY_ALIASES_VERSION, indexMetaData.getAliasesVersion()); | ||
| builder.field(KEY_ROUTING_NUM_SHARDS, indexMetaData.getRoutingNumShards()); | ||
|
|
||
| builder.field(KEY_STATE, indexMetaData.getState().toString().toLowerCase(Locale.ENGLISH)); | ||
|
|
||
| boolean binary = params.paramAsBoolean("binary", false); | ||
|
|
||
| builder.startObject(KEY_SETTINGS); | ||
| if (context != MetaData.XContentContext.API) { | ||
| indexMetaData.getSettings().toXContent(builder, new MapParams(Collections.singletonMap("flat_settings", "true"))); | ||
| } else { | ||
| indexMetaData.getSettings().toXContent(builder, params); | ||
| } | ||
| builder.endObject(); | ||
|
|
||
| if (context != MetaData.XContentContext.API) { | ||
| builder.startArray(KEY_MAPPINGS); | ||
| MappingMetaData mmd = indexMetaData.mapping(); | ||
| if (mmd != null) { | ||
| if (binary) { | ||
| builder.value(mmd.source().compressed()); | ||
| } else { | ||
| builder.map(XContentHelper.convertToMap(new BytesArray(mmd.source().uncompressed()), true).v2()); | ||
| } | ||
| } | ||
| builder.endArray(); | ||
| } else { | ||
| builder.startObject(KEY_MAPPINGS); | ||
| MappingMetaData mmd = indexMetaData.mapping(); | ||
| if (mmd != null) { | ||
| Map<String, Object> mapping = XContentHelper.convertToMap(new BytesArray(mmd.source().uncompressed()), false).v2(); | ||
| if (mapping.size() == 1 && mapping.containsKey(mmd.type())) { | ||
| // the type name is the root value, reduce it | ||
| mapping = (Map<String, Object>) mapping.get(mmd.type()); | ||
| } | ||
| builder.field(mmd.type()); | ||
| builder.map(mapping); | ||
| } | ||
| builder.endObject(); | ||
| } | ||
|
|
||
| for (ObjectObjectCursor<String, DiffableStringMap> cursor : indexMetaData.customData) { | ||
| builder.field(cursor.key); | ||
| builder.map(cursor.value); | ||
| } | ||
|
|
||
| if (context != MetaData.XContentContext.API) { | ||
| builder.startObject(KEY_ALIASES); | ||
| for (ObjectCursor<AliasMetaData> cursor : indexMetaData.getAliases().values()) { | ||
| AliasMetaData.Builder.toXContent(cursor.value, builder, params); | ||
| } | ||
| builder.endObject(); | ||
|
|
||
| builder.startArray(KEY_PRIMARY_TERMS); | ||
| for (int i = 0; i < indexMetaData.getNumberOfShards(); i++) { | ||
| builder.value(indexMetaData.primaryTerm(i)); | ||
| } | ||
| builder.endArray(); | ||
| } else { | ||
| builder.startArray(KEY_ALIASES); | ||
| for (ObjectCursor<String> cursor : indexMetaData.getAliases().keys()) { | ||
| builder.value(cursor.value); | ||
| } | ||
| builder.endArray(); | ||
|
|
||
| builder.startObject(IndexMetaData.KEY_PRIMARY_TERMS); | ||
| for (int shard = 0; shard < indexMetaData.getNumberOfShards(); shard++) { | ||
| builder.field(Integer.toString(shard), indexMetaData.primaryTerm(shard)); | ||
| } | ||
| builder.endObject(); | ||
| } | ||
|
|
||
|
|
||
| builder.startObject(KEY_IN_SYNC_ALLOCATIONS); | ||
| for (IntObjectCursor<Set<String>> cursor : indexMetaData.inSyncAllocationIds) { | ||
|
|
@@ -1318,7 +1356,21 @@ public static IndexMetaData fromXContent(XContentParser parser) throws IOExcepti | |
| throw new IllegalArgumentException("Unexpected token: " + token); | ||
| } | ||
| } | ||
| } else if (KEY_PRIMARY_TERMS.equals(currentFieldName)) { | ||
|
There was a problem hiding this comment. why did you add deserialization code here? Is it because we are testing API deserialization somewhere? If so, I would rather have the tests changed to test non-API deserialization. Same comment of other places below. There was a problem hiding this comment. The primary reason for adding these is, the original In general, as the duplicated code diverges, there are conflicting serializations under API context. So the additional deserialization here is to be able to read back in the different serialization emitted by the new code (namely, array serialization for There was a problem hiding this comment. I think we should avoid being able to read back the API-serialized XContent. There is no requirement to do so, and it introduces leniency in the parsing here, with the possibility of silently hiding bugs where the wrong context is chosen for serialization. There was a problem hiding this comment. I've given this a shot and removed deserialization for both If we don't want to remove these many tests for such small changes, the other possible approach is to not emit those different serialization for Please let me know which way you prefer, or if you have other ideas around this. |
||
| LongArrayList list = new LongArrayList(); | ||
| while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { | ||
| if (token == XContentParser.Token.FIELD_NAME) { | ||
| currentFieldName = parser.currentName(); | ||
| } | ||
| else if (token == XContentParser.Token.VALUE_NUMBER) { | ||
| list.add(parser.longValue()); | ||
| } else { | ||
| throw new IllegalStateException("found a non-numeric value under [" + KEY_PRIMARY_TERMS + "]"); | ||
| } | ||
| } | ||
| builder.primaryTerms(list.toArray()); | ||
| } | ||
| else if ("warmers".equals(currentFieldName)) { | ||
| // TODO: do this in 6.0: | ||
| // throw new IllegalArgumentException("Warmers are not supported anymore - are you upgrading from 1.x?"); | ||
| // ignore: warmers have been removed in 5.0 and are | ||
|
|
@@ -1352,7 +1404,13 @@ public static IndexMetaData fromXContent(XContentParser parser) throws IOExcepti | |
| } | ||
| } | ||
| builder.primaryTerms(list.toArray()); | ||
| } | ||
| else if (KEY_ALIASES.equals(currentFieldName)) { | ||
| while (parser.nextToken() != XContentParser.Token.END_ARRAY) { | ||
| builder.putAlias(AliasMetaData.builder(parser.text()).build()); | ||
| } | ||
| } | ||
| else { | ||
|
There was a problem hiding this comment. move these There was a problem hiding this comment. I've removed these changes as they are also for reading back API-serialized XContent. I did correct |
||
| throw new IllegalArgumentException("Unexpected field for an array " + currentFieldName); | ||
| } | ||
| } else if (token.isValue()) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This mutates the global variable on every call? Certainly not what we want?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've removed
API_PARAMSas suggested so this is no longer relevant.