From 5c89f5e64adec53d3713881ddb605c9d0b3e82ee Mon Sep 17 00:00:00 2001 From: Alex Kalderimis Date: Tue, 17 Nov 2020 21:33:10 +0000 Subject: [PATCH] Add connection redaction and connection extension methods The connection redaction is in preparation for the coming authorization changes, and the connection methods are important so that we can test connections as if they are enumerable containers of their items. --- .../graphql/connection_collection_methods.rb | 13 +++++ lib/gitlab/graphql/connection_redaction.rb | 33 ++++++++++++ .../graphql/pagination/array_connection.rb | 15 ++++++ lib/gitlab/graphql/pagination/connections.rb | 4 ++ .../externally_paginated_array_connection.rb | 3 ++ .../graphql/pagination/keyset/connection.rb | 2 + ...ffset_active_record_relation_connection.rb | 2 + .../pagination/array_connection_spec.rb | 15 ++++++ ...ernally_paginated_array_connection_spec.rb | 6 +++ .../pagination/keyset/connection_spec.rb | 7 +++ ..._active_record_relation_connection_spec.rb | 11 ++++ .../connection_redaction_shared_examples.rb | 54 +++++++++++++++++++ .../graphql/connection_shared_examples.rb | 9 ++++ 13 files changed, 174 insertions(+) create mode 100644 lib/gitlab/graphql/connection_collection_methods.rb create mode 100644 lib/gitlab/graphql/connection_redaction.rb create mode 100644 lib/gitlab/graphql/pagination/array_connection.rb create mode 100644 spec/lib/gitlab/graphql/pagination/array_connection_spec.rb create mode 100644 spec/support/shared_examples/graphql/connection_redaction_shared_examples.rb create mode 100644 spec/support/shared_examples/graphql/connection_shared_examples.rb diff --git a/lib/gitlab/graphql/connection_collection_methods.rb b/lib/gitlab/graphql/connection_collection_methods.rb new file mode 100644 index 00000000000000..0e2c4a98bb62ee --- /dev/null +++ b/lib/gitlab/graphql/connection_collection_methods.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + module ConnectionCollectionMethods + extend ActiveSupport::Concern + + included do + delegate :to_a, :size, :include?, :empty?, to: :nodes + end + end + end +end diff --git a/lib/gitlab/graphql/connection_redaction.rb b/lib/gitlab/graphql/connection_redaction.rb new file mode 100644 index 00000000000000..5e037bb9f6394f --- /dev/null +++ b/lib/gitlab/graphql/connection_redaction.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Gitlab + module Graphql + module ConnectionRedaction + class RedactionState + attr_reader :redactor + attr_reader :redacted_nodes + + def redactor=(redactor) + @redactor = redactor + @redacted_nodes = nil + end + + def redacted(&block) + @redacted_nodes ||= redactor.present? ? redactor.redact(yield) : yield + end + end + + delegate :redactor=, to: :redaction_state + + def nodes + redaction_state.redacted { super.to_a } + end + + private + + def redaction_state + @redaction_state ||= RedactionState.new + end + end + end +end diff --git a/lib/gitlab/graphql/pagination/array_connection.rb b/lib/gitlab/graphql/pagination/array_connection.rb new file mode 100644 index 00000000000000..efc912eaeca7e6 --- /dev/null +++ b/lib/gitlab/graphql/pagination/array_connection.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +# We use the Keyset / Stable cursor connection by default for ActiveRecord::Relation. +# However, there are times when that may not be powerful enough (yet), and we +# want to use standard offset pagination. +module Gitlab + module Graphql + module Pagination + class ArrayConnection < ::GraphQL::Pagination::ArrayConnection + prepend ::Gitlab::Graphql::ConnectionRedaction + include ::Gitlab::Graphql::ConnectionCollectionMethods + end + end + end +end diff --git a/lib/gitlab/graphql/pagination/connections.rb b/lib/gitlab/graphql/pagination/connections.rb index 8f37fa3f474100..54a84be4274b30 100644 --- a/lib/gitlab/graphql/pagination/connections.rb +++ b/lib/gitlab/graphql/pagination/connections.rb @@ -12,6 +12,10 @@ def self.use(schema) schema.connections.add( Gitlab::Graphql::ExternallyPaginatedArray, Gitlab::Graphql::Pagination::ExternallyPaginatedArrayConnection) + + schema.connections.add( + Array, + Gitlab::Graphql::Pagination::ArrayConnection) end end end diff --git a/lib/gitlab/graphql/pagination/externally_paginated_array_connection.rb b/lib/gitlab/graphql/pagination/externally_paginated_array_connection.rb index 12e047420bf54d..90a20861b0d290 100644 --- a/lib/gitlab/graphql/pagination/externally_paginated_array_connection.rb +++ b/lib/gitlab/graphql/pagination/externally_paginated_array_connection.rb @@ -5,6 +5,9 @@ module Gitlab module Graphql module Pagination class ExternallyPaginatedArrayConnection < GraphQL::Pagination::ArrayConnection + include ::Gitlab::Graphql::ConnectionCollectionMethods + prepend ::Gitlab::Graphql::ConnectionRedaction + def start_cursor items.previous_cursor end diff --git a/lib/gitlab/graphql/pagination/keyset/connection.rb b/lib/gitlab/graphql/pagination/keyset/connection.rb index 252f63717657ed..2ad8d2f7ab7ca8 100644 --- a/lib/gitlab/graphql/pagination/keyset/connection.rb +++ b/lib/gitlab/graphql/pagination/keyset/connection.rb @@ -31,6 +31,8 @@ module Pagination module Keyset class Connection < GraphQL::Pagination::ActiveRecordRelationConnection include Gitlab::Utils::StrongMemoize + include ::Gitlab::Graphql::ConnectionCollectionMethods + prepend ::Gitlab::Graphql::ConnectionRedaction # rubocop: disable Naming/PredicateName # https://relay.dev/graphql/connections.htm#sec-undefined.PageInfo.Fields diff --git a/lib/gitlab/graphql/pagination/offset_active_record_relation_connection.rb b/lib/gitlab/graphql/pagination/offset_active_record_relation_connection.rb index 33f847015620f1..4a57b7aceca0de 100644 --- a/lib/gitlab/graphql/pagination/offset_active_record_relation_connection.rb +++ b/lib/gitlab/graphql/pagination/offset_active_record_relation_connection.rb @@ -7,6 +7,8 @@ module Gitlab module Graphql module Pagination class OffsetActiveRecordRelationConnection < GraphQL::Pagination::ActiveRecordRelationConnection + prepend ::Gitlab::Graphql::ConnectionRedaction + include ::Gitlab::Graphql::ConnectionCollectionMethods end end end diff --git a/spec/lib/gitlab/graphql/pagination/array_connection_spec.rb b/spec/lib/gitlab/graphql/pagination/array_connection_spec.rb new file mode 100644 index 00000000000000..03cf53bb9903b0 --- /dev/null +++ b/spec/lib/gitlab/graphql/pagination/array_connection_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Gitlab::Graphql::Pagination::ArrayConnection do + let(:nodes) { (1..10) } + + subject(:connection) { described_class.new(nodes, max_page_size: 100) } + + it_behaves_like 'a connection with collection methods' + + it_behaves_like 'a redactable connection' do + let(:unwanted) { 5 } + end +end diff --git a/spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb b/spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb index 932bcd8cd92a6b..84e8f8b95e86a2 100644 --- a/spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb +++ b/spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb @@ -13,6 +13,12 @@ described_class.new(all_nodes, { max_page_size: values.size }.merge(arguments)) end + it_behaves_like 'a connection with collection methods' + + it_behaves_like 'a redactable connection' do + let(:unwanted) { 3 } + end + describe '#nodes' do let(:paged_nodes) { connection.nodes } diff --git a/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb index c8f368b15fc78a..1fee24bdc1ff3b 100644 --- a/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb +++ b/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb @@ -21,6 +21,13 @@ def decoded_cursor(cursor) Gitlab::Json.parse(Base64Bp.urlsafe_decode64(cursor)) end + it_behaves_like 'a connection with collection methods' + + it_behaves_like 'a redactable connection' do + let_it_be(:projects) { create_list(:project, 2) } + let(:unwanted) { projects.second } + end + describe '#cursor_for' do let(:project) { create(:project) } let(:cursor) { connection.cursor_for(project) } diff --git a/spec/lib/gitlab/graphql/pagination/offset_active_record_relation_connection_spec.rb b/spec/lib/gitlab/graphql/pagination/offset_active_record_relation_connection_spec.rb index 86f35de94ed276..1ca7c1c3c69518 100644 --- a/spec/lib/gitlab/graphql/pagination/offset_active_record_relation_connection_spec.rb +++ b/spec/lib/gitlab/graphql/pagination/offset_active_record_relation_connection_spec.rb @@ -6,4 +6,15 @@ it 'subclasses from GraphQL::Relay::RelationConnection' do expect(described_class.superclass).to eq GraphQL::Pagination::ActiveRecordRelationConnection end + + it_behaves_like 'a connection with collection methods' do + let(:connection) { described_class.new(Project.all) } + end + + it_behaves_like 'a redactable connection' do + let_it_be(:users) { create_list(:user, 2) } + + let(:connection) { described_class.new(User.all, max_page_size: 10) } + let(:unwanted) { users.second } + end end diff --git a/spec/support/shared_examples/graphql/connection_redaction_shared_examples.rb b/spec/support/shared_examples/graphql/connection_redaction_shared_examples.rb new file mode 100644 index 00000000000000..12a7b3fe414e2c --- /dev/null +++ b/spec/support/shared_examples/graphql/connection_redaction_shared_examples.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +# requires: +# - `connection` (no-empty, containing `unwanted` and at least one more item) +# - `unwanted` (single item in collection) +RSpec.shared_examples 'a redactable connection' do + context 'no redactor set' do + it 'contains the unwanted item' do + expect(connection.nodes).to include(unwanted) + end + + it 'does not redact more than once' do + connection.nodes + r_state = connection.send(:redaction_state) + + expect(r_state.redacted { raise 'Should not be called!' }).to be_present + end + end + + let_it_be(:constant_redactor) do + Class.new do + def initialize(remove) + @remove = remove + end + + def redact(items) + items - @remove + end + end + end + + context 'redactor is set' do + let(:redactor) do + constant_redactor.new([unwanted]) + end + + before do + connection.redactor = redactor + end + + it 'does not contain the unwanted item' do + expect(connection.nodes).not_to include(unwanted) + expect(connection.nodes).not_to be_empty + end + + it 'does not redact more than once' do + expect(redactor).to receive(:redact).once.and_call_original + + connection.nodes + connection.nodes + connection.nodes + end + end +end diff --git a/spec/support/shared_examples/graphql/connection_shared_examples.rb b/spec/support/shared_examples/graphql/connection_shared_examples.rb new file mode 100644 index 00000000000000..4cba5b5a69dab6 --- /dev/null +++ b/spec/support/shared_examples/graphql/connection_shared_examples.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'a connection with collection methods' do + %i[to_a size include? empty?].each do |method_name| + it "responds to #{method_name}" do + expect(connection).to respond_to(method_name) + end + end +end -- GitLab