diff options
| -rw-r--r-- | Gemfile | 2 | ||||
| -rw-r--r-- | Gemfile.checksum | 2 | ||||
| -rw-r--r-- | Gemfile.lock | 4 | ||||
| -rw-r--r-- | app/services/members/update_service.rb | 5 | ||||
| -rw-r--r-- | doc/api/freeze_periods.md | 34 | ||||
| -rw-r--r-- | doc/api/releases/links.md | 20 | ||||
| -rw-r--r-- | doc/user/project/releases/release_fields.md | 4 | ||||
| -rw-r--r-- | lib/banzai/filter/kroki_filter.rb | 5 | ||||
| -rw-r--r-- | lib/banzai/filter/syntax_highlight_filter.rb | 3 | ||||
| -rw-r--r-- | lib/gitlab/asciidoc.rb | 1 | ||||
| -rw-r--r-- | lib/gitlab/kroki.rb | 1 | ||||
| -rw-r--r-- | spec/lib/gitlab/asciidoc_spec.rb | 4 | ||||
| -rw-r--r-- | spec/lib/gitlab/kroki_spec.rb | 3 | ||||
| -rw-r--r-- | spec/lib/gitlab/redis/multi_store_spec.rb | 502 | ||||
| -rw-r--r-- | spec/services/members/update_service_spec.rb | 60 |
15 files changed, 260 insertions, 390 deletions
@@ -187,7 +187,7 @@ gem 'wikicloth', '0.8.1' gem 'asciidoctor', '~> 2.0.17' gem 'asciidoctor-include-ext', '~> 0.4.0', require: false gem 'asciidoctor-plantuml', '~> 0.0.16' -gem 'asciidoctor-kroki', '~> 0.5.0', require: false +gem 'asciidoctor-kroki', '~> 0.7.0', require: false gem 'rouge', '~> 3.30.0' gem 'truncato', '~> 0.7.12' gem 'bootstrap_form', '~> 4.2.0' diff --git a/Gemfile.checksum b/Gemfile.checksum index 37655fcb569..c693260714d 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -24,7 +24,7 @@ {"name":"asana","version":"0.10.13","platform":"ruby","checksum":"36d0d37f8dd6118a54580f1b80224875d7b6a9027598938e1722a508bfc2d7ac"}, {"name":"asciidoctor","version":"2.0.17","platform":"ruby","checksum":"ed5b5e399e8d64994cc16f0983f993d6e33990909a8415b6fc8b786cdeb00f3d"}, {"name":"asciidoctor-include-ext","version":"0.4.0","platform":"ruby","checksum":"406adb9d2fbfc25536609ca13b787ed704dc06a4e49d6709b83f3bad578f7878"}, -{"name":"asciidoctor-kroki","version":"0.5.0","platform":"ruby","checksum":"622c8b74796689bdaf8abdf89ad5295b11ce310e3d193e28f19e5baf58d45f12"}, +{"name":"asciidoctor-kroki","version":"0.7.0","platform":"ruby","checksum":"528ae4e49cae10e98c76e91f9aa40c67bf8540aa5ce4bbd44c5cd57af9f0b121"}, {"name":"asciidoctor-plantuml","version":"0.0.16","platform":"ruby","checksum":"407e47cd1186ded5ccc75f0c812e5524c26c571d542247c5132abb8f47bd1793"}, {"name":"ast","version":"2.4.2","platform":"ruby","checksum":"1e280232e6a33754cde542bc5ef85520b74db2aac73ec14acef453784447cc12"}, {"name":"atlassian-jwt","version":"0.2.0","platform":"ruby","checksum":"52e653e9d6062d7a740c3675b0e79fa08367927c6fc17f5476d1b6b3798c6eb2"}, diff --git a/Gemfile.lock b/Gemfile.lock index 0447008023e..1978a1aa352 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -172,7 +172,7 @@ GEM asciidoctor (2.0.17) asciidoctor-include-ext (0.4.0) asciidoctor (>= 1.5.6, < 3.0.0) - asciidoctor-kroki (0.5.0) + asciidoctor-kroki (0.7.0) asciidoctor (~> 2.0) asciidoctor-plantuml (0.0.16) asciidoctor (>= 2.0.17, < 3.0.0) @@ -1549,7 +1549,7 @@ DEPENDENCIES asana (~> 0.10.13) asciidoctor (~> 2.0.17) asciidoctor-include-ext (~> 0.4.0) - asciidoctor-kroki (~> 0.5.0) + asciidoctor-kroki (~> 0.7.0) asciidoctor-plantuml (~> 0.0.16) atlassian-jwt (~> 0.2.0) attr_encrypted (~> 3.2.4)! diff --git a/app/services/members/update_service.rb b/app/services/members/update_service.rb index 8ef3e307519..3d8a76c50c2 100644 --- a/app/services/members/update_service.rb +++ b/app/services/members/update_service.rb @@ -12,7 +12,10 @@ module Members old_access_level = member.human_access old_expiry = member.expires_at - if member.update(params) + member.attributes = params + return success(member: member) unless member.changed? + + if member.save after_execute(action: permission, old_access_level: old_access_level, old_expiry: old_expiry, member: member) # Deletes only confidential issues todos for guests diff --git a/doc/api/freeze_periods.md b/doc/api/freeze_periods.md index 36d069df607..9d2989229ae 100644 --- a/doc/api/freeze_periods.md +++ b/doc/api/freeze_periods.md @@ -16,9 +16,9 @@ You can use the Freeze Periods API to manipulate GitLab [Freeze Period](../user/ Only users with Maintainer [permissions](../user/permissions.md) can interact with the Freeze Period API endpoints. -## List Freeze Periods +## List freeze periods -Paginated list of Freeze Periods, sorted by `created_at` in ascending order. +Paginated list of freeze periods, sorted by `created_at` in ascending order. ```plaintext GET /projects/:id/freeze_periods @@ -49,9 +49,9 @@ Example response: ] ``` -## Get a Freeze Period by a `freeze_period_id` +## Get a freeze period by a `freeze_period_id` -Get a Freeze Period for the given `freeze_period_id`. +Get a freeze period for the given `freeze_period_id`. ```plaintext GET /projects/:id/freeze_periods/:freeze_period_id @@ -60,7 +60,7 @@ GET /projects/:id/freeze_periods/:freeze_period_id | Attribute | Type | Required | Description | | ------------- | -------------- | -------- | ----------------------------------------------------------------------------------- | | `id` | integer or string | yes | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding). | -| `freeze_period_id` | string | yes | The database ID of the Freeze Period. | +| `freeze_period_id` | integer | yes | The ID of the freeze period. | Example request: @@ -81,9 +81,9 @@ Example response: } ``` -## Create a Freeze Period +## Create a freeze period -Create a Freeze Period. +Create a freeze period. ```plaintext POST /projects/:id/freeze_periods @@ -92,8 +92,8 @@ POST /projects/:id/freeze_periods | Attribute | Type | Required | Description | | -------------------| --------------- | -------- | -------------------------------------------------------------------------------------------------------------------------------- | | `id` | integer or string | yes | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding). | -| `freeze_start` | string | yes | Start of the Freeze Period in [cron](https://crontab.guru/) format. | -| `freeze_end` | string | yes | End of the Freeze Period in [cron](https://crontab.guru/) format. | +| `freeze_start` | string | yes | Start of the freeze period in [cron](https://crontab.guru/) format. | +| `freeze_end` | string | yes | End of the freeze period in [cron](https://crontab.guru/) format. | | `cron_timezone` | string | no | The time zone for the cron fields, defaults to UTC if not provided. | Example request: @@ -117,9 +117,9 @@ Example response: } ``` -## Update a Freeze Period +## Update a freeze period -Update a Freeze Period for the given `freeze_period_id`. +Update a freeze period for the given `freeze_period_id`. ```plaintext PUT /projects/:id/freeze_periods/:freeze_period_id @@ -128,9 +128,9 @@ PUT /projects/:id/freeze_periods/:freeze_period_id | Attribute | Type | Required | Description | | ------------- | --------------- | -------- | ----------------------------------------------------------------------------------------------------------- | | `id` | integer or string | yes | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding). | -| `freeze_period_id` | integer or string | yes | The database ID of the Freeze Period. | -| `freeze_start` | string | no | Start of the Freeze Period in [cron](https://crontab.guru/) format. | -| `freeze_end` | string | no | End of the Freeze Period in [cron](https://crontab.guru/) format. | +| `freeze_period_id` | integer | yes | The ID of the freeze period. | +| `freeze_start` | string | no | Start of the freeze period in [cron](https://crontab.guru/) format. | +| `freeze_end` | string | no | End of the freeze period in [cron](https://crontab.guru/) format. | | `cron_timezone` | string | no | The time zone for the cron fields. | Example request: @@ -154,9 +154,9 @@ Example response: } ``` -## Delete a Freeze Period +## Delete a freeze period -Delete a Freeze Period for the given `freeze_period_id`. +Delete a freeze period for the given `freeze_period_id`. ```plaintext DELETE /projects/:id/freeze_periods/:freeze_period_id @@ -165,7 +165,7 @@ DELETE /projects/:id/freeze_periods/:freeze_period_id | Attribute | Type | Required | Description | | ------------- | -------------- | -------- | ----------------------------------------------------------------------------------- | | `id` | integer or string | yes | The ID or [URL-encoded path of the project](index.md#namespaced-path-encoding). | -| `freeze_period_id` | string | yes | The database ID of the Freeze Period. | +| `freeze_period_id` | integer | yes | The ID of the freeze period. | Example request: diff --git a/doc/api/releases/links.md b/doc/api/releases/links.md index 050d7cabdf9..732f5ea57ef 100644 --- a/doc/api/releases/links.md +++ b/doc/api/releases/links.md @@ -13,9 +13,9 @@ links. For manipulating other Release assets, see [Release API](index.md). GitLab supports links to `http`, `https`, and `ftp` assets. -## Get links +## List links of a release -Get assets as links from a Release. +Get assets as links from a release. ```plaintext GET /projects/:id/releases/:tag_name/assets/links @@ -53,9 +53,9 @@ Example response: ] ``` -## Get a link +## Get a release link -Get an asset as a link from a Release. +Get an asset as a link from a release. ```plaintext GET /projects/:id/releases/:tag_name/assets/links/:link_id @@ -85,9 +85,9 @@ Example response: } ``` -## Create a link +## Create a release link -Create an asset as a link from a Release. +Creates an asset as a link from a release. ```plaintext POST /projects/:id/releases/:tag_name/assets/links @@ -126,9 +126,9 @@ Example response: } ``` -## Update a link +## Update a release link -Update an asset as a link from a Release. +Updates an asset as a link from a release. ```plaintext PUT /projects/:id/releases/:tag_name/assets/links/:link_id @@ -167,9 +167,9 @@ Example response: } ``` -## Delete a link +## Delete a release link -Delete an asset as a link from a Release. +Deletes an asset as a link from a release. ```plaintext DELETE /projects/:id/releases/:tag_name/assets/links/:link_id diff --git a/doc/user/project/releases/release_fields.md b/doc/user/project/releases/release_fields.md index 5ae1c69847d..c06e746268f 100644 --- a/doc/user/project/releases/release_fields.md +++ b/doc/user/project/releases/release_fields.md @@ -93,7 +93,7 @@ By default, GitLab fetches the release using `released_at` time. The use of the The assets associated with a release are accessible through a permanent URL. GitLab always redirects this URL to the actual asset location, so even if the assets move to a different location, you can continue -to use the same URL. This is defined during [link creation](../../../api/releases/links.md#create-a-link) or [updating](../../../api/releases/links.md#update-a-link) using the `filepath` API attribute. +to use the same URL. This is defined during [link creation](../../../api/releases/links.md#create-a-release-link) or [updating](../../../api/releases/links.md#update-a-release-link) using the `filepath` API attribute. The format of the URL is: @@ -133,7 +133,7 @@ The format of the URL is: https://host/namespace/project/-/releases/permalink/latest/downloads/:filepath ``` -If you have an asset with [`filepath`](../../../api/releases/links.md#create-a-link) for the `v11.9.0-rc2` latest release in the `gitlab-org` +If you have an asset with [`filepath`](../../../api/releases/links.md#create-a-release-link) for the `v11.9.0-rc2` latest release in the `gitlab-org` namespace and `gitlab-runner` project on `gitlab.com`, for example: ```json diff --git a/lib/banzai/filter/kroki_filter.rb b/lib/banzai/filter/kroki_filter.rb index 713ff2439fc..0ce70843675 100644 --- a/lib/banzai/filter/kroki_filter.rb +++ b/lib/banzai/filter/kroki_filter.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true -require "nokogiri" -require "asciidoctor/extensions/asciidoctor_kroki/extension" +require 'nokogiri' +require 'asciidoctor/extensions/asciidoctor_kroki/version' +require 'asciidoctor/extensions/asciidoctor_kroki/extension' module Banzai module Filter diff --git a/lib/banzai/filter/syntax_highlight_filter.rb b/lib/banzai/filter/syntax_highlight_filter.rb index 7175e99f1c7..7837959257b 100644 --- a/lib/banzai/filter/syntax_highlight_filter.rb +++ b/lib/banzai/filter/syntax_highlight_filter.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true require 'rouge/plugins/common_mark' -require "asciidoctor/extensions/asciidoctor_kroki/extension" +require 'asciidoctor/extensions/asciidoctor_kroki/version' +require 'asciidoctor/extensions/asciidoctor_kroki/extension' # Generated HTML is transformed back to GFM by app/assets/javascripts/behaviors/markdown/nodes/code_block.js module Banzai diff --git a/lib/gitlab/asciidoc.rb b/lib/gitlab/asciidoc.rb index a9c2dd001cb..d55f2bc8ac9 100644 --- a/lib/gitlab/asciidoc.rb +++ b/lib/gitlab/asciidoc.rb @@ -2,6 +2,7 @@ require 'asciidoctor' require 'asciidoctor-plantuml' +require 'asciidoctor/extensions/asciidoctor_kroki/version' require 'asciidoctor/extensions/asciidoctor_kroki/extension' require 'asciidoctor/extensions' require 'gitlab/asciidoc/html5_converter' diff --git a/lib/gitlab/kroki.rb b/lib/gitlab/kroki.rb index 6799be8e279..5fa77c1f1ba 100644 --- a/lib/gitlab/kroki.rb +++ b/lib/gitlab/kroki.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require 'asciidoctor/extensions/asciidoctor_kroki/version' require 'asciidoctor/extensions/asciidoctor_kroki/extension' module Gitlab diff --git a/spec/lib/gitlab/asciidoc_spec.rb b/spec/lib/gitlab/asciidoc_spec.rb index 8fec8bce23e..6dfc6abe965 100644 --- a/spec/lib/gitlab/asciidoc_spec.rb +++ b/spec/lib/gitlab/asciidoc_spec.rb @@ -567,7 +567,7 @@ module Gitlab it 'does not allow kroki-plantuml-include to be overridden' do input = <<~ADOC - [plantuml, test="{counter:kroki-plantuml-include:/etc/passwd}", format="png"] + [plantuml, test="{counter:kroki-plantuml-include:README.md}", format="png"] .... class BlockProcessor @@ -578,7 +578,7 @@ module Gitlab output = <<~HTML <div> <div> - <a class=\"no-attachment-icon\" href=\"https://kroki.io/plantuml/png/eNpLzkksLlZwyslPzg4oyk9OLS7OL-LiQuUr2NTo6ipUJ-eX5pWkFlllF-VnZ-oW5CTmlZTm5uhm5iXnlKak1gIABQEb8A==\" target=\"_blank\" rel=\"noopener noreferrer\"><img src=\"data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==\" alt=\"Diagram\" decoding=\"async\" class=\"lazy\" data-src=\"https://kroki.io/plantuml/png/eNpLzkksLlZwyslPzg4oyk9OLS7OL-LiQuUr2NTo6ipUJ-eX5pWkFlllF-VnZ-oW5CTmlZTm5uhm5iXnlKak1gIABQEb8A==\"></a> + <a class=\"no-attachment-icon\" href=\"https://kroki.io/plantuml/png/eNpLzkksLlZwyslPzg4oyk9OLS7OL-LiQuUr2NTo6ipUJ-eX5pWkFlllF-VnZ-oW5CTmlZTm5uhm5iXnlKak1gIABQEb8A==?test=README.md\" target=\"_blank\" rel=\"noopener noreferrer\"><img src=\"data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==\" alt=\"Diagram\" decoding=\"async\" class=\"lazy\" data-src=\"https://kroki.io/plantuml/png/eNpLzkksLlZwyslPzg4oyk9OLS7OL-LiQuUr2NTo6ipUJ-eX5pWkFlllF-VnZ-oW5CTmlZTm5uhm5iXnlKak1gIABQEb8A==?test=README.md\"></a> </div> </div> HTML diff --git a/spec/lib/gitlab/kroki_spec.rb b/spec/lib/gitlab/kroki_spec.rb index 7d29d018ff1..3d6ecf20377 100644 --- a/spec/lib/gitlab/kroki_spec.rb +++ b/spec/lib/gitlab/kroki_spec.rb @@ -6,7 +6,8 @@ RSpec.describe Gitlab::Kroki do describe '.formats' do def default_formats - %w[bytefield c4plantuml ditaa erd graphviz nomnoml pikchr plantuml svgbob umlet vega vegalite wavedrom].freeze + %w[bytefield c4plantuml ditaa erd graphviz nomnoml pikchr plantuml + structurizr svgbob umlet vega vegalite wavedrom].freeze end subject { described_class.formats(Gitlab::CurrentSettings) } diff --git a/spec/lib/gitlab/redis/multi_store_spec.rb b/spec/lib/gitlab/redis/multi_store_spec.rb index ff7b4be2754..82ae58783be 100644 --- a/spec/lib/gitlab/redis/multi_store_spec.rb +++ b/spec/lib/gitlab/redis/multi_store_spec.rb @@ -217,120 +217,80 @@ RSpec.describe Gitlab::Redis::MultiStore do allow(secondary_store).to receive(name).and_call_original end - context 'with feature flag :use_primary_and_secondary_stores_for_test_store' do - before do - stub_feature_flags(use_primary_and_secondary_stores_for_test_store: true) - end - - context 'when reading from the primary is successful' do - it 'returns the correct value' do - expect(primary_store).to receive(name).with(*args).and_call_original - - subject - end - - it 'does not execute on the secondary store' do - expect(secondary_store).not_to receive(name) + context 'when reading from the primary is successful' do + it 'returns the correct value' do + expect(primary_store).to receive(name).with(*args).and_call_original - subject - end - - include_examples 'reads correct value' + subject end - context 'when reading from primary instance is raising an exception' do - before do - allow(primary_store).to receive(name).with(*args).and_raise(StandardError) - allow(Gitlab::ErrorTracking).to receive(:log_exception) - end + it 'does not execute on the secondary store' do + expect(secondary_store).not_to receive(name) - it 'logs the exception' do - expect(Gitlab::ErrorTracking).to receive(:log_exception).with(an_instance_of(StandardError), - hash_including(:multi_store_error_message, instance_name: instance_name, command_name: name)) + subject + end - subject - end + include_examples 'reads correct value' + end - include_examples 'fallback read from the secondary store' + context 'when reading from primary instance is raising an exception' do + before do + allow(primary_store).to receive(name).with(*args).and_raise(StandardError) + allow(Gitlab::ErrorTracking).to receive(:log_exception) end - context 'when reading from primary instance return no value' do - before do - allow(primary_store).to receive(name).and_return(nil) - end + it 'logs the exception' do + expect(Gitlab::ErrorTracking).to receive(:log_exception).with(an_instance_of(StandardError), + hash_including(:multi_store_error_message, instance_name: instance_name, command_name: name)) - include_examples 'fallback read from the secondary store' + subject end - context 'when the command is executed within pipelined block' do - subject do - multi_store.pipelined do |pipeline| - pipeline.send(name, *args) - end - end + include_examples 'fallback read from the secondary store' + end - it 'is executed only 1 time on primary and secondary instance' do - expect(primary_store).to receive(:pipelined).and_call_original - expect(secondary_store).to receive(:pipelined).and_call_original + context 'when reading from primary instance return no value' do + before do + allow(primary_store).to receive(name).and_return(nil) + end - 2.times do - expect_next_instance_of(Redis::PipelinedConnection) do |pipeline| - expect(pipeline).to receive(name).with(*args).once.and_call_original - end - end + include_examples 'fallback read from the secondary store' + end - subject + context 'when the command is executed within pipelined block' do + subject do + multi_store.pipelined do |pipeline| + pipeline.send(name, *args) end end - if params[:block] - subject do - multi_store.send(name, *args, &block) - end + it 'is executed only 1 time on primary and secondary instance' do + expect(primary_store).to receive(:pipelined).and_call_original + expect(secondary_store).to receive(:pipelined).and_call_original - context 'when block is provided' do - it 'yields to the block' do - expect(primary_store).to receive(name).and_yield(value) - - subject + 2.times do + expect_next_instance_of(Redis::PipelinedConnection) do |pipeline| + expect(pipeline).to receive(name).with(*args).once.and_call_original end - - include_examples 'reads correct value' end - end - end - context 'with feature flag :use_primary_and_secondary_stores_for_test_store' do - before do - stub_feature_flags(use_primary_and_secondary_stores_for_test_store: false) + subject end + end - context 'with feature flag :use_primary_store_as_default_for_test_store is disabled' do - before do - stub_feature_flags(use_primary_store_as_default_for_test_store: false) - end - - it_behaves_like 'secondary store' + if params[:block] + subject do + multi_store.send(name, *args, &block) end - context 'with feature flag :use_primary_store_as_default_for_test_store is enabled' do - before do - stub_feature_flags(use_primary_store_as_default_for_test_store: true) - end - - it 'execute on the primary instance' do - expect(primary_store).to receive(name).with(*args).and_call_original + context 'when block is provided' do + it 'yields to the block' do + expect(primary_store).to receive(name).and_yield(value) subject end include_examples 'reads correct value' - - it 'does not execute on the secondary store' do - expect(secondary_store).not_to receive(name) - - subject - end end end @@ -411,100 +371,58 @@ RSpec.describe Gitlab::Redis::MultiStore do allow(secondary_store).to receive(name).and_call_original end - context 'with feature flag :use_primary_and_secondary_stores_for_test_store' do - before do - stub_feature_flags(use_primary_and_secondary_stores_for_test_store: true) - end - - context 'when executing on primary instance is successful' do - it 'executes on both primary and secondary redis store', :aggregate_errors do - expect(primary_store).to receive(name).with(*expected_args).and_call_original - expect(secondary_store).to receive(name).with(*expected_args).and_call_original - - subject - end - - include_examples 'verify that store contains values', :primary_store - include_examples 'verify that store contains values', :secondary_store - end - - context 'when executing on the primary instance is raising an exception' do - before do - allow(primary_store).to receive(name).with(*expected_args).and_raise(StandardError) - allow(Gitlab::ErrorTracking).to receive(:log_exception) - end - - it 'logs the exception and execute on secondary instance', :aggregate_errors do - expect(Gitlab::ErrorTracking).to receive(:log_exception).with(an_instance_of(StandardError), - hash_including(:multi_store_error_message, command_name: name, instance_name: instance_name)) - expect(secondary_store).to receive(name).with(*expected_args).and_call_original - - subject - end + context 'when executing on primary instance is successful' do + it 'executes on both primary and secondary redis store', :aggregate_errors do + expect(primary_store).to receive(name).with(*expected_args).and_call_original + expect(secondary_store).to receive(name).with(*expected_args).and_call_original - include_examples 'verify that store contains values', :secondary_store + subject end - context 'when the command is executed within pipelined block' do - subject do - multi_store.pipelined do |pipeline| - pipeline.send(name, *args) - end - end - - it 'is executed only 1 time on each instance', :aggregate_errors do - expect(primary_store).to receive(:pipelined).and_call_original - expect_next_instance_of(Redis::PipelinedConnection) do |pipeline| - expect(pipeline).to receive(name).with(*expected_args).once.and_call_original - end - - expect(secondary_store).to receive(:pipelined).and_call_original - expect_next_instance_of(Redis::PipelinedConnection) do |pipeline| - expect(pipeline).to receive(name).with(*expected_args).once.and_call_original - end - - subject - end - - include_examples 'verify that store contains values', :primary_store - include_examples 'verify that store contains values', :secondary_store - end + include_examples 'verify that store contains values', :primary_store + include_examples 'verify that store contains values', :secondary_store end - context 'with feature flag :use_primary_and_secondary_stores_for_test_store is disabled' do + context 'when executing on the primary instance is raising an exception' do before do - stub_feature_flags(use_primary_and_secondary_stores_for_test_store: false) + allow(primary_store).to receive(name).with(*expected_args).and_raise(StandardError) + allow(Gitlab::ErrorTracking).to receive(:log_exception) end - context 'with feature flag :use_primary_store_as_default_for_test_store is disabled' do - before do - stub_feature_flags(use_primary_store_as_default_for_test_store: false) - end + it 'logs the exception and execute on secondary instance', :aggregate_errors do + expect(Gitlab::ErrorTracking).to receive(:log_exception).with(an_instance_of(StandardError), + hash_including(:multi_store_error_message, command_name: name, instance_name: instance_name)) + expect(secondary_store).to receive(name).with(*expected_args).and_call_original - it 'executes only on the secondary redis store', :aggregate_errors do - expect(secondary_store).to receive(name).with(*expected_args) - expect(primary_store).not_to receive(name).with(*expected_args) + subject + end - subject - end + include_examples 'verify that store contains values', :secondary_store + end - include_examples 'verify that store contains values', :secondary_store + context 'when the command is executed within pipelined block' do + subject do + multi_store.pipelined do |pipeline| + pipeline.send(name, *args) + end end - context 'with feature flag :use_primary_store_as_default_for_test_store is enabled' do - before do - stub_feature_flags(use_primary_store_as_default_for_test_store: true) + it 'is executed only 1 time on each instance', :aggregate_errors do + expect(primary_store).to receive(:pipelined).and_call_original + expect_next_instance_of(Redis::PipelinedConnection) do |pipeline| + expect(pipeline).to receive(name).with(*expected_args).once.and_call_original end - it 'executes only on the primary_redis redis store', :aggregate_errors do - expect(primary_store).to receive(name).with(*expected_args) - expect(secondary_store).not_to receive(name).with(*expected_args) - - subject + expect(secondary_store).to receive(:pipelined).and_call_original + expect_next_instance_of(Redis::PipelinedConnection) do |pipeline| + expect(pipeline).to receive(name).with(*expected_args).once.and_call_original end - include_examples 'verify that store contains values', :primary_store + subject end + + include_examples 'verify that store contains values', :primary_store + include_examples 'verify that store contains values', :secondary_store end end end @@ -539,151 +457,109 @@ RSpec.describe Gitlab::Redis::MultiStore do end end - context 'with feature flag :use_primary_and_secondary_stores_for_test_store' do - before do - stub_feature_flags(use_primary_and_secondary_stores_for_test_store: true) - end + context 'when executing on primary instance is successful' do + it 'executes on both primary and secondary redis store', :aggregate_errors do + expect(primary_store).to receive(name).and_call_original + expect(secondary_store).to receive(name).and_call_original - context 'when executing on primary instance is successful' do - it 'executes on both primary and secondary redis store', :aggregate_errors do - expect(primary_store).to receive(name).and_call_original - expect(secondary_store).to receive(name).and_call_original - - subject - end - - include_examples 'verify that store contains values', :primary_store - include_examples 'verify that store contains values', :secondary_store + subject end - context 'when executing on the primary instance is raising an exception' do - before do - allow(primary_store).to receive(name).and_raise(StandardError) - allow(Gitlab::ErrorTracking).to receive(:log_exception) - end - - it 'logs the exception and execute on secondary instance', :aggregate_errors do - expect(Gitlab::ErrorTracking).to receive(:log_exception).with(an_instance_of(StandardError), - hash_including(:multi_store_error_message, command_name: name)) - expect(secondary_store).to receive(name).and_call_original - - subject - end + include_examples 'verify that store contains values', :primary_store + include_examples 'verify that store contains values', :secondary_store + end - include_examples 'verify that store contains values', :secondary_store + context 'when executing on the primary instance is raising an exception' do + before do + allow(primary_store).to receive(name).and_raise(StandardError) + allow(Gitlab::ErrorTracking).to receive(:log_exception) end - describe 'return values from a pipelined command' do - RSpec::Matchers.define :pipeline_diff_error_with_stacktrace do |message| - match do |object| - expect(object).to be_a(Gitlab::Redis::MultiStore::PipelinedDiffError) - expect(object.backtrace).not_to be_nil - expect(object.message).to eq(message) - end - end - - subject do - multi_store.send(name) do |redis| - redis.get(key1) - end - end + it 'logs the exception and execute on secondary instance', :aggregate_errors do + expect(Gitlab::ErrorTracking).to receive(:log_exception).with(an_instance_of(StandardError), + hash_including(:multi_store_error_message, command_name: name)) + expect(secondary_store).to receive(name).and_call_original - context 'when the value exists on both and are equal' do - before do - primary_store.set(key1, value1) - secondary_store.set(key1, value1) - end + subject + end - it 'returns the value' do - expect(Gitlab::ErrorTracking).not_to receive(:log_exception) + include_examples 'verify that store contains values', :secondary_store + end - expect(subject).to eq([value1]) - end + describe 'return values from a pipelined command' do + RSpec::Matchers.define :pipeline_diff_error_with_stacktrace do |message| + match do |object| + expect(object).to be_a(Gitlab::Redis::MultiStore::PipelinedDiffError) + expect(object.backtrace).not_to be_nil + expect(object.message).to eq(message) end + end - context 'when the value exists on both but differ' do - before do - primary_store.set(key1, value1) - secondary_store.set(key1, value2) - end - - it 'returns the value from the secondary store, logging an error' do - expect(Gitlab::ErrorTracking).to receive(:log_exception).with( - pipeline_diff_error_with_stacktrace( - 'Pipelined command executed on both stores successfully but results differ between them. ' \ - "Result from the primary: [#{value1.inspect}]. Result from the secondary: [#{value2.inspect}]." - ), - hash_including(command_name: name, instance_name: instance_name) - ).and_call_original - expect(counter).to receive(:increment).with(command: name, instance_name: instance_name) - - expect(subject).to eq([value2]) - end + subject do + multi_store.send(name) do |redis| + redis.get(key1) end + end - context 'when the value does not exist on the primary but it does on the secondary' do - before do - secondary_store.set(key1, value2) - end - - it 'returns the value from the secondary store, logging an error' do - expect(Gitlab::ErrorTracking).to receive(:log_exception).with( - pipeline_diff_error_with_stacktrace( - 'Pipelined command executed on both stores successfully but results differ between them. ' \ - "Result from the primary: [nil]. Result from the secondary: [#{value2.inspect}]." - ), - hash_including(command_name: name, instance_name: instance_name) - ) - expect(counter).to receive(:increment).with(command: name, instance_name: instance_name) - - expect(subject).to eq([value2]) - end + context 'when the value exists on both and are equal' do + before do + primary_store.set(key1, value1) + secondary_store.set(key1, value1) end - context 'when the value does not exist in either' do - it 'returns nil without logging an error' do - expect(Gitlab::ErrorTracking).not_to receive(:log_exception) - expect(counter).not_to receive(:increment) + it 'returns the value' do + expect(Gitlab::ErrorTracking).not_to receive(:log_exception) - expect(subject).to eq([nil]) - end + expect(subject).to eq([value1]) end end - end - context 'with feature flag :use_primary_and_secondary_stores_for_test_store is disabled' do - before do - stub_feature_flags(use_primary_and_secondary_stores_for_test_store: false) - end - - context 'with feature flag :use_primary_store_as_default_for_test_store is disabled' do + context 'when the value exists on both but differ' do before do - stub_feature_flags(use_primary_store_as_default_for_test_store: false) + primary_store.set(key1, value1) + secondary_store.set(key1, value2) end - it 'executes only on the secondary redis store', :aggregate_errors do - expect(secondary_store).to receive(name) - expect(primary_store).not_to receive(name) - - subject + it 'returns the value from the secondary store, logging an error' do + expect(Gitlab::ErrorTracking).to receive(:log_exception).with( + pipeline_diff_error_with_stacktrace( + 'Pipelined command executed on both stores successfully but results differ between them. ' \ + "Result from the primary: [#{value1.inspect}]. Result from the secondary: [#{value2.inspect}]." + ), + hash_including(command_name: name, instance_name: instance_name) + ).and_call_original + expect(counter).to receive(:increment).with(command: name, instance_name: instance_name) + + expect(subject).to eq([value2]) end - - include_examples 'verify that store contains values', :secondary_store end - context 'with feature flag :use_primary_store_as_default_for_test_store is enabled' do + context 'when the value does not exist on the primary but it does on the secondary' do before do - stub_feature_flags(use_primary_store_as_default_for_test_store: true) + secondary_store.set(key1, value2) end - it 'executes only on the primary_redis redis store', :aggregate_errors do - expect(primary_store).to receive(name) - expect(secondary_store).not_to receive(name) - - subject + it 'returns the value from the secondary store, logging an error' do + expect(Gitlab::ErrorTracking).to receive(:log_exception).with( + pipeline_diff_error_with_stacktrace( + 'Pipelined command executed on both stores successfully but results differ between them. ' \ + "Result from the primary: [nil]. Result from the secondary: [#{value2.inspect}]." + ), + hash_including(command_name: name, instance_name: instance_name) + ) + expect(counter).to receive(:increment).with(command: name, instance_name: instance_name) + + expect(subject).to eq([value2]) end + end - include_examples 'verify that store contains values', :primary_store + context 'when the value does not exist in either' do + it 'returns nil without logging an error' do + expect(Gitlab::ErrorTracking).not_to receive(:log_exception) + expect(counter).not_to receive(:increment) + + expect(subject).to eq([nil]) + end end end end @@ -827,40 +703,8 @@ RSpec.describe Gitlab::Redis::MultiStore do describe '#to_s' do subject { multi_store.to_s } - context 'with feature flag :use_primary_and_secondary_stores_for_test_store is enabled' do - before do - stub_feature_flags(use_primary_and_secondary_stores_for_test_store: true) - end - - it 'returns same value as primary_store' do - is_expected.to eq(primary_store.to_s) - end - end - - context 'with feature flag :use_primary_and_secondary_stores_for_test_store is disabled' do - before do - stub_feature_flags(use_primary_and_secondary_stores_for_test_store: false) - end - - context 'with feature flag :use_primary_store_as_default_for_test_store is enabled' do - before do - stub_feature_flags(use_primary_store_as_default_for_test_store: true) - end - - it 'returns same value as primary_store' do - is_expected.to eq(primary_store.to_s) - end - end - - context 'with feature flag :use_primary_store_as_default_for_test_store is disabled' do - before do - stub_feature_flags(use_primary_store_as_default_for_test_store: false) - end - - it 'returns same value as primary_store' do - is_expected.to eq(secondary_store.to_s) - end - end + it 'returns same value as primary_store' do + is_expected.to eq(primary_store.to_s) end end @@ -871,24 +715,8 @@ RSpec.describe Gitlab::Redis::MultiStore do end describe '#use_primary_and_secondary_stores?' do - context 'with feature flag :use_primary_and_secondary_stores_for_test_store is enabled' do - before do - stub_feature_flags(use_primary_and_secondary_stores_for_test_store: true) - end - - it 'multi store is disabled' do - expect(multi_store.use_primary_and_secondary_stores?).to be true - end - end - - context 'with feature flag :use_primary_and_secondary_stores_for_test_store is disabled' do - before do - stub_feature_flags(use_primary_and_secondary_stores_for_test_store: false) - end - - it 'multi store is disabled' do - expect(multi_store.use_primary_and_secondary_stores?).to be false - end + it 'multi store is enabled' do + expect(multi_store.use_primary_and_secondary_stores?).to be true end context 'with empty DB' do @@ -913,24 +741,8 @@ RSpec.describe Gitlab::Redis::MultiStore do end describe '#use_primary_store_as_default?' do - context 'with feature flag :use_primary_store_as_default_for_test_store is enabled' do - before do - stub_feature_flags(use_primary_store_as_default_for_test_store: true) - end - - it 'multi store is disabled' do - expect(multi_store.use_primary_store_as_default?).to be true - end - end - - context 'with feature flag :use_primary_store_as_default_for_test_store is disabled' do - before do - stub_feature_flags(use_primary_store_as_default_for_test_store: false) - end - - it 'multi store is disabled' do - expect(multi_store.use_primary_store_as_default?).to be false - end + it 'multi store is disabled' do + expect(multi_store.use_primary_store_as_default?).to be true end context 'with empty DB' do diff --git a/spec/services/members/update_service_spec.rb b/spec/services/members/update_service_spec.rb index f919d6d1516..eca40860bda 100644 --- a/spec/services/members/update_service_spec.rb +++ b/spec/services/members/update_service_spec.rb @@ -14,6 +14,11 @@ RSpec.describe Members::UpdateService do { access_level: access_level } end + before do + project.add_developer(member_user) + group.add_developer(member_user) + end + subject { described_class.new(current_user, params).execute(member, permission: permission) } shared_examples 'a service raising Gitlab::Access::AccessDeniedError' do @@ -83,11 +88,6 @@ RSpec.describe Members::UpdateService do end end - before do - project.add_developer(member_user) - group.add_developer(member_user) - end - context 'when current user cannot update the given member' do it_behaves_like 'a service raising Gitlab::Access::AccessDeniedError' do let(:source) { project } @@ -183,4 +183,54 @@ RSpec.describe Members::UpdateService do end end end + + context 'authorization updates' do + let_it_be(:user) { create(:user) } + + shared_examples 'manages authorization updates' do + context 'access level changes' do + let(:params) do + { access_level: Gitlab::Access::MAINTAINER } + end + + it 'authorization update callback is triggered' do + expect(member).to receive(:refresh_member_authorized_projects).once + + described_class.new(current_user, params).execute(member, permission: permission) + end + end + + context 'no attribute changes' do + let(:params) do + { access_level: Gitlab::Access::DEVELOPER } + end + + it 'authorization update callback is not triggered' do + expect(member).not_to receive(:refresh_member_authorized_projects) + + described_class.new(current_user, params).execute(member, permission: permission) + end + end + end + + context 'group member' do + let(:source) { group } + + before do + group.add_owner(current_user) + end + + include_examples 'manages authorization updates' + end + + context 'project member' do + let(:source) { project } + + before do + project.add_maintainer(current_user) + end + + include_examples 'manages authorization updates' + end + end end |