Restore schema-evolution declarations to elasticgraph-schema_definition#1294
Open
jwils wants to merge 1 commit into
Open
Restore schema-evolution declarations to elasticgraph-schema_definition#1294jwils wants to merge 1 commit into
jwils wants to merge 1 commit into
Conversation
Moves `field.renamed_from`, `type.renamed_from`, `type.deleted_field`, and `schema.deleted_type` — along with the state registries and `DeprecatedElement` record they populate — from `elasticgraph-json_ingestion` back into the core schema definition gem, partially reverting the placement chosen in #1259. These declarations record serializer-neutral facts about how a schema has evolved, and we now have a second consumer: `elasticgraph-proto_ingestion` (#1080) needs rename metadata to keep protobuf field numbers stable across renames (with `reserved` field numbers for deletions as a natural follow-up). Keeping the markers inside one serializer forced other consumers to duck-type against state that may or may not be present; with the declarations in core, every consumer reads typed `State` members directly. This also addresses the concern raised in review of #1259 about `elasticgraph-json_ingestion` bolting 9 fields onto `::ElasticGraph::SchemaDefinition::State`: its `StateExtension` now adds only the 5 genuinely JSON-specific fields, and the four deprecation registries are ordinary typed `State` members that need no `State & StateExtension` casts. All JSON-specific behavior — schema version enforcement, merge reporting, pruning, and the warnings that flag no-longer-needed declarations — stays in `elasticgraph-json_ingestion`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
elasticgraph-proto_ingestion(#1080) is a second consumer of the schema-evolution declarations: it needsfield.renamed_frommetadata to keep protobuf field numbers stable across public field renames, andtype.deleted_fieldmaps directly onto protobufreservedfield numbers as a natural follow-up. With the markers living insideelasticgraph-json_ingestion(where #1259 moved them), any other consumer has to duck-type against state that may or may not be present (state.respond_to?(:renamed_fields_by_type_name_and_old_field_name)), and proto-only projects can't declare renames at all.These declarations record serializer-neutral facts about how a schema has evolved, so this PR restores them to core
elasticgraph-schema_definition(where they lived prior to #1259), partially reverting #1259's placement — the situation has changed now that two serializers consume them.This also addresses the concern raised in review of #1259 about
elasticgraph-json_ingestionbolting 9 fields onto::ElasticGraph::SchemaDefinition::StatewithState & StateExtensioncasts: itsStateExtensionnow adds only the 5 genuinely JSON-specific fields, and the four deprecation registries are ordinary typedStatemembers that need no casts.What
DeprecatedElementtoElasticGraph::SchemaDefinition::SchemaElements::DeprecatedElement.renamed_types_by_old_name,deleted_types_by_old_name,renamed_fields_by_type_name_and_old_field_name,deleted_fields_by_type_name_and_old_field_name) and theirregister_*methods to coreState, always initialized.field.renamed_from,type.renamed_from,type.deleted_field,schema.deleted_type— onto coreField/TypeWithSubfields/API, with docs reworded to be serializer-neutral.elasticgraph-json_ingestionkeeps everything actually JSON-specific (schema version enforcement, merge reporting and "no longer needed" warnings, pruning), reading the core registries.DeprecatedElementspec toelasticgraph-schema_definition, where it now also proves the DSL works with no extension active and thatdefined_atreports the user's schema definition location through theDelegateClassindirection.No user-facing behavior changes:
f.renamed_frometc. work exactly as before for json_ingestion users; they now also work without json_ingestion.Stacked PRs
#1080 rebases on this to drop its
respond_to?probe and spec shim.Verification
script/quick_build(all green, includingscript/run_gem_specsfor both changed gems at 100% line + branch coverage)script/type_check,script/lint,script/spellcheckReferences