From 2dd6afebdd9e848eef1e192f5212038a6a0de45b Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Tue, 28 May 2024 19:53:55 -0400 Subject: [PATCH 1/8] stop relying on :schema_generation_context? to avoid visibility rules There is a built-in alternative that doesn't leaks all over client code. --- CHANGELOG.md | 16 +++++++ README.md | 44 +------------------ lib/nulogy_graphql_api.rb | 1 - .../schema/base_mutation.rb | 12 ----- .../tasks/schema_generator.rb | 29 +++++------- 5 files changed, 29 insertions(+), 73 deletions(-) delete mode 100644 lib/nulogy_graphql_api/schema/base_mutation.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index dc1c4da..545bec5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,22 @@ _none_ +## 4.0.0 (2024-05-28) + +**Changes** + +* Remove the `schema_generation_context?` attribute to the GraphQL `context` when generating the schema. Use the + already available `GraphQL::Schema::AlwaysVisible` plugin instead. + * **(Breaking)** Remove the `NulogyGraphqlApi::Schema::BaseMutation` class which introduced a new API for the + `visible?` method that took a block. This introduced a deviation from the ruby graphql gem's API only for + Mutations and so it was removed. Please ensure that any invocations of `visible?` do not take a block and use + `GraphQL::Schema::Mutation` instead. + * **(Breaking)** Change the `NulogyGraphqlApi::Tasks::SchemaGenerator#generate_schema` method to output the + stringified version of the schema instead of checking it for changes and writing it to a file. + * Expose the `#check_changes` and `#write_schema_to_file` methods on the + `NulogyGraphqlApi::Tasks::SchemaGenerator` to give the user more control over how to build their + tooling. + ## 3.0.1 (2024-01-30) * Add `include_graphql_error` RSpec matcher diff --git a/README.md b/README.md index c8a5346..30c1547 100644 --- a/README.md +++ b/README.md @@ -130,8 +130,6 @@ end There is a Rake task to generate the `schema.graphql` file. You need to provide the `schema_file_path` and the schema class so that the task can detect breaking changes and generate the file. If you don't have a schema file because it's your first time generating it then the rake task will just create one for you in the path provided. -There is also a third argument `context`. You'll have to configure it to be able to generate the SDL of fields or types that are only visible for more privileged users. - ```ruby namespace :graphql_api do desc "Generate the graphql schema of the api." @@ -142,47 +140,7 @@ namespace :graphql_api do NulogyGraphqlApi::Tasks::SchemaGenerator .new(schema_file_path, schema) - .generate_schema - end -end -``` - -### Node visibility - -When you customize the visibility of parts of your graph you have to make sure that all nodes are visible when the schema is being generated by the rake task. You can do so by using the `schema_generation_context?` attribute that is added to the context by the `SchemaGenerator` mentioned in the previous section. - -Here is how to use it: - -##### On fields -```ruby -field :entity, MyApp::EntityType, null: false do - description "Find an entity by ID" - argument :id, ID, required: true - - def visible?(context) - context[:schema_generation_context?] || context[:current_user].superuser? - end -end -``` - - -##### On mutations - -In this case the `schema_generation_context?` attribute is checked by the `BaseMutation` class. All you have to do is inheriting from it and override `visible?` passing a block to the base method. - -```ruby -module MyApp - class CreateEntity < NulogyGraphqlApi::Schema::BaseMutation - field :entity, MyApp::EntityType, null: false - field :errors, [NulogyGraphqlApi::Types::UserErrorType], null: false - - def self.visible?(context) - super { context[:current_user].super_user? } - end - - def resolve(args) - # ... - end + .write_schema_to_file end end ``` diff --git a/lib/nulogy_graphql_api.rb b/lib/nulogy_graphql_api.rb index acf8d87..391935f 100644 --- a/lib/nulogy_graphql_api.rb +++ b/lib/nulogy_graphql_api.rb @@ -3,7 +3,6 @@ require "nulogy_graphql_api/error_handling" require "nulogy_graphql_api/graphql_executor" require "nulogy_graphql_api/graphql_response" -require "nulogy_graphql_api/schema/base_mutation" require "nulogy_graphql_api/transaction_service" require "nulogy_graphql_api/tasks/schema_changes_checker" require "nulogy_graphql_api/tasks/schema_generator" diff --git a/lib/nulogy_graphql_api/schema/base_mutation.rb b/lib/nulogy_graphql_api/schema/base_mutation.rb deleted file mode 100644 index d1f2db8..0000000 --- a/lib/nulogy_graphql_api/schema/base_mutation.rb +++ /dev/null @@ -1,12 +0,0 @@ -module NulogyGraphqlApi - module Schema - class BaseMutation < GraphQL::Schema::Mutation - def self.visible?(context) - return true if context[:schema_generation_context?] - return super && yield if block_given? - - super - end - end - end -end diff --git a/lib/nulogy_graphql_api/tasks/schema_generator.rb b/lib/nulogy_graphql_api/tasks/schema_generator.rb index 5702b50..05cf2a3 100644 --- a/lib/nulogy_graphql_api/tasks/schema_generator.rb +++ b/lib/nulogy_graphql_api/tasks/schema_generator.rb @@ -4,37 +4,32 @@ class SchemaGenerator def initialize(schema_output_path, schema, context: {}) @schema_output_path = schema_output_path @schema = schema - @context = context.merge( - schema_generation_context?: true - ) + @context = context end def generate_schema - check_changes - write_schema_to_file + # We will want to create a subclass of the schema here to make sure we don't pollute the original schema. + GraphQL::Schema::AlwaysVisible.use(@schema) + @schema.to_definition(context: @context) end - private - def check_changes return if old_schema.blank? - SchemaChangesChecker.new.check_changes(old_schema, @schema) - end - - def old_schema - return unless File.exist?(@schema_output_path) - - File.read(@schema_output_path) + SchemaChangesChecker.new.check_changes(old_schema, generate_schema) end def write_schema_to_file - File.write(@schema_output_path, schema_definition) + File.write(@schema_output_path, generate_schema) puts Rainbow("\nSuccessfully updated #{@schema_output_path}").green end - def schema_definition - GraphQL::Schema::Printer.print_schema(@schema, context: @context) + private + + def old_schema + return unless File.exist?(@schema_output_path) + + File.read(@schema_output_path) end end end From 86970af09a7f126d0522aa520e492b6755ebdd14 Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Tue, 28 May 2024 19:54:29 -0400 Subject: [PATCH 2/8] allow the user to be specified in the controller test helper --- CHANGELOG.md | 1 + README.md | 2 +- lib/nulogy_graphql_api/rspec/graphql_helpers.rb | 6 ++++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 545bec5..7de2b28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ _none_ * Expose the `#check_changes` and `#write_schema_to_file` methods on the `NulogyGraphqlApi::Tasks::SchemaGenerator` to give the user more control over how to build their tooling. +* Allow the user to be specified for the `request_graphql` test helper. ## 3.0.1 (2024-01-30) diff --git a/README.md b/README.md index 30c1547..c15ff39 100644 --- a/README.md +++ b/README.md @@ -195,7 +195,7 @@ The `request_graphql` helper issues a POST request against the provided URL. Thi ```ruby RSpec.describe MyApp::Graphql::Query, :graphql, type: :request do it "returns 401 Unauthorized given an unauthenticated request" do - gql_response = request_graphql(url, <<~GRAPHQL, headers: { "HTTP_AUTHORIZATION" => nil }) + gql_response = request_graphql(url, <<~GRAPHQL, headers: { "HTTP_AUTHORIZATION" => nil }, user: default_user) query { entities { id diff --git a/lib/nulogy_graphql_api/rspec/graphql_helpers.rb b/lib/nulogy_graphql_api/rspec/graphql_helpers.rb index 13debc8..ce405be 100644 --- a/lib/nulogy_graphql_api/rspec/graphql_helpers.rb +++ b/lib/nulogy_graphql_api/rspec/graphql_helpers.rb @@ -1,5 +1,7 @@ module NulogyGraphqlApi module GraphqlHelpers + # Prefer the `request_graphql` method over this one because it exercises more of the stack but doesn't run + # much slower. def execute_graphql(query, schema, variables: {}, context: {}) camelized_variables = variables.deep_transform_keys! { |key| key.to_s.camelize(:lower) } || {} @@ -13,11 +15,11 @@ def execute_graphql(query, schema, variables: {}, context: {}) response.to_h.deep_symbolize_keys end - def request_graphql(url, query, variables: {}, headers: {}) + def request_graphql(url, query, variables: {}, headers: {}, user: nil) params = { query: query, variables: variables }.to_json default_headers = { CONTENT_TYPE: "application/json", - HTTP_AUTHORIZATION: basic_auth_token(default_user.login) + HTTP_AUTHORIZATION: basic_auth_token((user || default_user).login) } post url, params: params, headers: default_headers.merge(headers) From 0498130f9e69ffe0ef3729b2f177e85901fb089f Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Tue, 28 May 2024 19:54:37 -0400 Subject: [PATCH 3/8] bump version --- lib/nulogy_graphql_api/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/nulogy_graphql_api/version.rb b/lib/nulogy_graphql_api/version.rb index 4e75de5..66efba6 100644 --- a/lib/nulogy_graphql_api/version.rb +++ b/lib/nulogy_graphql_api/version.rb @@ -1,3 +1,3 @@ module NulogyGraphqlApi - VERSION = "3.0.1" + VERSION = "4.0.0" end From 95b51fa462e06ae42f8640e858f84167dffc43b2 Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Wed, 29 May 2024 09:36:21 -0400 Subject: [PATCH 4/8] get tests working --- spec/dummy/config/initializers/assets.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/dummy/config/initializers/assets.rb b/spec/dummy/config/initializers/assets.rb index fe48fc3..9c7a620 100644 --- a/spec/dummy/config/initializers/assets.rb +++ b/spec/dummy/config/initializers/assets.rb @@ -1,7 +1,7 @@ # Be sure to restart your server when you modify this file. # Version of your assets, change this if you want to expire all your assets. -Rails.application.config.assets.version = '1.0' +# Rails.application.config.assets.version = '1.0' # Add additional assets to the asset load path. # Rails.application.config.assets.paths << Emoji.images_path From 53905aa11c83ed8d424706a4f03a13a0d98b242f Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Wed, 29 May 2024 09:36:46 -0400 Subject: [PATCH 5/8] safely modify schema visibility --- .../tasks/schema_generator.rb | 6 +- .../tasks/schema_generator_spec.rb | 62 +++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 spec/integration/lib/nulogy_graphql_api/tasks/schema_generator_spec.rb diff --git a/lib/nulogy_graphql_api/tasks/schema_generator.rb b/lib/nulogy_graphql_api/tasks/schema_generator.rb index 05cf2a3..b15b5d3 100644 --- a/lib/nulogy_graphql_api/tasks/schema_generator.rb +++ b/lib/nulogy_graphql_api/tasks/schema_generator.rb @@ -8,9 +8,9 @@ def initialize(schema_output_path, schema, context: {}) end def generate_schema - # We will want to create a subclass of the schema here to make sure we don't pollute the original schema. - GraphQL::Schema::AlwaysVisible.use(@schema) - @schema.to_definition(context: @context) + visible_schema = Class.new(@schema) + GraphQL::Schema::AlwaysVisible.use(visible_schema) + visible_schema.to_definition(context: @context) end def check_changes diff --git a/spec/integration/lib/nulogy_graphql_api/tasks/schema_generator_spec.rb b/spec/integration/lib/nulogy_graphql_api/tasks/schema_generator_spec.rb new file mode 100644 index 0000000..3e1571d --- /dev/null +++ b/spec/integration/lib/nulogy_graphql_api/tasks/schema_generator_spec.rb @@ -0,0 +1,62 @@ +class VisibleQuery < GraphQL::Schema::Object + field :test, String, null: false + end + +class InvisibleQuery < GraphQL::Schema::Object + field :test, String, null: false + + def self.visible?(context) + false + end +end + +RSpec.describe NulogyGraphqlApi::Tasks::SchemaGenerator do + + it "stringifies the schema" do + fake_schema = Class.new(GraphQL::Schema) do + query VisibleQuery + end + task = NulogyGraphqlApi::Tasks::SchemaGenerator.new("schema_output_path", fake_schema) + + result = task.generate_schema + + expect(result).to eq(<<~GQL) + schema { + query: VisibleQuery + } + + type VisibleQuery { + test: String! + } + GQL + end + + it "stringifies invisible parts of the schema" do + fake_schema = Class.new(GraphQL::Schema) do + query InvisibleQuery + end + task = NulogyGraphqlApi::Tasks::SchemaGenerator.new("schema_output_path", fake_schema) + + result = task.generate_schema + + expect(result).to eq(<<~GQL) + schema { + query: InvisibleQuery + } + + type InvisibleQuery { + test: String! + } + GQL + end + + it "does not alter the given schema" do + fake_schema = Class.new(GraphQL::Schema) + old_warden = fake_schema.warden_class + task = NulogyGraphqlApi::Tasks::SchemaGenerator.new("schema_output_path", fake_schema) + + task.generate_schema + + expect(fake_schema.warden_class).to eq(old_warden) + end +end From fb0445963af4e64a515f732ed522ac0d4336d497 Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Wed, 29 May 2024 12:24:25 -0400 Subject: [PATCH 6/8] apply plugin more idiomatically This is the way that plugins are applied in the graphql gem docs. --- lib/nulogy_graphql_api/tasks/schema_generator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/nulogy_graphql_api/tasks/schema_generator.rb b/lib/nulogy_graphql_api/tasks/schema_generator.rb index b15b5d3..88bae26 100644 --- a/lib/nulogy_graphql_api/tasks/schema_generator.rb +++ b/lib/nulogy_graphql_api/tasks/schema_generator.rb @@ -9,7 +9,7 @@ def initialize(schema_output_path, schema, context: {}) def generate_schema visible_schema = Class.new(@schema) - GraphQL::Schema::AlwaysVisible.use(visible_schema) + visible_schema.use(GraphQL::Schema::AlwaysVisible) visible_schema.to_definition(context: @context) end From 2eb08c885b193aeae0cb7ec8a635225da179aa8d Mon Sep 17 00:00:00 2001 From: Sean Kirby Date: Wed, 29 May 2024 12:24:49 -0400 Subject: [PATCH 7/8] ignore local env-specific files --- .gitignore | 2 ++ spec/dummy/db/test.sqlite3 | Bin 0 -> 4096 bytes 2 files changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index da91d7c..43894cb 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,8 @@ /pkg/ /spec/reports/ /tmp/ +/spec/dummy/tmp/local_secret.txt +/spec/dummy/db/test.sqlite3* # rspec failure tracking .rspec_status diff --git a/spec/dummy/db/test.sqlite3 b/spec/dummy/db/test.sqlite3 index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..210e705467a7b7c47f0f429110e6f79034d4e95a 100644 GIT binary patch literal 4096 zcmWFz^vNtqRY=P(%1ta$FlG>7U}9o$P*7lCU|@t|AVoG{WY8 Date: Wed, 29 May 2024 14:23:59 -0400 Subject: [PATCH 8/8] appease rubocop --- .../lib/nulogy_graphql_api/tasks/schema_generator_spec.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/integration/lib/nulogy_graphql_api/tasks/schema_generator_spec.rb b/spec/integration/lib/nulogy_graphql_api/tasks/schema_generator_spec.rb index 3e1571d..65c37b2 100644 --- a/spec/integration/lib/nulogy_graphql_api/tasks/schema_generator_spec.rb +++ b/spec/integration/lib/nulogy_graphql_api/tasks/schema_generator_spec.rb @@ -1,17 +1,16 @@ class VisibleQuery < GraphQL::Schema::Object field :test, String, null: false - end +end class InvisibleQuery < GraphQL::Schema::Object field :test, String, null: false - def self.visible?(context) + def self.visible?(_context) false end end RSpec.describe NulogyGraphqlApi::Tasks::SchemaGenerator do - it "stringifies the schema" do fake_schema = Class.new(GraphQL::Schema) do query VisibleQuery