Address PR feedback: restore new_deprecated_element factory method, tighten docs, expand defined_at test coverage

Author: jwils

elasticgraph-json_ingestion/lib/elastic_graph/json_ingestion/schema_definition/indexing/json_schema_with_metadata.rb

@@ -53,11 +53,11 @@ class Merger
 
             def initialize(schema_def_results)
               @field_metadata_by_type_and_field_name = schema_def_results.json_schema_field_metadata_by_type_and_field_name
-              @renamed_types_by_old_name = schema_def_results.state.renamed_types_by_old_name
-              @deleted_types_by_old_name = schema_def_results.state.deleted_types_by_old_name
-              @renamed_fields_by_type_name_and_old_field_name = schema_def_results.state.renamed_fields_by_type_name_and_old_field_name
-              @deleted_fields_by_type_name_and_old_field_name = schema_def_results.state.deleted_fields_by_type_name_and_old_field_name
               @state = schema_def_results.state
+              @renamed_types_by_old_name = @state.renamed_types_by_old_name
+              @deleted_types_by_old_name = @state.deleted_types_by_old_name
+              @renamed_fields_by_type_name_and_old_field_name = @state.renamed_fields_by_type_name_and_old_field_name
+              @deleted_fields_by_type_name_and_old_field_name = @state.deleted_fields_by_type_name_and_old_field_name
               @derived_indexing_type_names = schema_def_results.derived_indexing_type_names
 
               @unused_deprecated_elements = (

elasticgraph-schema_definition/lib/elastic_graph/schema_definition/api.rb

@@ -303,7 +303,7 @@ def scalar_type(name, &block)
 
       # Registers the name of a type that existed in a prior version of the schema but has been deleted.
       #
-      # @note When `elasticgraph-json_ingestion` is in use and this API applies, ElasticGraph will give you an error message
+      # @note In situations where this API is needed, ElasticGraph will give you an error message
       #   indicating that you need to use this API or {SchemaElements::TypeWithSubfields#renamed_from}. Likewise, when
       #   ElasticGraph no longer needs to know about this, it'll give you a warning indicating the call to this method can
       #   be removed.

elasticgraph-schema_definition/lib/elastic_graph/schema_definition/factory.rb

@@ -17,6 +17,7 @@
 require "elastic_graph/schema_definition/schema_artifact_manager"
 require "elastic_graph/schema_definition/schema_elements/argument"
 require "elastic_graph/schema_definition/schema_elements/built_in_types"
+require "elastic_graph/schema_definition/schema_elements/deprecated_element"
 require "elastic_graph/schema_definition/schema_elements/directive"
 require "elastic_graph/schema_definition/schema_elements/enum_type"
 require "elastic_graph/schema_definition/schema_elements/enum_value"
@@ -79,6 +80,11 @@ def self.prevent_non_factory_instantiation_of(klass)
         end
       end
 
+      def new_deprecated_element(name, defined_at:, defined_via:)
+        @@deprecated_element_new.call(schema_def_state: @state, name: name, defined_at: defined_at, defined_via: defined_via)
+      end
+      @@deprecated_element_new = prevent_non_factory_instantiation_of(SchemaElements::DeprecatedElement)
+
       def new_argument(field, name, value_type)
         @@argument_new.call(@state, field, name, value_type).tap do |argument|
           yield argument if block_given?

elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/field.rb

@@ -565,9 +565,9 @@ def resolve_with(resolver_name, **config)
 
         # Registers an old name that this field used to have in a prior version of the schema. Extensions
         # use this to deal with schema evolution — for example, `elasticgraph-json_ingestion` uses it to
-        # migrate data ingested under the old JSON schema version.
+        # ingest events that contain old field names that are no longer in the current schema.
         #
-        # @note When `elasticgraph-json_ingestion` is in use and this API applies, ElasticGraph will give you an error message
+        # @note In situations where this API is needed, ElasticGraph will give you an error message
         #   indicating that you need to use this API or `deleted_field`. Likewise, when ElasticGraph no longer needs to know
         #   about this, it'll give you a warning indicating the call to this method can be removed.
         #

elasticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/type_with_subfields.rb

@@ -421,7 +421,7 @@ def relates_to_many(field_name, type, via:, dir:, singular: nil, indexing_only:
 
         # Registers the name of a field that existed in a prior version of the schema but has been deleted.
         #
-        # @note When `elasticgraph-json_ingestion` is in use and this API applies, ElasticGraph will give you an error message
+        # @note In situations where this API is needed, ElasticGraph will give you an error message
         #   indicating that you need to use this API or {Field#renamed_from}. Likewise, when ElasticGraph no longer needs to
         #   know about this, it'll give you a warning indicating the call to this method can be removed.
         #
@@ -445,7 +445,7 @@ def deleted_field(field_name)
 
         # Registers an old name that this type used to have in a prior version of the schema.
         #
-        # @note When `elasticgraph-json_ingestion` is in use and this API applies, ElasticGraph will give you an error message
+        # @note In situations where this API is needed, ElasticGraph will give you an error message
         #   indicating that you need to use this API or {API#deleted_type}. Likewise, when ElasticGraph no longer needs to
         #   know about this, it'll give you a warning indicating the call to this method can be removed.
         #

elasticgraph-schema_definition/lib/elastic_graph/schema_definition/state.rb

@@ -9,7 +9,6 @@
 require "elastic_graph/errors"
 require "elastic_graph/schema_definition/factory"
 require "elastic_graph/schema_definition/mixins/has_readable_to_s_and_inspect"
-require "elastic_graph/schema_definition/schema_elements/deprecated_element"
 require "elastic_graph/schema_definition/schema_elements/enum_value_namer"
 require "elastic_graph/schema_definition/schema_elements/field_path"
 require "elastic_graph/schema_definition/schema_elements/sub_aggregation_path"
@@ -146,41 +145,21 @@ def register_user_defined_field(field)
       end
 
       def register_renamed_type(type_name, from:, defined_at:, defined_via:)
-        renamed_types_by_old_name[from] = SchemaElements::DeprecatedElement.new(
-          schema_def_state: self,
-          name: type_name,
-          defined_at: defined_at,
-          defined_via: defined_via
-        )
+        renamed_types_by_old_name[from] = factory.new_deprecated_element(type_name, defined_at: defined_at, defined_via: defined_via)
       end
 
       def register_deleted_type(type_name, defined_at:, defined_via:)
-        deleted_types_by_old_name[type_name] = SchemaElements::DeprecatedElement.new(
-          schema_def_state: self,
-          name: type_name,
-          defined_at: defined_at,
-          defined_via: defined_via
-        )
+        deleted_types_by_old_name[type_name] = factory.new_deprecated_element(type_name, defined_at: defined_at, defined_via: defined_via)
       end
 
       def register_renamed_field(type_name, from:, to:, defined_at:, defined_via:)
         renamed_fields_by_old_field_name = renamed_fields_by_type_name_and_old_field_name[type_name]
-        renamed_fields_by_old_field_name[from] = SchemaElements::DeprecatedElement.new(
-          schema_def_state: self,
-          name: to,
-          defined_at: defined_at,
-          defined_via: defined_via
-        )
+        renamed_fields_by_old_field_name[from] = factory.new_deprecated_element(to, defined_at: defined_at, defined_via: defined_via)
       end
 
       def register_deleted_field(type_name, field_name, defined_at:, defined_via:)
         deleted_fields_by_old_field_name = deleted_fields_by_type_name_and_old_field_name[type_name]
-        deleted_fields_by_old_field_name[field_name] = SchemaElements::DeprecatedElement.new(
-          schema_def_state: self,
-          name: field_name,
-          defined_at: defined_at,
-          defined_via: defined_via
-        )
+        deleted_fields_by_old_field_name[field_name] = factory.new_deprecated_element(field_name, defined_at: defined_at, defined_via: defined_via)
       end
 
       def user_defined_field_references_by_type_name

elasticgraph-schema_definition/sig/elastic_graph/schema_definition/factory.rbs

@@ -7,6 +7,13 @@ module ElasticGraph
 
       def self.prevent_non_factory_instantiation_of: (::Class) -> ::Method
 
+      def new_deprecated_element: (
+        ::String,
+        defined_at: ::Thread::Backtrace::Location,
+        defined_via: ::String
+      ) -> SchemaElements::DeprecatedElement
+      @@deprecated_element_new: ::Method
+
       def new_argument: (SchemaElements::Field, ::String, SchemaElements::TypeReference) ?{ (SchemaElements::Argument) -> void } -> SchemaElements::Argument
       @@argument_new: ::Method
 

elasticgraph-schema_definition/spec/unit/elastic_graph/schema_definition/schema_elements/deprecated_element_spec.rb

@@ -49,14 +49,46 @@ module SchemaElements
         end
 
         it "reports the caller's schema definition location (not ElasticGraph internals) as `defined_at`" do
+          deleted_type_callsite = nil
+          renamed_type_callsite = nil
+          deleted_field_callsite = nil
+          renamed_field_callsite = nil
+
           state = define_schema(schema_element_name_form: "snake_case") do |schema|
+            deleted_type_callsite = __LINE__ + 1
+            schema.deleted_type "OldType"
+
             schema.object_type "Widget" do |t|
+              renamed_type_callsite = __LINE__ + 1
               t.renamed_from "OldWidget"
+
+              deleted_field_callsite = __LINE__ + 1
+              t.deleted_field "legacy_name"
+
               t.field "id", "ID!"
+              t.field "name", "String" do |f|
+                renamed_field_callsite = __LINE__ + 1
+                f.renamed_from "old_name"
+              end
             end
           end.state
 
-          expect(state.renamed_types_by_old_name.fetch("OldWidget").defined_at.path).to eq __FILE__
+          expect(state.deleted_types_by_old_name.fetch("OldType").defined_at).to have_attributes(
+            path: __FILE__,
+            lineno: deleted_type_callsite
+          )
+          expect(state.renamed_types_by_old_name.fetch("OldWidget").defined_at).to have_attributes(
+            path: __FILE__,
+            lineno: renamed_type_callsite
+          )
+          expect(state.deleted_fields_by_type_name_and_old_field_name.fetch("Widget").fetch("legacy_name").defined_at).to have_attributes(
+            path: __FILE__,
+            lineno: deleted_field_callsite
+          )
+          expect(state.renamed_fields_by_type_name_and_old_field_name.fetch("Widget").fetch("old_name").defined_at).to have_attributes(
+            path: __FILE__,
+            lineno: renamed_field_callsite
+          )
         end
       end
     end