diff --git a/elasticgraph-graphql/lib/elastic_graph/graphql/config.rb b/elasticgraph-graphql/lib/elastic_graph/graphql/config.rb index 3216813a7..bb6964d23 100644 --- a/elasticgraph-graphql/lib/elastic_graph/graphql/config.rb +++ b/elasticgraph-graphql/lib/elastic_graph/graphql/config.rb @@ -7,7 +7,6 @@ # frozen_string_literal: true require "elastic_graph/support/config" -require "elastic_graph/errors" require "elastic_graph/graphql/client" require "elastic_graph/schema_artifacts/runtime_metadata/extension_loader" @@ -79,38 +78,7 @@ class Config < Support::Config.define( } ] }, - extension_modules: { - description: "Array of modules that will be extended onto the `GraphQL` instance to support extension libraries.", - type: "array", - items: { - type: "object", - properties: { - name: { - type: "string", - minLength: 1, - description: "The name of the extension module class to load.", - examples: ["MyExtensionModule", "ElasticGraph::MyExtension"] - }, - require_path: { - type: "string", - minLength: 1, - description: "The path to require to load the extension module.", - examples: ["./my_extension_module", "elastic_graph/my_extension"] - } - }, - required: ["name", "require_path"] - }, - default: [], # : untyped - examples: [ - [], # : untyped - [ - { - "name" => "MyExtensionModule", - "require_path" => "./my_extension_module" - } - ] - ] - } + extension_modules: Support::Config::EXTENSION_MODULE_SCHEMA } # The standard ElasticGraph root config setting keys; anything else is assumed to be extension settings. @@ -128,7 +96,7 @@ def self.from_parsed_yaml(parsed_yaml) def convert_values(client_resolver:, extension_modules:, **values) client_resolver = load_client_resolver(client_resolver) - extension_modules = load_extension_modules(extension_modules) + extension_modules = SchemaArtifacts::RuntimeMetadata::ExtensionLoader.load_component_extensions(extension_modules) values.merge({ client_resolver: client_resolver, @@ -150,18 +118,6 @@ def load_client_resolver(config) __skip__ = extension_class.new(extension.config) end - - def load_extension_modules(extension_module_hashes) - extension_loader = SchemaArtifacts::RuntimeMetadata::ExtensionLoader.new(::Module.new) - - extension_module_hashes.map do |mod_hash| - extension_loader.load(mod_hash.fetch("name"), from: mod_hash.fetch("require_path"), config: {}).extension_class.tap do |mod| - unless mod.instance_of?(::Module) - raise Errors::ConfigError, "`#{mod_hash.fetch("name")}` is not a module, but all application extension modules must be modules." - end - end - end - end end end end diff --git a/elasticgraph-graphql/sig/elastic_graph/graphql/config.rbs b/elasticgraph-graphql/sig/elastic_graph/graphql/config.rbs index 19ea7255c..f41e486ca 100644 --- a/elasticgraph-graphql/sig/elastic_graph/graphql/config.rbs +++ b/elasticgraph-graphql/sig/elastic_graph/graphql/config.rbs @@ -43,7 +43,6 @@ module ElasticGraph ) -> ::Hash[::Symbol, untyped] def load_client_resolver: (::Hash[::String, untyped]) -> Client::_Resolver - def load_extension_modules: (::Array[::Hash[::String, untyped]]) -> ::Array[::Module] end end end diff --git a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/config_spec.rb b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/config_spec.rb index d3a450dda..30557b634 100644 --- a/elasticgraph-graphql/spec/unit/elastic_graph/graphql/config_spec.rb +++ b/elasticgraph-graphql/spec/unit/elastic_graph/graphql/config_spec.rb @@ -150,16 +150,6 @@ module EgExtensionModule2 expect(extension_modules).to eq([::EgExtensionModule1, ::EgExtensionModule2]) end - it "raises a clear error if the extension can't be loaded" do - expect { - extension_modules_from(<<~YAML) - extension_modules: - - require_path: ./not_real - name: NotReal - YAML - }.to raise_error LoadError, a_string_including("not_real") - end - it "raises a clear error if the config is malformed" do expect { extension_modules_from(<<~YAML) @@ -183,33 +173,6 @@ module EgExtensionModule1 }.to raise_error a_string_including("name") end - it "raises a clear error if the named extension is not a module" do - File.write("eg_extension_class1.rb", <<~EOS) - class EgExtensionClass1 - end - EOS - - expect { - extension_modules_from(<<~YAML) - extension_modules: - - require_path: ./eg_extension_class1 - name: EgExtensionClass1 - YAML - }.to raise_error a_string_including("not a module") - - File.write("eg_extension_object1.rb", <<~EOS) - EgExtensionObject1 = Object.new - EOS - - expect { - extension_modules_from(<<~YAML) - extension_modules: - - require_path: ./eg_extension_object1 - name: EgExtensionObject1 - YAML - }.to raise_error a_string_including("not a class or module") - end - def load_config_from_yaml(yaml) yaml = <<~EOS default_page_size: 27 diff --git a/elasticgraph-indexer/lib/elastic_graph/indexer.rb b/elasticgraph-indexer/lib/elastic_graph/indexer.rb index 2d64978bd..218b6818d 100644 --- a/elasticgraph-indexer/lib/elastic_graph/indexer.rb +++ b/elasticgraph-indexer/lib/elastic_graph/indexer.rb @@ -40,6 +40,10 @@ def initialize( @schema_artifacts = @datastore_core.schema_artifacts @monotonic_clock = monotonic_clock @clock = clock || ::Time + + # Apply any extension modules that have been configured. + config.extension_modules.each { |mod| extend mod } + @schema_artifacts.runtime_metadata.indexer_extension_modules.each { |ext_mod| extend ext_mod.load_extension.extension_class } end def datastore_router diff --git a/elasticgraph-indexer/lib/elastic_graph/indexer/config.rb b/elasticgraph-indexer/lib/elastic_graph/indexer/config.rb index 2b760f0ca..6eca6a902 100644 --- a/elasticgraph-indexer/lib/elastic_graph/indexer/config.rb +++ b/elasticgraph-indexer/lib/elastic_graph/indexer/config.rb @@ -6,12 +6,12 @@ # # frozen_string_literal: true +require "elastic_graph/schema_artifacts/runtime_metadata/extension_loader" require "elastic_graph/support/config" -require "elastic_graph/errors" module ElasticGraph class Indexer - class Config < Support::Config.define(:latency_slo_thresholds_by_timestamp_in_ms, :skip_derived_indexing_type_updates) + class Config < Support::Config.define(:latency_slo_thresholds_by_timestamp_in_ms, :skip_derived_indexing_type_updates, :extension_modules) json_schema at: "indexer", optional: false, description: "Configuration for indexing operations and metrics used by `elasticgraph-indexer`.", @@ -42,15 +42,17 @@ class Config < Support::Config.define(:latency_slo_thresholds_by_timestamp_in_ms {}, # : untyped {"WidgetWorkspace" => ["ABC12345678"]} ] - } + }, + extension_modules: Support::Config::EXTENSION_MODULE_SCHEMA } private - def convert_values(skip_derived_indexing_type_updates:, latency_slo_thresholds_by_timestamp_in_ms:) + def convert_values(skip_derived_indexing_type_updates:, latency_slo_thresholds_by_timestamp_in_ms:, extension_modules:) { skip_derived_indexing_type_updates: skip_derived_indexing_type_updates.transform_values(&:to_set), - latency_slo_thresholds_by_timestamp_in_ms: latency_slo_thresholds_by_timestamp_in_ms + latency_slo_thresholds_by_timestamp_in_ms: latency_slo_thresholds_by_timestamp_in_ms, + extension_modules: SchemaArtifacts::RuntimeMetadata::ExtensionLoader.load_component_extensions(extension_modules) } end end diff --git a/elasticgraph-indexer/sig/elastic_graph/indexer/config.rbs b/elasticgraph-indexer/sig/elastic_graph/indexer/config.rbs index 1228afd4d..a4f343d59 100644 --- a/elasticgraph-indexer/sig/elastic_graph/indexer/config.rbs +++ b/elasticgraph-indexer/sig/elastic_graph/indexer/config.rbs @@ -5,14 +5,17 @@ module ElasticGraph attr_reader latency_slo_thresholds_by_timestamp_in_ms: ::Hash[::String, ::Integer] attr_reader skip_derived_indexing_type_updates: ::Hash[::String, ::Set[::String]] + attr_reader extension_modules: ::Array[::Module] def initialize: ( ?latency_slo_thresholds_by_timestamp_in_ms: ::Hash[::String, ::Integer], - ?skip_derived_indexing_type_updates: ::Hash[::String, ::Set[::String]]) -> void + ?skip_derived_indexing_type_updates: ::Hash[::String, ::Set[::String]], + ?extension_modules: ::Array[::Module]) -> void def with: ( ?latency_slo_thresholds_by_timestamp_in_ms: ::Hash[::String, ::Integer], - ?skip_derived_indexing_type_updates: ::Hash[::String, ::Set[::String]]) -> Config + ?skip_derived_indexing_type_updates: ::Hash[::String, ::Set[::String]], + ?extension_modules: ::Array[::Module]) -> Config def self.members: () -> ::Array[::Symbol] end @@ -22,8 +25,10 @@ module ElasticGraph def convert_values: ( latency_slo_thresholds_by_timestamp_in_ms: untyped, - skip_derived_indexing_type_updates: untyped + skip_derived_indexing_type_updates: untyped, + extension_modules: untyped ) -> ::Hash[::Symbol, untyped] + end end end diff --git a/elasticgraph-indexer/spec/unit/elastic_graph/indexer/config_spec.rb b/elasticgraph-indexer/spec/unit/elastic_graph/indexer/config_spec.rb index 072d43707..2c157a8e9 100644 --- a/elasticgraph-indexer/spec/unit/elastic_graph/indexer/config_spec.rb +++ b/elasticgraph-indexer/spec/unit/elastic_graph/indexer/config_spec.rb @@ -31,6 +31,57 @@ class Indexer expect(config.skip_derived_indexing_type_updates).to eq("WidgetCurrency" => ["USD"].to_set) end + + describe "#extension_modules", :in_temp_dir do + it "loads the extension modules from disk" do + File.write("eg_extension_module1.rb", <<~EOS) + module EgExtensionModule1 + end + EOS + + File.write("eg_extension_module2.rb", <<~EOS) + module EgExtensionModule2 + end + EOS + + extension_modules = extension_modules_from(<<~YAML) + extension_modules: + - require_path: ./eg_extension_module1 + name: EgExtensionModule1 + - require_path: ./eg_extension_module2 + name: EgExtensionModule2 + YAML + + expect(extension_modules).to eq([::EgExtensionModule1, ::EgExtensionModule2]) + end + + it "raises a clear error if the config is malformed" do + expect { + extension_modules_from(<<~YAML) + extension_modules: + - require: ./not_real + name: NotReal + YAML + }.to raise_error a_string_including("require_path") + + File.write("eg_extension_module1.rb", <<~EOS) + module EgExtensionModule1 + end + EOS + + expect { + extension_modules_from(<<~YAML) + extension_modules: + - require_path: ./eg_extension_module1 + extension: EgExtensionModule1 + YAML + }.to raise_error a_string_including("name") + end + + def extension_modules_from(yaml) + Config.from_parsed_yaml("indexer" => ::YAML.safe_load(yaml)).extension_modules + end + end end end end diff --git a/elasticgraph-indexer/spec/unit/elastic_graph/indexer_spec.rb b/elasticgraph-indexer/spec/unit/elastic_graph/indexer_spec.rb index abd32bc88..6830fd64a 100644 --- a/elasticgraph-indexer/spec/unit/elastic_graph/indexer_spec.rb +++ b/elasticgraph-indexer/spec/unit/elastic_graph/indexer_spec.rb @@ -29,5 +29,50 @@ module ElasticGraph expect(indexer).to be_a(Indexer) end end + + context "when `config.extension_modules` or runtime metadata indexer extension modules are configured" do + it "applies the extensions when the Indexer instance is instantiated without impacting any other instances" do + config_extension_module = Module.new do + def datastore_router + :config_router + end + end + stub_const("ConfigExtensionModule", config_extension_module) + + runtime_metadata_extension_module = Module.new do + def processor + :runtime_metadata_processor + end + end + stub_const("RuntimeMetadataExtensionModule", runtime_metadata_extension_module) + + extended_indexer = build_indexer( + extension_modules: [config_extension_module], + schema_definition: lambda do |schema| + # `defined_at` just needs a valid require path, but needs to be outside ElasticGraph + # to not mess with our code coverage measurement. + schema.register_indexer_extension runtime_metadata_extension_module, defined_at: "time" + define_schema_elements(schema) + end + ) + + normal_indexer = build_indexer( + schema_definition: lambda { |schema| define_schema_elements(schema) } + ) + + expect(extended_indexer.datastore_router).to eq :config_router + expect(extended_indexer.processor).to eq :runtime_metadata_processor + + expect(normal_indexer.datastore_router).to be_a(Indexer::DatastoreIndexingRouter) + expect(normal_indexer.processor).to be_a(Indexer::Processor) + end + + def define_schema_elements(schema) + schema.object_type "Widget" do |t| + t.field "id", "ID!" + t.index "widgets" + end + end + end end end diff --git a/elasticgraph-local/lib/elastic_graph/local/spec_support/config_schema.yaml b/elasticgraph-local/lib/elastic_graph/local/spec_support/config_schema.yaml index b42bfc9f5..f3c74ad6a 100644 --- a/elasticgraph-local/lib/elastic_graph/local/spec_support/config_schema.yaml +++ b/elasticgraph-local/lib/elastic_graph/local/spec_support/config_schema.yaml @@ -441,7 +441,7 @@ properties: header_name: X-Client-Name additionalProperties: false extension_modules: - description: Array of modules that will be extended onto the `GraphQL` instance + description: Array of modules that will be extended onto the component instance to support extension libraries. type: array items: @@ -508,6 +508,36 @@ properties: - {} - WidgetWorkspace: - ABC12345678 + extension_modules: + description: Array of modules that will be extended onto the component instance + to support extension libraries. + type: array + items: + type: object + properties: + name: + type: string + minLength: 1 + description: The name of the extension module class to load. + examples: + - MyExtensionModule + - ElasticGraph::MyExtension + require_path: + type: string + minLength: 1 + description: The path to require to load the extension module. + examples: + - "./my_extension_module" + - elastic_graph/my_extension + required: + - name + - require_path + additionalProperties: false + default: [] + examples: + - [] + - - name: MyExtensionModule + require_path: "./my_extension_module" additionalProperties: false logger: description: Configuration for logging used by all parts of ElasticGraph. diff --git a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/extension_loader.rb b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/extension_loader.rb index 06f4c7b6f..982483564 100644 --- a/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/extension_loader.rb +++ b/elasticgraph-schema_artifacts/lib/elastic_graph/schema_artifacts/runtime_metadata/extension_loader.rb @@ -25,6 +25,20 @@ module RuntimeMetadata # # @private class ExtensionLoader + # Loads the component extension modules described by the given config hashes (as configured + # via an `extension_modules` config setting), verifying that each is a module. + def self.load_component_extensions(extension_module_hashes) + extension_loader = new(::Module.new) + + extension_module_hashes.map do |mod_hash| + extension_loader.load(mod_hash.fetch("name"), from: mod_hash.fetch("require_path"), config: {}).extension_class.tap do |mod| + unless mod.instance_of?(::Module) + raise Errors::ConfigError, "`#{mod_hash.fetch("name")}` is not a module, but all application extension modules must be modules." + end + end + end + end + def initialize(interface_def) @interface_def = interface_def @loaded_by_name = {} diff --git a/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/extension_loader.rbs b/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/extension_loader.rbs index b62ffcf59..40eb61906 100644 --- a/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/extension_loader.rbs +++ b/elasticgraph-schema_artifacts/sig/elastic_graph/schema_artifacts/runtime_metadata/extension_loader.rbs @@ -2,6 +2,7 @@ module ElasticGraph module SchemaArtifacts module RuntimeMetadata class ExtensionLoader + def self.load_component_extensions: (::Array[::Hash[::String, untyped]]) -> ::Array[::Module] def initialize: (extensionClass) -> void def load: (::String, from: ::String, config: ::Hash[::Symbol | ::String, untyped]) -> Extension diff --git a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/extension_loader_spec.rb b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/extension_loader_spec.rb index f4471245a..12dd22af6 100644 --- a/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/extension_loader_spec.rb +++ b/elasticgraph-schema_artifacts/spec/unit/elastic_graph/schema_artifacts/runtime_metadata/extension_loader_spec.rb @@ -109,6 +109,58 @@ module RuntimeMetadata end end + describe ".load_component_extensions", :in_temp_dir do + it "loads the extension modules from disk" do + ::File.write("eg_extension_module1.rb", <<~EOS) + module EgExtensionModule1 + end + EOS + + ::File.write("eg_extension_module2.rb", <<~EOS) + module EgExtensionModule2 + end + EOS + + extension_modules = ExtensionLoader.load_component_extensions([ + {"name" => "EgExtensionModule1", "require_path" => "./eg_extension_module1"}, + {"name" => "EgExtensionModule2", "require_path" => "./eg_extension_module2"} + ]) + + expect(extension_modules).to eq([::EgExtensionModule1, ::EgExtensionModule2]) + end + + it "raises a clear error if an extension can't be loaded" do + expect { + ExtensionLoader.load_component_extensions([ + {"name" => "NotReal", "require_path" => "./not_real"} + ]) + }.to raise_error LoadError, a_string_including("not_real") + end + + it "raises a clear error if a named extension is not a module" do + ::File.write("eg_extension_class1.rb", <<~EOS) + class EgExtensionClass1 + end + EOS + + expect { + ExtensionLoader.load_component_extensions([ + {"name" => "EgExtensionClass1", "require_path" => "./eg_extension_class1"} + ]) + }.to raise_error a_string_including("not a module") + + ::File.write("eg_extension_object1.rb", <<~EOS) + EgExtensionObject1 = Object.new + EOS + + expect { + ExtensionLoader.load_component_extensions([ + {"name" => "EgExtensionObject1", "require_path" => "./eg_extension_object1"} + ]) + }.to raise_error a_string_including("not a class or module") + end + end + def eq_extension(extension_class, from:, config:) eq(Extension.new(extension_class, from, config)) end diff --git a/elasticgraph-support/lib/elastic_graph/support/config.rb b/elasticgraph-support/lib/elastic_graph/support/config.rb index b7e057629..fc55e9768 100644 --- a/elasticgraph-support/lib/elastic_graph/support/config.rb +++ b/elasticgraph-support/lib/elastic_graph/support/config.rb @@ -16,6 +16,41 @@ module ElasticGraph module Support # Provides a standard way to define an ElasticGraph configuration class. module Config + # JSON schema for an `extension_modules` config setting, for use by ElasticGraph components + # (such as `elasticgraph-graphql` and `elasticgraph-indexer`) that support extension modules + # being extended onto their component instances. + EXTENSION_MODULE_SCHEMA = { + description: "Array of modules that will be extended onto the component instance to support extension libraries.", + type: "array", + items: { + type: "object", + properties: { + name: { + type: "string", + minLength: 1, + description: "The name of the extension module class to load.", + examples: ["MyExtensionModule", "ElasticGraph::MyExtension"] + }, + require_path: { + type: "string", + minLength: 1, + description: "The path to require to load the extension module.", + examples: ["./my_extension_module", "elastic_graph/my_extension"] + } + }, + required: ["name", "require_path"] + }, + default: [], # : untyped + examples: [ + [], # : untyped + [ + { + "name" => "MyExtensionModule", + "require_path" => "./my_extension_module" + } + ] + ] + } # Defines a configuration class with the given attributes. # # @param attrs [::Symbol] attribute names diff --git a/elasticgraph-support/sig/elastic_graph/support/config.rbs b/elasticgraph-support/sig/elastic_graph/support/config.rbs index 4ea8bc915..01be07598 100644 --- a/elasticgraph-support/sig/elastic_graph/support/config.rbs +++ b/elasticgraph-support/sig/elastic_graph/support/config.rbs @@ -1,6 +1,8 @@ module ElasticGraph module Support module Config + EXTENSION_MODULE_SCHEMA: ::Hash[::Symbol, untyped] + def self.define: [T] (*::Symbol) ?{ (ClassMethods[T]) -> void } -> T class ConfigData < Data diff --git a/spec_support/lib/elastic_graph/spec_support/builds_indexer.rb b/spec_support/lib/elastic_graph/spec_support/builds_indexer.rb index 23fa72092..2e32b7cf3 100644 --- a/spec_support/lib/elastic_graph/spec_support/builds_indexer.rb +++ b/spec_support/lib/elastic_graph/spec_support/builds_indexer.rb @@ -18,18 +18,24 @@ def build_indexer( datastore_core: nil, latency_slo_thresholds_by_timestamp_in_ms: {}, skip_derived_indexing_type_updates: {}, + extension_modules: [], datastore_router: nil, clock: nil, monotonic_clock: nil, **datastore_core_options, &customize_datastore_config ) + config = Indexer::Config.new( + latency_slo_thresholds_by_timestamp_in_ms: latency_slo_thresholds_by_timestamp_in_ms, + skip_derived_indexing_type_updates: skip_derived_indexing_type_updates + ) + + # This config setting must bypass the JSON schema validation so we provide it via `with`. + config = config.with(extension_modules: config.extension_modules + extension_modules) + Indexer.new( datastore_core: datastore_core || build_datastore_core(**datastore_core_options, &customize_datastore_config), - config: Indexer::Config.new( - latency_slo_thresholds_by_timestamp_in_ms: latency_slo_thresholds_by_timestamp_in_ms, - skip_derived_indexing_type_updates: skip_derived_indexing_type_updates - ), + config: config, datastore_router: datastore_router, clock: clock, monotonic_clock: monotonic_clock