Address review feedback on indexing event decoder PR

- Make `indexing_event_decoder` a required `SqsProcessor` kwarg instead of a
  nilable one with a second, duplicate default-construction path. The warehouse
  lambda now passes its indexer's configured decoder, so there is a single
  source of truth for the default and decoders can rely on receiving non-nil
  `schema_artifacts`.
- Make `JSONLines` inherit from `Interface` so its RBS superclass declaration
  matches the runtime class.
- Validate the decoder's `name` config with the same class-name pattern used
  for query interceptors, and regenerate the config schema artifact.
- Match the established extension-interface style (explanatory comments,
  `:nocov:` only around the body that must return a value) and document the
  interface publicly since it is the contract decoder authors implement.
- Replace the `_ =` cast with an inline type annotation, alphabetize requires,
  and simplify the SQS processor spec helper.

Author: jwils

elasticgraph-indexer/lib/elastic_graph/indexer.rb

@@ -89,7 +89,8 @@ def operation_factory
     def indexing_event_decoder
       @indexing_event_decoder ||= begin
         extension = config.indexing_event_decoder
-        (_ = extension.extension_class).new(
+        decoder_class = extension.extension_class # : untyped
+        decoder_class.new(
           config: extension.config,
           schema_artifacts: schema_artifacts,
           logger: logger

elasticgraph-indexer/lib/elastic_graph/indexer/config.rb

@@ -6,10 +6,10 @@
 #
 # frozen_string_literal: true
 
-require "elastic_graph/support/config"
 require "elastic_graph/errors"
 require "elastic_graph/indexer/indexing_event_decoder"
 require "elastic_graph/schema_artifacts/runtime_metadata/extension_loader"
+require "elastic_graph/support/config"
 
 module ElasticGraph
   class Indexer
@@ -57,7 +57,7 @@ class Config < Support::Config.define(:latency_slo_thresholds_by_timestamp_in_ms
               name: {
                 description: "The name of the indexing event decoder extension class.",
                 type: "string",
-                minLength: 1,
+                pattern: /^[A-Z]\w+(::[A-Z]\w+)*$/.source, # https://rubular.com/r/UuqAz4fR3kdMip
                 examples: ["MyCompany::ElasticGraph::CSVIndexingEventDecoder"]
               },
               require_path: {

elasticgraph-indexer/lib/elastic_graph/indexer/indexing_event_decoder.rb

@@ -10,28 +10,36 @@
 
 module ElasticGraph
   class Indexer
+    # Namespace for indexing event decoders, which turn raw payload strings from a transport into
+    # ElasticGraph indexing event hashes. The decoder to use is configured via the
+    # `indexer.indexing_event_decoder` setting.
     module IndexingEventDecoder
-      # Defines the extension interface implemented by indexing event decoders.
-      #
-      # @api private
+      # Defines the indexing event decoder interface, which our extension loader will validate against.
       class Interface
-        # :nocov:
+        # @param config [Hash<String, Object>] configuration from the `indexing_event_decoder.config` setting
+        # @param schema_artifacts [SchemaArtifacts::FromDisk] the schema artifacts
+        # @param logger [Logger] the ElasticGraph logger
         def initialize(config:, schema_artifacts:, logger:)
+          # must be defined, but nothing to do
         end
 
+        # @param payload [String] a raw payload from the transport
+        # @return [Array<Hash<String, Object>>] the decoded ElasticGraph indexing events
         def decode(payload)
+          # :nocov: -- must return an array to satisfy Steep type checking but never called
           []
+          # :nocov:
         end
-        # :nocov:
       end
 
       # The default indexing event decoder, which expects newline-delimited JSON objects.
-      #
-      # @api private
-      class JSONLines
+      class JSONLines < Interface
+        # (see Interface#initialize)
         def initialize(config:, schema_artifacts:, logger:)
+          # must be defined for extension interface verification, but nothing to do
         end
 
+        # (see Interface#decode)
         def decode(payload)
           payload.split("\n").map { |event| JSON.parse(event) }
         end

elasticgraph-indexer/sig/elastic_graph/indexer/indexing_event_decoder.rbs

@@ -6,7 +6,7 @@ module ElasticGraph
       class Interface
         def initialize: (
           config: ::Hash[::Symbol | ::String, untyped],
-          schema_artifacts: schemaArtifacts?,
+          schema_artifacts: schemaArtifacts,
           logger: ::Logger
         ) -> void
 

elasticgraph-indexer/spec/unit/elastic_graph/indexer/indexing_event_decoder_spec.rb

@@ -10,9 +10,9 @@
 
 module ElasticGraph
   class Indexer
-    RSpec.describe IndexingEventDecoder::JSONLines do
+    RSpec.describe IndexingEventDecoder::JSONLines, :capture_logs do
       it "decodes newline-delimited JSON objects" do
-        decoder = described_class.new(config: {}, schema_artifacts: nil, logger: nil)
+        decoder = described_class.new(config: {}, schema_artifacts: nil, logger: logger)
         payload = <<~JSONL
           {"op":"upsert","id":"1"}
           {"op":"upsert","id":"2"}

elasticgraph-indexer_lambda/lib/elastic_graph/indexer_lambda/sqs_processor.rb

@@ -19,10 +19,10 @@ class SqsProcessor
       # @dynamic ignore_sqs_latency_timestamps_from_arns
       attr_reader :ignore_sqs_latency_timestamps_from_arns
 
-      def initialize(indexer_processor, logger:, ignore_sqs_latency_timestamps_from_arns:, indexing_event_decoder: nil, s3_client: nil)
+      def initialize(indexer_processor, logger:, ignore_sqs_latency_timestamps_from_arns:, indexing_event_decoder:, s3_client: nil)
         @indexer_processor = indexer_processor
         @logger = logger
-        @indexing_event_decoder = indexing_event_decoder || default_indexing_event_decoder
+        @indexing_event_decoder = indexing_event_decoder
         @s3_client = s3_client
         @ignore_sqs_latency_timestamps_from_arns = ignore_sqs_latency_timestamps_from_arns
       end
@@ -94,16 +94,6 @@ def decoded_events_from(payload)
         @indexing_event_decoder.decode(payload)
       end
 
-      def default_indexing_event_decoder
-        require "elastic_graph/indexer/indexing_event_decoder"
-
-        Indexer::IndexingEventDecoder::JSONLines.new(
-          config: {},
-          schema_artifacts: nil,
-          logger: @logger
-        )
-      end
-
       def extract_sqs_metadata(record)
         sqs_timestamps = {
           "processing_first_attempted_at" => millis_to_iso8601(record.dig("attributes", "ApproximateFirstReceiveTimestamp")),

elasticgraph-indexer_lambda/sig/elastic_graph/indexer_lambda/sqs_processor.rbs

@@ -5,7 +5,7 @@ module ElasticGraph
         Indexer::Processor,
         logger: ::Logger,
         ignore_sqs_latency_timestamps_from_arns: ::Set[::String],
-        ?indexing_event_decoder: Indexer::indexingEventDecoder?,
+        indexing_event_decoder: Indexer::indexingEventDecoder,
         ?s3_client: Aws::S3::Client?,
       ) -> void
 
@@ -25,7 +25,6 @@ module ElasticGraph
       def extract_sqs_metadata: (::Hash[String, untyped]) -> ::Hash[::String, untyped]
       def millis_to_iso8601: (::String) -> ::String?
       def decoded_events_from: (::String) -> ::Array[::Hash[::String, untyped]]
-      def default_indexing_event_decoder: () -> Indexer::indexingEventDecoder
       def get_payload_from_s3: (::String) -> ::String
       def s3_client: () -> Aws::S3::Client
       def format_response: (

elasticgraph-indexer_lambda/spec/unit/elastic_graph/indexer_lambda/sqs_processor_spec.rb

@@ -394,12 +394,16 @@ def jsonl(*items)
       end
 
       def build_sqs_processor(**options)
-        event_decoder = options.delete(:indexing_event_decoder) { nil }
+        json_lines_decoder = Indexer::IndexingEventDecoder::JSONLines.new(
+          config: {},
+          schema_artifacts: nil, # not used by `JSONLines`
+          logger: logger
+        )
 
         SqsProcessor.new(
           indexer_processor,
           logger: logger,
-          indexing_event_decoder: event_decoder,
+          indexing_event_decoder: json_lines_decoder,
           ignore_sqs_latency_timestamps_from_arns: ignore_sqs_latency_timestamps_from_arns,
           **options
         )

elasticgraph-local/lib/elastic_graph/local/spec_support/config_schema.yaml

@@ -516,7 +516,7 @@ properties:
           name:
             description: The name of the indexing event decoder extension class.
             type: string
-            minLength: 1
+            pattern: "^[A-Z]\\w+(::[A-Z]\\w+)*$"
             examples:
             - MyCompany::ElasticGraph::CSVIndexingEventDecoder
           require_path:

elasticgraph-warehouse_lambda/lib/elastic_graph/warehouse_lambda/lambda_function.rb

@@ -28,6 +28,7 @@ def initialize
         @sqs_processor = IndexerLambda::SqsProcessor.new(
           warehouse_lambda.processor,
           ignore_sqs_latency_timestamps_from_arns: ignore_sqs_latency_timestamps_from_arns,
+          indexing_event_decoder: warehouse_lambda.indexer.indexing_event_decoder,
           logger: warehouse_lambda.logger
         )
       end