diff options
19 files changed, 82 insertions, 72 deletions
diff --git a/app/controllers/projects/uploads_controller.rb b/app/controllers/projects/uploads_controller.rb index a364668ea5f..8f4987a07f6 100644 --- a/app/controllers/projects/uploads_controller.rb +++ b/app/controllers/projects/uploads_controller.rb @@ -35,14 +35,4 @@ class Projects::UploadsController < Projects::ApplicationController Project.find_by_full_path("#{namespace}/#{id}") end - - # Overrides ApplicationController#build_canonical_path since there are - # multiple routes that match project uploads: - # https://gitlab.com/gitlab-org/gitlab/issues/196396 - def build_canonical_path(project) - return super unless action_name == 'show' - return super unless params[:secret] && params[:filename] - - show_namespace_project_uploads_url(project.namespace.to_param, project.to_param, params[:secret], params[:filename]) - end end diff --git a/app/models/issue.rb b/app/models/issue.rb index ae76df30007..b988f7fb727 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -396,8 +396,6 @@ class Issue < ApplicationRecord attribute_name: 'relative_position', column_expression: arel_table[:relative_position], order_expression: Issue.arel_table[:relative_position].asc.nulls_last, - reversed_order_expression: Issue.arel_table[:relative_position].desc.nulls_last, - order_direction: :asc, nullable: :nulls_last, distinct: false ) diff --git a/app/models/merge_request_assignee.rb b/app/models/merge_request_assignee.rb index 3e481e35deb..be3a1d42eac 100644 --- a/app/models/merge_request_assignee.rb +++ b/app/models/merge_request_assignee.rb @@ -1,6 +1,9 @@ # frozen_string_literal: true class MergeRequestAssignee < ApplicationRecord + include IgnorableColumns + ignore_column %i[state updated_state_by_user_id], remove_with: '15.6', remove_after: '2022-10-22' + belongs_to :merge_request, touch: true belongs_to :assignee, class_name: "User", foreign_key: :user_id, inverse_of: :merge_request_assignees diff --git a/app/models/merge_request_reviewer.rb b/app/models/merge_request_reviewer.rb index 4abf0fa09f0..4b5b71481d3 100644 --- a/app/models/merge_request_reviewer.rb +++ b/app/models/merge_request_reviewer.rb @@ -2,6 +2,8 @@ class MergeRequestReviewer < ApplicationRecord include MergeRequestReviewerState + include IgnorableColumns + ignore_column :updated_state_by_user_id, remove_with: '15.6', remove_after: '2022-10-22' belongs_to :merge_request belongs_to :reviewer, class_name: 'User', foreign_key: :user_id, inverse_of: :merge_request_reviewers diff --git a/config/routes/uploads.rb b/config/routes/uploads.rb index ba2e8493ef9..7b598e84975 100644 --- a/config/routes/uploads.rb +++ b/config/routes/uploads.rb @@ -22,13 +22,6 @@ scope path: :uploads do constraints: { model: /appearance/, mounted_as: /logo|header_logo|favicon/, filename: /.+/ }, as: 'appearance_upload' - # Project markdown uploads - # DEPRECATED: Remove this in GitLab 13.0 because this is redundant to show_namespace_project_uploads - # https://gitlab.com/gitlab-org/gitlab/issues/196396 - get ":namespace_id/:project_id/:secret/:filename", - to: redirect("%{namespace_id}/%{project_id}/uploads/%{secret}/%{filename}"), - constraints: { namespace_id: /[a-zA-Z.0-9_\-]+/, project_id: /[a-zA-Z.0-9_\-]+/, filename: %r{[^/]+} }, format: false, defaults: { format: nil } - # create uploads for models, snippets (notes) available for now post ':model', to: 'uploads#create', diff --git a/db/post_migrate/20220606080509_add_tmp_index_job_artifacts_id_and_expire_at.rb b/db/post_migrate/20220606054503_add_tmp_index_job_artifacts_id_and_expire_at.rb index 28346eb1a97..28346eb1a97 100644 --- a/db/post_migrate/20220606080509_add_tmp_index_job_artifacts_id_and_expire_at.rb +++ b/db/post_migrate/20220606054503_add_tmp_index_job_artifacts_id_and_expire_at.rb diff --git a/db/post_migrate/20220606054503_fix_incorrect_job_artifacts_expire_at.rb b/db/post_migrate/20220606080509_fix_incorrect_job_artifacts_expire_at.rb index 8fea22f5579..8fea22f5579 100644 --- a/db/post_migrate/20220606054503_fix_incorrect_job_artifacts_expire_at.rb +++ b/db/post_migrate/20220606080509_fix_incorrect_job_artifacts_expire_at.rb diff --git a/doc/api/groups.md b/doc/api/groups.md index 278dc0093e6..b5e6fb15204 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -1469,7 +1469,7 @@ To delete the LDAP group link, provide either a `cn` or a `filter`, but not both ## SAML Group Links **(PREMIUM)** > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/290367) in GitLab 15.3.0. -> - `access_level` type [changed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/95607) from `string` to `integer` in GitLab 15.3.2. +> - `access_level` type [changed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/95607) from `string` to `integer` in GitLab 15.3.3. List, get, add, and delete SAML group links. @@ -1492,7 +1492,7 @@ If successful, returns [`200`](index.md#status-codes) and the following response | Attribute | Type | Description | |:-------------------|:--------|:-----------------------------------------------------------------------------| | `[].name` | string | Name of the SAML group | -| `[].access_level` | integer | [Access level](members.md#valid-access-levels) for members of the SAML group. The attribute had a string type from GitLab 15.3.0 to GitLab 15.3.2 | +| `[].access_level` | integer | [Access level](members.md#valid-access-levels) for members of the SAML group. The attribute had a string type from GitLab 15.3.0 to GitLab 15.3.3 | Example request: @@ -1535,7 +1535,7 @@ If successful, returns [`200`](index.md#status-codes) and the following response | Attribute | Type | Description | |:---------------|:--------|:-----------------------------------------------------------------------------| | `name` | string | Name of the SAML group | -| `access_level` | integer | [Access level](members.md#valid-access-levels) for members of the SAML group. The attribute had a string type from GitLab 15.3.0 to GitLab 15.3.2 | +| `access_level` | integer | [Access level](members.md#valid-access-levels) for members of the SAML group. The attribute had a string type from GitLab 15.3.0 to GitLab 15.3.3 | Example request: @@ -1573,7 +1573,7 @@ If successful, returns [`201`](index.md#status-codes) and the following response | Attribute | Type | Description | |:---------------|:--------|:-----------------------------------------------------------------------------| | `name` | string | Name of the SAML group | -| `access_level` | integer | [Access level](members.md#valid-access-levels) for members of the for members of the SAML group. The attribute had a string type from GitLab 15.3.0 to GitLab 15.3.2 | +| `access_level` | integer | [Access level](members.md#valid-access-levels) for members of the for members of the SAML group. The attribute had a string type from GitLab 15.3.0 to GitLab 15.3.3 | Example request: diff --git a/doc/update/index.md b/doc/update/index.md index c121c5b5d5e..4ce90a9a116 100644 --- a/doc/update/index.md +++ b/doc/update/index.md @@ -470,9 +470,9 @@ and [Helm Chart deployments](https://docs.gitlab.com/charts/). They come with ap - GitLab 15.4.0 includes a [batched background migration](#batched-background-migrations) to [remove incorrect values from `expire_at` in `ci_job_artifacts` table](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/89318). This migration might take hours or days to complete on larger GitLab instances. -### 15.3.2 +### 15.3.3 -In GitLab 15.3.2, [SAML Group Links](../api/groups.md#saml-group-links) API `access_level` attribute type changed to `integer`. See +In GitLab 15.3.3, [SAML Group Links](../api/groups.md#saml-group-links) API `access_level` attribute type changed to `integer`. See [valid access levels](../api/members.md#valid-access-levels) documentation. ### 15.2.0 diff --git a/lib/gitlab/import_export/project/import_export.yml b/lib/gitlab/import_export/project/import_export.yml index bf969ed0da4..77311e14803 100644 --- a/lib/gitlab/import_export/project/import_export.yml +++ b/lib/gitlab/import_export/project/import_export.yml @@ -606,7 +606,6 @@ included_attributes: merge_request_assignees: - :user_id - :created_at - - :state merge_request_reviewers: - :user_id - :created_at diff --git a/lib/gitlab/pagination/keyset/column_order_definition.rb b/lib/gitlab/pagination/keyset/column_order_definition.rb index 302e7b406b1..d1fe1d2dfc1 100644 --- a/lib/gitlab/pagination/keyset/column_order_definition.rb +++ b/lib/gitlab/pagination/keyset/column_order_definition.rb @@ -213,7 +213,7 @@ module Gitlab attr_reader :reversed_order_expression, :nullable, :distinct def calculate_reversed_order(order_expression) - unless AREL_ORDER_CLASSES.has_key?(order_expression.class) # Arel can reverse simple orders + unless order_expression.is_a?(Arel::Nodes::Ordering) raise "Couldn't determine reversed order for `#{order_expression}`, please provide the `reversed_order_expression` parameter." end @@ -229,10 +229,10 @@ module Gitlab end def parse_order_direction(order_expression, order_direction) - transformed_order_direction = if order_direction.nil? && AREL_ORDER_CLASSES[order_expression.class] - AREL_ORDER_CLASSES[order_expression.class] - elsif order_direction.present? + transformed_order_direction = if order_direction.present? order_direction.to_s.downcase.to_sym + elsif order_expression.is_a?(Arel::Nodes::Ordering) + AREL_ORDER_CLASSES[order_expression.class] || AREL_ORDER_CLASSES[order_expression.value.class] end unless REVERSED_ORDER_DIRECTIONS.has_key?(transformed_order_direction) diff --git a/spec/lib/gitlab/pagination/keyset/column_order_definition_spec.rb b/spec/lib/gitlab/pagination/keyset/column_order_definition_spec.rb index 778244677ef..100574cc75f 100644 --- a/spec/lib/gitlab/pagination/keyset/column_order_definition_spec.rb +++ b/spec/lib/gitlab/pagination/keyset/column_order_definition_spec.rb @@ -50,6 +50,20 @@ RSpec.describe Gitlab::Pagination::Keyset::ColumnOrderDefinition do it { expect(project_calculated_column).to be_ascending_order } it { expect(project_calculated_column).not_to be_descending_order } + context 'when order expression is an Arel node with nulls_last' do + it 'can automatically determine the reversed expression' do + column_order_definition = described_class.new( + attribute_name: :name, + column_expression: Project.arel_table[:name], + order_expression: Project.arel_table[:name].asc.nulls_last, + nullable: :nulls_last, + distinct: false + ) + + expect(column_order_definition).to be_ascending_order + end + end + it 'raises error when order direction cannot be infered' do expect do described_class.new( @@ -132,6 +146,21 @@ RSpec.describe Gitlab::Pagination::Keyset::ColumnOrderDefinition do expect(column_order_definition.reverse.order_expression).to eq('name desc') end end + + context 'when order expression is an Arel node with nulls_last' do + it 'can automatically determine the reversed expression' do + column_order_definition = described_class.new( + attribute_name: :name, + column_expression: Project.arel_table[:name], + order_expression: Project.arel_table[:name].asc.nulls_last, + order_direction: :asc, + nullable: :nulls_last, + distinct: false + ) + + expect(column_order_definition.reverse.order_expression).to eq(Project.arel_table[:name].desc.nulls_first) + end + end end describe '#nullable' do diff --git a/spec/migrations/20220606054503_fix_incorrect_job_artifacts_expire_at_spec.rb b/spec/migrations/20220606080509_fix_incorrect_job_artifacts_expire_at_spec.rb index 5921dd64c0e..5921dd64c0e 100644 --- a/spec/migrations/20220606054503_fix_incorrect_job_artifacts_expire_at_spec.rb +++ b/spec/migrations/20220606080509_fix_incorrect_job_artifacts_expire_at_spec.rb diff --git a/spec/requests/api/graphql/project/issues_spec.rb b/spec/requests/api/graphql/project/issues_spec.rb index d0ed4212349..28282860416 100644 --- a/spec/requests/api/graphql/project/issues_spec.rb +++ b/spec/requests/api/graphql/project/issues_spec.rb @@ -301,7 +301,7 @@ RSpec.describe 'getting an issue list for a project' do let_it_be(:relative_issue5) { create(:issue, project: sort_project, relative_position: 500) } context 'when ascending' do - it_behaves_like 'sorted paginated query' do + it_behaves_like 'sorted paginated query', is_reversible: true do let(:sort_param) { :RELATIVE_POSITION_ASC } let(:first_param) { 2 } let(:all_records) do diff --git a/spec/routing/uploads_routing_spec.rb b/spec/routing/uploads_routing_spec.rb index 41646d1b515..9eb421ec7d0 100644 --- a/spec/routing/uploads_routing_spec.rb +++ b/spec/routing/uploads_routing_spec.rb @@ -39,12 +39,4 @@ RSpec.describe 'Uploads', 'routing' do expect(post("/uploads/#{model}?id=1")).not_to be_routable end end - - describe 'legacy paths' do - include RSpec::Rails::RequestExampleGroup - - it 'redirects project uploads to canonical path under project namespace' do - expect(get('/uploads/namespace/project/12345/test.png')).to redirect_to('/namespace/project/uploads/12345/test.png') - end - end end diff --git a/workhorse/Makefile b/workhorse/Makefile index fe9bf639753..6bd80a981bb 100644 --- a/workhorse/Makefile +++ b/workhorse/Makefile @@ -112,8 +112,7 @@ clean-build: rm -rf $(TARGET_DIR) .PHONY: prepare-tests -prepare-tests: testdata/data/group/test.git $(EXE_ALL) -prepare-tests: testdata/scratch +prepare-tests: testdata/scratch $(EXE_ALL) .PHONY: run-gitaly run-gitaly: $(GITALY_PID_FILE) @@ -130,10 +129,6 @@ gitaly.toml: ../tmp/tests/gitaly/config.toml $(call message, "Building a complete test environment") cd .. ; ./scripts/setup-test-env -testdata/data/group/test.git: - $(call message,$@) - git clone --quiet --bare https://gitlab.com/gitlab-org/gitlab-test.git $@ - testdata/scratch: mkdir -p testdata/scratch diff --git a/workhorse/gitaly_integration_test.go b/workhorse/gitaly_integration_test.go index d578ae50765..a2826c3edc4 100644 --- a/workhorse/gitaly_integration_test.go +++ b/workhorse/gitaly_integration_test.go @@ -18,6 +18,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v15/proto/go/gitalypb" + "gitlab.com/gitlab-org/gitaly/v15/streamio" "gitlab.com/gitlab-org/gitlab/workhorse/internal/api" "gitlab.com/gitlab-org/gitlab/workhorse/internal/gitaly" @@ -78,13 +79,28 @@ func ensureGitalyRepository(t *testing.T, apiResponse *api.Response) error { return err } - createReq := &gitalypb.CreateRepositoryFromURLRequest{ - Repository: &apiResponse.Repository, - Url: "https://gitlab.com/gitlab-org/gitlab-test.git", + stream, err := repository.CreateRepositoryFromBundle(ctx) + if err != nil { + return fmt.Errorf("initiate stream: %w", err) + } + + if err := stream.Send(&gitalypb.CreateRepositoryFromBundleRequest{Repository: &apiResponse.Repository}); err != nil { + return err + } + + gitBundle := exec.Command("git", "-C", path.Join(testRepoRoot, testRepo), "bundle", "create", "-", "--all") + gitBundle.Stdout = streamio.NewWriter(func(p []byte) error { + return stream.Send(&gitalypb.CreateRepositoryFromBundleRequest{Data: p}) + }) + + if err := gitBundle.Run(); err != nil { + return fmt.Errorf("run git bundle --create: %w", err) + } + if _, err := stream.CloseAndRecv(); err != nil { + return fmt.Errorf("finish CreateRepositoryFromBundle: %w", err) } - _, err = repository.CreateRepositoryFromURL(ctx, createReq) - return err + return nil } func TestAllowedClone(t *testing.T) { @@ -282,24 +298,22 @@ func TestAllowedGetGitDiff(t *testing.T) { apiResponse := realGitalyOkBody(t) require.NoError(t, ensureGitalyRepository(t, apiResponse)) - leftCommit := "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab" - rightCommit := "e395f646b1499e8e0279445fc99a0596a65fab7e" - expectedBody := "diff --git a/README.md b/README.md" - msg := serializedMessage("RawDiffRequest", &gitalypb.RawDiffRequest{ Repository: &apiResponse.Repository, - LeftCommitId: leftCommit, - RightCommitId: rightCommit, + LeftCommitId: "b0e52af38d7ea43cf41d8a6f2471351ac036d6c9", + RightCommitId: "732401c65e924df81435deb12891ef570167d2e2", }) jsonParams := buildGitalyRPCParams(gitalyAddress, msg) resp, body, err := doSendDataRequest("/something", "git-diff", jsonParams) require.NoError(t, err) - shortBody := string(body[:len(expectedBody)]) require.Equal(t, 200, resp.StatusCode, "GET %q: status code", resp.Request.URL) - require.Equal(t, expectedBody, shortBody, "GET %q: response body", resp.Request.URL) requireNginxResponseBuffering(t, "no", resp, "GET %q: nginx response buffering", resp.Request.URL) + + expectedBody := "diff --git a/LICENSE b/LICENSE\n" + require.Equal(t, expectedBody, string(body[:len(expectedBody)]), + "GET %q: response body", resp.Request.URL) } func TestAllowedGetGitFormatPatch(t *testing.T) { @@ -309,12 +323,10 @@ func TestAllowedGetGitFormatPatch(t *testing.T) { apiResponse := realGitalyOkBody(t) require.NoError(t, ensureGitalyRepository(t, apiResponse)) - leftCommit := "8a0f2ee90d940bfb0ba1e14e8214b0649056e4ab" - rightCommit := "e395f646b1499e8e0279445fc99a0596a65fab7e" msg := serializedMessage("RawPatchRequest", &gitalypb.RawPatchRequest{ Repository: &apiResponse.Repository, - LeftCommitId: leftCommit, - RightCommitId: rightCommit, + LeftCommitId: "b0e52af38d7ea43cf41d8a6f2471351ac036d6c9", + RightCommitId: "0e1b353b348f8477bdbec1ef47087171c5032cd9", }) jsonParams := buildGitalyRPCParams(gitalyAddress, msg) @@ -324,14 +336,10 @@ func TestAllowedGetGitFormatPatch(t *testing.T) { require.Equal(t, 200, resp.StatusCode, "GET %q: status code", resp.Request.URL) requireNginxResponseBuffering(t, "no", resp, "GET %q: nginx response buffering", resp.Request.URL) - requirePatchSeries( - t, - body, - "372ab6950519549b14d220271ee2322caa44d4eb", - "57290e673a4c87f51294f5216672cbc58d485d25", - "41ae11ba5d091d73d5de671f6fa7d1a4539e979e", - "742518b2be68fc750bb4c357c0df821a88113286", - rightCommit, + requirePatchSeries(t, body, + "732401c65e924df81435deb12891ef570167d2e2", + "33bcff41c232a11727ac6d660bd4b0c2ba86d63d", + "0e1b353b348f8477bdbec1ef47087171c5032cd9", ) } diff --git a/workhorse/go.mod b/workhorse/go.mod index bcb529caa01..c264dacbee6 100644 --- a/workhorse/go.mod +++ b/workhorse/go.mod @@ -30,7 +30,7 @@ require ( gitlab.com/gitlab-org/golang-archive-zip v0.1.1 gitlab.com/gitlab-org/labkit v1.16.0 gocloud.dev v0.25.0 - golang.org/x/image v0.0.0-20191009234506-e7c1f5e7dbb8 + golang.org/x/image v0.0.0-20220722155232-062f8c9fd539 golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 golang.org/x/net v0.0.0-20220531201128-c960675eff93 golang.org/x/tools v0.1.11 diff --git a/workhorse/go.sum b/workhorse/go.sum index 71bf3dc2379..7ba967c4d33 100644 --- a/workhorse/go.sum +++ b/workhorse/go.sum @@ -1234,8 +1234,9 @@ golang.org/x/exp/typeparams v0.0.0-20220218215828-6cf2b201936e/go.mod h1:AbB0pIl golang.org/x/image v0.0.0-20180708004352-c73c2afc3b81/go.mod h1:ux5Hcp/YLpHSI86hEcLt0YII63i6oz57MZXIpbrjZUs= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= -golang.org/x/image v0.0.0-20191009234506-e7c1f5e7dbb8 h1:hVwzHzIUGRjiF7EcUjqNxk3NCfkPxbDKRdnNE1Rpg0U= golang.org/x/image v0.0.0-20191009234506-e7c1f5e7dbb8/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= +golang.org/x/image v0.0.0-20220722155232-062f8c9fd539 h1:/eM0PCrQI2xd471rI+snWuu251/+/jpBpZqir2mPdnU= +golang.org/x/image v0.0.0-20220722155232-062f8c9fd539/go.mod h1:doUCurBvlfPMKfmIpRIywoHmhN3VyhnoFDbvIEWF4hY= golang.org/x/lint v0.0.0-20180702182130-06c8688daad7/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU= |