diff options
50 files changed, 562 insertions, 171 deletions
diff --git a/CHANGELOG-EE.md b/CHANGELOG-EE.md index 7050d44cd88..dc4e390ebc8 100644 --- a/CHANGELOG-EE.md +++ b/CHANGELOG-EE.md @@ -1,5 +1,9 @@ Please view this file on the master branch, on stable branches it's out of date. +## 12.6.1 + +- No changes. + ## 12.6.0 ### Fixed (32 changes, 5 of them are from the community) diff --git a/CHANGELOG.md b/CHANGELOG.md index b868abf5034..2336cddc352 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,15 @@ entry. ## 12.6.1 -- No changes. +### Fixed (2 changes) + +- Handle forbidden error when checking for knative. !22170 +- Fix stack trace highlight for PHP. !22258 + +### Performance (1 change) + +- Eliminate N+1 queries in PipelinesController#index. !22189 + ## 12.6.0 diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb index ccfc38d97b2..3f6e116a62b 100644 --- a/app/controllers/projects/git_http_client_controller.rb +++ b/app/controllers/projects/git_http_client_controller.rb @@ -3,6 +3,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController include ActionController::HttpAuthentication::Basic include KerberosSpnegoHelper + include Gitlab::Utils::StrongMemoize attr_reader :authentication_result, :redirected_path @@ -47,7 +48,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController send_final_spnego_response return # Allow access end - elsif project && download_request? && http_allowed? && Guest.can?(:download_code, project) + elsif http_download_allowed? @authentication_result = Gitlab::Auth::Result.new(nil, project, :none, [:download_code]) @@ -89,11 +90,9 @@ class Projects::GitHttpClientController < Projects::ApplicationController end def repository - repo_type.repository_for(project) - end - - def wiki? - repo_type.wiki? + strong_memoize(:repository) do + repo_type.repository_for(project) + end end def repo_type @@ -113,8 +112,10 @@ class Projects::GitHttpClientController < Projects::ApplicationController authentication_result.ci?(project) end - def http_allowed? - Gitlab::ProtocolAccess.allowed?('http') + def http_download_allowed? + Gitlab::ProtocolAccess.allowed?('http') && + download_request? && + project && Guest.can?(:download_code, project) end end diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb index 93f7ce73a51..ba9a25f3733 100644 --- a/app/controllers/projects/git_http_controller.rb +++ b/app/controllers/projects/git_http_controller.rb @@ -75,17 +75,19 @@ class Projects::GitHttpController < Projects::GitHttpClientController end def enqueue_fetch_statistics_update - return if wiki? - return unless project.daily_statistics_enabled? + return if repo_type.wiki? + return unless project&.daily_statistics_enabled? ProjectDailyStatisticsWorker.perform_async(project.id) end def access - @access ||= access_klass.new(access_actor, project, - 'http', authentication_abilities: authentication_abilities, - namespace_path: params[:namespace_id], project_path: project_path, - redirected_path: redirected_path, auth_result_type: auth_result_type) + @access ||= access_klass.new(access_actor, project, 'http', + authentication_abilities: authentication_abilities, + namespace_path: params[:namespace_id], + project_path: project_path, + redirected_path: redirected_path, + auth_result_type: auth_result_type) end def access_actor diff --git a/app/models/repository.rb b/app/models/repository.rb index 2a67c26d840..3ec7d78d5f7 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -95,7 +95,7 @@ class Repository def path_to_repo @path_to_repo ||= begin - storage = Gitlab.config.repositories.storages[@project.repository_storage] + storage = Gitlab.config.repositories.storages[project.repository_storage] File.expand_path( File.join(storage.legacy_disk_path, disk_path + '.git') @@ -128,7 +128,7 @@ class Repository commits = Gitlab::Git::Commit.batch_by_oid(raw_repository, oids) if commits.present? - Commit.decorate(commits, @project) + Commit.decorate(commits, project) else [] end @@ -159,14 +159,14 @@ class Repository } commits = Gitlab::Git::Commit.where(options) - commits = Commit.decorate(commits, @project) if commits.present? + commits = Commit.decorate(commits, project) if commits.present? CommitCollection.new(project, commits, ref) end def commits_between(from, to) commits = Gitlab::Git::Commit.between(raw_repository, from, to) - commits = Commit.decorate(commits, @project) if commits.present? + commits = Commit.decorate(commits, project) if commits.present? commits end @@ -695,13 +695,13 @@ class Repository commits = raw_repository.list_last_commits_for_tree(sha, path, offset: offset, limit: limit) commits.each do |path, commit| - commits[path] = ::Commit.new(commit, @project) + commits[path] = ::Commit.new(commit, project) end end def last_commit_for_path(sha, path) commit = raw_repository.last_commit_for_path(sha, path) - ::Commit.new(commit, @project) if commit + ::Commit.new(commit, project) if commit end def last_commit_id_for_path(sha, path) @@ -1136,7 +1136,7 @@ class Repository Gitlab::Git::Commit.find(raw_repository, oid_or_ref) end - ::Commit.new(commit, @project) if commit + ::Commit.new(commit, project) if commit end def cache diff --git a/app/services/merge_requests/get_urls_service.rb b/app/services/merge_requests/get_urls_service.rb index 7c88c9abb41..de3f2acdf63 100644 --- a/app/services/merge_requests/get_urls_service.rb +++ b/app/services/merge_requests/get_urls_service.rb @@ -9,7 +9,7 @@ module MergeRequests end def execute(changes) - return [] unless project.printing_merge_request_link_enabled + return [] unless project&.printing_merge_request_link_enabled branches = get_branches(changes) merge_requests_map = opened_merge_requests_from_source_branches(branches) diff --git a/changelogs/unreleased/121714-fix-sentry-stack-trace-highlight-for-php.yml b/changelogs/unreleased/121714-fix-sentry-stack-trace-highlight-for-php.yml deleted file mode 100644 index 4f3b80dfcaa..00000000000 --- a/changelogs/unreleased/121714-fix-sentry-stack-trace-highlight-for-php.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix stack trace highlight for PHP -merge_request: 22258 -author: -type: fixed diff --git a/changelogs/unreleased/31301-expose-reference-path-in-api-for-issuables.yml b/changelogs/unreleased/31301-expose-reference-path-in-api-for-issuables.yml new file mode 100644 index 00000000000..f9f41ceb615 --- /dev/null +++ b/changelogs/unreleased/31301-expose-reference-path-in-api-for-issuables.yml @@ -0,0 +1,5 @@ +--- +title: Expose full reference path for issuables in API +merge_request: 20354 +author: +type: changed diff --git a/changelogs/unreleased/alexives-119379-handle_http_error_instead_of_not_found.yml b/changelogs/unreleased/alexives-119379-handle_http_error_instead_of_not_found.yml deleted file mode 100644 index 6f5eea45660..00000000000 --- a/changelogs/unreleased/alexives-119379-handle_http_error_instead_of_not_found.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Handle forbidden error when checking for knative -merge_request: 22170 -author: -type: fixed diff --git a/changelogs/unreleased/sh-optimize-pipeline-sql.yml b/changelogs/unreleased/sh-optimize-pipeline-sql.yml deleted file mode 100644 index f40fff99454..00000000000 --- a/changelogs/unreleased/sh-optimize-pipeline-sql.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Eliminate N+1 queries in PipelinesController#index -merge_request: 22189 -author: -type: performance diff --git a/doc/api/epics.md b/doc/api/epics.md index 531c75fd8c5..4b20534eeed 100644 --- a/doc/api/epics.md +++ b/doc/api/epics.md @@ -29,6 +29,14 @@ are paginated. Read more on [pagination](README.md#pagination). +CAUTION: **Deprecation** +> `reference` attribute in response is deprecated in favour of `references`. +> Introduced [GitLab 12.6](https://gitlab.com/gitlab-org/gitlab/merge_requests/20354) + +NOTE: **Note** +> `references.relative` is relative to the group that the epic is being requested. When epic is fetched from its origin group +> `relative` format would be the same as `short` format and when requested cross groups it is expected to be the same as `full` format. + ## List epics for a group Gets all epics of the requested group and its subgroups. @@ -73,6 +81,51 @@ Example response: "state": "opened", "web_url": "http://localhost:3001/groups/test/-/epics/4", "reference": "&4", + "references": { + "short": "&4", + "relative": "&4", + "full": "test&4" + }, + "author": { + "id": 10, + "name": "Lu Mayer", + "username": "kam", + "state": "active", + "avatar_url": "http://www.gravatar.com/avatar/018729e129a6f31c80a6327a30196823?s=80&d=identicon", + "web_url": "http://localhost:3001/kam" + }, + "start_date": null, + "start_date_is_fixed": false, + "start_date_fixed": null, + "start_date_from_milestones": null, //deprecated in favor of start_date_from_inherited_source + "start_date_from_inherited_source": null, + "end_date": "2018-07-31", //deprecated in favor of due_date + "due_date": "2018-07-31", + "due_date_is_fixed": false, + "due_date_fixed": null, + "due_date_from_milestones": "2018-07-31", //deprecated in favor of start_date_from_inherited_source + "due_date_from_inherited_source": "2018-07-31", + "created_at": "2018-07-17T13:36:22.770Z", + "updated_at": "2018-07-18T12:22:05.239Z", + "closed_at": "2018-08-18T12:22:05.239Z", + "labels": [], + "upvotes": 4, + "downvotes": 0 + }, + { + "id": 50, + "iid": 35, + "group_id": 17, + "title": "Accusamus iste et ullam ratione voluptatem omnis debitis dolor est.", + "description": "Molestias dolorem eos vitae expedita impedit necessitatibus quo voluptatum.", + "state": "opened", + "web_url": "http://localhost:3001/groups/test/sample/-/epics/4", + "reference": "&4", + "references": { + "short": "&4", + "relative": "sample&4", + "full": "test/sample&4" + }, "author": { "id": 10, "name": "Lu Mayer", @@ -131,6 +184,11 @@ Example response: "state": "opened", "web_url": "http://localhost:3001/groups/test/-/epics/5", "reference": "&5", + "references": { + "short": "&5", + "relative": "&5", + "full": "test&5" + }, "author":{ "id": 7, "name": "Pamella Huel", @@ -199,8 +257,13 @@ Example response: "title": "Epic", "description": "Epic description", "state": "opened", - "web_url": "http://localhost:3001/groups/test/-/epics/5", + "web_url": "http://localhost:3001/groups/test/-/epics/6", "reference": "&6", + "references": { + "short": "&6", + "relative": "&6", + "full": "test&6" + }, "author": { "name" : "Alexandra Bashirian", "avatar_url" : null, @@ -269,8 +332,13 @@ Example response: "title": "New Title", "description": "Epic description", "state": "opened", - "web_url": "http://localhost:3001/groups/test/-/epics/5", + "web_url": "http://localhost:3001/groups/test/-/epics/6", "reference": "&6", + "references": { + "short": "&6", + "relative": "&6", + "full": "test&6" + }, "author": { "name" : "Alexandra Bashirian", "avatar_url" : null, @@ -372,6 +440,13 @@ Example response: "avatar_url": "http://www.gravatar.com/avatar/a2f5c6fcef64c9c69cb8779cb292be1b?s=80&d=identicon", "web_url": "http://localhost:3001/arnita" }, + "web_url": "http://localhost:3001/groups/test/-/epics/5", + "reference": "&5", + "references": { + "short": "&5", + "relative": "&5", + "full": "test&5" + }, "start_date": null, "end_date": null, "created_at": "2018-01-21T06:21:13.165Z", diff --git a/doc/api/issues.md b/doc/api/issues.md index fe551cfb397..383d190a045 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -12,6 +12,14 @@ are paginated. Read more on [pagination](README.md#pagination). +CAUTION: **Deprecation** +> `reference` attribute in response is deprecated in favour of `references`. +> Introduced [GitLab 12.6](https://gitlab.com/gitlab-org/gitlab/merge_requests/20354) + +NOTE: **Note** +> `references.relative` is relative to the group / project that the issue is being requested. When issue is fetched from its project +> `relative` format would be the same as `short` format and when requested across groups / projects it is expected to be the same as `full` format. + ## List issues Get all issues the authenticated user has access to. By default it @@ -121,7 +129,12 @@ Example response: "merge_requests_count": 0, "user_notes_count": 1, "due_date": "2016-07-22", - "web_url": "http://example.com/example/example/issues/6", + "web_url": "http://example.com/my-group/my-project/issues/6", + "references": { + "short": "#6", + "relative": "my-group/my-project#6", + "full": "my-group/my-project#6" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -270,7 +283,12 @@ Example response: "closed_by" : null, "user_notes_count": 1, "due_date": null, - "web_url": "http://example.com/example/example/issues/1", + "web_url": "http://example.com/my-group/my-project/issues/1", + "references": { + "short": "#1", + "relative": "my-project#1", + "full": "my-group/my-project#1" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -426,7 +444,12 @@ Example response: }, "user_notes_count": 1, "due_date": "2016-07-22", - "web_url": "http://example.com/example/example/issues/1", + "web_url": "http://example.com/my-group/my-project/issues/1", + "references": { + "short": "#1", + "relative": "#1", + "full": "my-group/my-project#1" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -543,7 +566,12 @@ Example response: "subscribed": false, "user_notes_count": 1, "due_date": null, - "web_url": "http://example.com/example/example/issues/1", + "web_url": "http://example.com/my-group/my-project/issues/1", + "references": { + "short": "#1", + "relative": "#1", + "full": "my-group/my-project#1" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -668,7 +696,12 @@ Example response: "subscribed" : true, "user_notes_count": 0, "due_date": null, - "web_url": "http://example.com/example/example/issues/14", + "web_url": "http://example.com/my-group/my-project/issues/14", + "references": { + "short": "#14", + "relative": "#14", + "full": "my-group/my-project#14" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -778,7 +811,12 @@ Example response: "subscribed" : true, "user_notes_count": 0, "due_date": "2016-07-22", - "web_url": "http://example.com/example/example/issues/15", + "web_url": "http://example.com/my-group/my-project/issues/15", + "references": { + "short": "#15", + "relative": "#15", + "full": "my-group/my-project#15" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -900,7 +938,12 @@ Example response: "web_url": "https://gitlab.example.com/solon.cremin" }, "due_date": null, - "web_url": "http://example.com/example/example/issues/11", + "web_url": "http://example.com/my-group/my-project/issues/11", + "references": { + "short": "#11", + "relative": "#11", + "full": "my-group/my-project#11" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -1001,7 +1044,12 @@ Example response: "web_url": "https://gitlab.example.com/solon.cremin" }, "due_date": null, - "web_url": "http://example.com/example/example/issues/11", + "web_url": "http://example.com/my-group/my-project/issues/11", + "references": { + "short": "#11", + "relative": "#11", + "full": "my-group/my-project#11" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -1095,7 +1143,12 @@ Example response: }, "subscribed": false, "due_date": null, - "web_url": "http://example.com/example/example/issues/12", + "web_url": "http://example.com/my-group/my-project/issues/12", + "references": { + "short": "#12", + "relative": "#12", + "full": "my-group/my-project#12" + }, "confidential": false, "discussion_locked": false, "task_completion_status":{ @@ -1197,7 +1250,12 @@ Example response: "downvotes": 0, "merge_requests_count": 0, "due_date": null, - "web_url": "http://example.com/example/example/issues/110", + "web_url": "http://example.com/my-group/my-project/issues/10", + "references": { + "short": "#10", + "relative": "#10", + "full": "my-group/my-project#10" + }, "confidential": false, "discussion_locked": false, "task_completion_status":{ @@ -1436,6 +1494,11 @@ Example response: "force_remove_source_branch": false, "reference": "!11", "web_url": "https://gitlab.example.com/twitter/flight/merge_requests/4", + "references": { + "short": "!4", + "relative": "!4", + "full": "twitter/flight!4" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -1566,6 +1629,12 @@ Example response: "should_remove_source_branch": null, "force_remove_source_branch": false, "web_url": "https://gitlab.example.com/gitlab-org/gitlab-test/merge_requests/6432", + "reference": "!6432", + "references": { + "short": "!6432", + "relative": "!6432", + "full": "gitlab-org/gitlab-test!6432" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 541aa03450f..d17db8deeb2 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -2,6 +2,14 @@ Every API call to merge requests must be authenticated. +CAUTION: **Deprecation** +> `reference` attribute in response is deprecated in favour of `references`. +> Introduced [GitLab 12.6](https://gitlab.com/gitlab-org/gitlab/merge_requests/20354) + +NOTE: **Note** +> `references.relative` is relative to the group / project that the merge request is being requested. When merge request is fetched from its project +> `relative` format would be the same as `short` format and when requested across groups / projects it is expected to be the same as `full` format. + ## List merge requests > [Introduced][ce-13060] in GitLab 9.5. @@ -134,6 +142,11 @@ Parameters: "allow_collaboration": false, "allow_maintainer_to_push": false, "web_url": "http://gitlab.example.com/my-group/my-project/merge_requests/1", + "references": { + "short": "!1", + "relative": "my-group/my-project!1", + "full": "my-group/my-project!1" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -296,6 +309,11 @@ Parameters: "allow_collaboration": false, "allow_maintainer_to_push": false, "web_url": "http://gitlab.example.com/my-group/my-project/merge_requests/1", + "references": { + "short": "!1", + "relative": "!1", + "full": "my-group/my-project!1" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -448,6 +466,11 @@ Parameters: "should_remove_source_branch": true, "force_remove_source_branch": false, "web_url": "http://gitlab.example.com/my-group/my-project/merge_requests/1", + "references": { + "short": "!1", + "relative": "my-project!1", + "full": "my-group/my-project!1" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -574,6 +597,11 @@ Parameters: "allow_collaboration": false, "allow_maintainer_to_push": false, "web_url": "http://gitlab.example.com/my-group/my-project/merge_requests/1", + "references": { + "short": "!1", + "relative": "!1", + "full": "my-group/my-project!1" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -779,7 +807,12 @@ Parameters: "should_remove_source_branch": true, "force_remove_source_branch": false, "squash": false, - "web_url": "http://example.com/example/example/merge_requests/1", + "web_url": "http://gitlab.example.com/my-group/my-project/merge_requests/1", + "references": { + "short": "!1", + "relative": "!1", + "full": "my-group/my-project!1" + }, "discussion_locked": false, "time_stats": { "time_estimate": 0, @@ -989,6 +1022,11 @@ order for it to take effect: "allow_collaboration": false, "allow_maintainer_to_push": false, "web_url": "http://gitlab.example.com/my-group/my-project/merge_requests/1", + "references": { + "short": "!1", + "relative": "!1", + "full": "my-group/my-project!1" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -1143,6 +1181,11 @@ Must include at least one non-required attribute from above. "allow_collaboration": false, "allow_maintainer_to_push": false, "web_url": "http://gitlab.example.com/my-group/my-project/merge_requests/1", + "references": { + "short": "!1", + "relative": "!1", + "full": "my-group/my-project!1" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -1313,6 +1356,11 @@ Parameters: "allow_collaboration": false, "allow_maintainer_to_push": false, "web_url": "http://gitlab.example.com/my-group/my-project/merge_requests/1", + "references": { + "short": "!1", + "relative": "!1", + "full": "my-group/my-project!1" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -1486,6 +1534,11 @@ Parameters: "allow_collaboration": false, "allow_maintainer_to_push": false, "web_url": "http://gitlab.example.com/my-group/my-project/merge_requests/1", + "references": { + "short": "!1", + "relative": "!1", + "full": "my-group/my-project!1" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -1772,6 +1825,11 @@ Example response: "allow_collaboration": false, "allow_maintainer_to_push": false, "web_url": "http://gitlab.example.com/my-group/my-project/merge_requests/1", + "references": { + "short": "!1", + "relative": "!1", + "full": "my-group/my-project!1" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -1918,6 +1976,11 @@ Example response: "allow_collaboration": false, "allow_maintainer_to_push": false, "web_url": "http://gitlab.example.com/my-group/my-project/merge_requests/1", + "references": { + "short": "!1", + "relative": "!1", + "full": "my-group/my-project!1" + }, "time_stats": { "time_estimate": 0, "total_time_spent": 0, @@ -2078,7 +2141,12 @@ Example response: "should_remove_source_branch": true, "force_remove_source_branch": false, "squash": false, - "web_url": "http://example.com/example/example/merge_requests/1" + "web_url": "http://example.com/my-group/my-project/merge_requests/1", + "references": { + "short": "!1", + "relative": "!1", + "full": "my-group/my-project!1" + }, }, "target_url": "https://gitlab.example.com/gitlab-org/gitlab-ci/merge_requests/7", "body": "Et voluptas laudantium minus nihil recusandae ut accusamus earum aut non.", diff --git a/lib/api/entities.rb b/lib/api/entities.rb index cc95be5e3be..c82821c3ca4 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -569,6 +569,20 @@ module API end end + class IssuableReferences < Grape::Entity + expose :short do |issuable| + issuable.to_reference + end + + expose :relative do |issuable, options| + issuable.to_reference(options[:group] || options[:project]) + end + + expose :full do |issuable| + issuable.to_reference(full: true) + end + end + class Diff < Grape::Entity expose :old_path, :new_path, :a_mode, :b_mode expose :new_file?, as: :new_file @@ -676,6 +690,10 @@ module API end end + expose :references, with: IssuableReferences do |issue| + issue + end + # Calculating the value of subscribed field triggers Markdown # processing. We can't do that for multiple issues / merge # requests in a single API request. @@ -787,10 +805,16 @@ module API # Deprecated expose :allow_collaboration, as: :allow_maintainer_to_push, if: -> (merge_request, _) { merge_request.for_fork? } + # reference is deprecated in favour of references + # Introduced [Gitlab 12.6](https://gitlab.com/gitlab-org/gitlab/merge_requests/20354) expose :reference do |merge_request, options| merge_request.to_reference(options[:project]) end + expose :references, with: IssuableReferences do |merge_request| + merge_request + end + expose :web_url do |merge_request| Gitlab::UrlBuilder.build(merge_request) end diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb index b03eb5ad440..607e0784415 100644 --- a/lib/api/helpers/internal_helpers.rb +++ b/lib/api/helpers/internal_helpers.rb @@ -120,21 +120,13 @@ module API end def gl_project_path - if wiki? - project.wiki.full_path - else - project.full_path - end + repository.full_path end # Return the repository depending on whether we want the wiki or the # regular repository def repository - if repo_type.wiki? - project.wiki.repository - else - project.repository - end + @repository ||= repo_type.repository_for(project) end # Return the Gitaly Address if it is enabled diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb index 50142b8641e..11f2a2ea1c0 100644 --- a/lib/api/internal/base.rb +++ b/lib/api/internal/base.rb @@ -224,9 +224,9 @@ module API response.add_merge_request_urls(merge_request_urls) - # A user is not guaranteed to be returned; an orphaned write deploy + # Neither User nor Project are guaranteed to be returned; an orphaned write deploy # key could be used - if user + if user && project redirect_message = Gitlab::Checks::ProjectMoved.fetch_message(user.id, project.id) project_created_message = Gitlab::Checks::ProjectCreated.fetch_message(user.id, project.id) diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 4208385a48d..1ef27d9f1b1 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -122,16 +122,15 @@ module API use :issues_params end get ":id/issues" do - group = find_group!(params[:id]) - - issues = paginate(find_issues(group_id: group.id, include_subgroups: true)) + issues = paginate(find_issues(group_id: user_group.id, include_subgroups: true)) options = { with: Entities::Issue, with_labels_details: declared_params[:with_labels_details], current_user: current_user, issuable_metadata: issuable_meta_data(issues, 'Issue', current_user), - include_subscribed: false + include_subscribed: false, + group: user_group } present issues, options @@ -142,9 +141,7 @@ module API use :issues_stats_params end get ":id/issues_statistics" do - group = find_group!(params[:id]) - - present issues_statistics(group_id: group.id, include_subgroups: true), with: Grape::Presenters::Presenter + present issues_statistics(group_id: user_group.id, include_subgroups: true), with: Grape::Presenters::Presenter end end @@ -161,9 +158,7 @@ module API use :issues_params end get ":id/issues" do - project = find_project!(params[:id]) - - issues = paginate(find_issues(project_id: project.id)) + issues = paginate(find_issues(project_id: user_project.id)) options = { with: Entities::Issue, @@ -182,9 +177,7 @@ module API use :issues_stats_params end get ":id/issues_statistics" do - project = find_project!(params[:id]) - - present issues_statistics(project_id: project.id), with: Grape::Presenters::Presenter + present issues_statistics(project_id: user_project.id), with: Grape::Presenters::Presenter end desc 'Get a single project issue' do diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 794237f8032..0e161d5589a 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -157,11 +157,9 @@ module API use :merge_requests_params end get ":id/merge_requests" do - group = find_group!(params[:id]) + merge_requests = find_merge_requests(group_id: user_group.id, include_subgroups: true) - merge_requests = find_merge_requests(group_id: group.id, include_subgroups: true) - - present merge_requests, serializer_options_for(merge_requests) + present merge_requests, serializer_options_for(merge_requests).merge(group: user_group) end end @@ -215,7 +213,7 @@ module API merge_requests = find_merge_requests(project_id: user_project.id) - options = serializer_options_for(merge_requests) + options = serializer_options_for(merge_requests).merge(project: user_project) options[:project] = user_project present merge_requests, options diff --git a/lib/gitlab/backtrace_cleaner.rb b/lib/gitlab/backtrace_cleaner.rb new file mode 100644 index 00000000000..30ec99808f7 --- /dev/null +++ b/lib/gitlab/backtrace_cleaner.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module Gitlab + module BacktraceCleaner + IGNORE_BACKTRACES = %w[ + config/initializers + ee/lib/gitlab/middleware/ + lib/gitlab/correlation_id.rb + lib/gitlab/database/load_balancing/ + lib/gitlab/etag_caching/ + lib/gitlab/i18n.rb + lib/gitlab/metrics/ + lib/gitlab/middleware/ + lib/gitlab/performance_bar/ + lib/gitlab/profiler.rb + lib/gitlab/query_limiting/ + lib/gitlab/request_context.rb + lib/gitlab/request_profiler/ + lib/gitlab/sidekiq_logging/ + lib/gitlab/sidekiq_middleware/ + lib/gitlab/sidekiq_status/ + lib/gitlab/tracing/ + lib/gitlab/webpack/dev_server_middleware.rb + ].freeze + + IGNORED_BACKTRACES_REGEXP = Regexp.union(IGNORE_BACKTRACES).freeze + + def self.clean_backtrace(backtrace) + return unless backtrace + + Array(Rails.backtrace_cleaner.clean(backtrace)).reject do |line| + line.match(IGNORED_BACKTRACES_REGEXP) + end + end + end +end diff --git a/lib/gitlab/bitbucket_import/importer.rb b/lib/gitlab/bitbucket_import/importer.rb index 67118aed549..349ca26ec03 100644 --- a/lib/gitlab/bitbucket_import/importer.rb +++ b/lib/gitlab/bitbucket_import/importer.rb @@ -42,7 +42,7 @@ module Gitlab end def store_pull_request_error(pull_request, ex) - backtrace = Gitlab::Profiler.clean_backtrace(ex.backtrace) + backtrace = Gitlab::BacktraceCleaner.clean_backtrace(ex.backtrace) error = { type: :pull_request, iid: pull_request.iid, errors: ex.message, trace: backtrace, raw_response: pull_request.raw } Gitlab::ErrorTracking.log_exception(ex, error) diff --git a/lib/gitlab/exception_log_formatter.rb b/lib/gitlab/exception_log_formatter.rb index e0de0219294..92d55213cc2 100644 --- a/lib/gitlab/exception_log_formatter.rb +++ b/lib/gitlab/exception_log_formatter.rb @@ -13,7 +13,7 @@ module Gitlab ) if exception.backtrace - payload['exception.backtrace'] = Gitlab::Profiler.clean_backtrace(exception.backtrace) + payload['exception.backtrace'] = Gitlab::BacktraceCleaner.clean_backtrace(exception.backtrace) end end end diff --git a/lib/gitlab/git/rugged_impl/use_rugged.rb b/lib/gitlab/git/rugged_impl/use_rugged.rb index 80b75689334..ca5d533bd75 100644 --- a/lib/gitlab/git/rugged_impl/use_rugged.rb +++ b/lib/gitlab/git/rugged_impl/use_rugged.rb @@ -27,7 +27,7 @@ module Gitlab feature: method_name, args: args, duration: duration, - backtrace: Gitlab::Profiler.clean_backtrace(caller)) + backtrace: Gitlab::BacktraceCleaner.clean_backtrace(caller)) end result diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 25785089a34..9636e75aba1 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -179,7 +179,7 @@ module Gitlab self.query_time += duration if Gitlab::PerformanceBar.enabled_for_request? add_call_details(feature: "#{service}##{rpc}", duration: duration, request: request_hash, rpc: rpc, - backtrace: Gitlab::Profiler.clean_backtrace(caller)) + backtrace: Gitlab::BacktraceCleaner.clean_backtrace(caller)) end end @@ -438,7 +438,7 @@ module Gitlab def self.count_stack return unless Gitlab::SafeRequestStore.active? - stack_string = Gitlab::Profiler.clean_backtrace(caller).drop(1).join("\n") + stack_string = Gitlab::BacktraceCleaner.clean_backtrace(caller).drop(1).join("\n") Gitlab::SafeRequestStore[:stack_counter] ||= Hash.new diff --git a/lib/gitlab/profiler.rb b/lib/gitlab/profiler.rb index f2f6180c464..f47ccb8fed9 100644 --- a/lib/gitlab/profiler.rb +++ b/lib/gitlab/profiler.rb @@ -107,7 +107,7 @@ module Gitlab super - Gitlab::Profiler.clean_backtrace(caller).each do |caller_line| + Gitlab::BacktraceCleaner.clean_backtrace(caller).each do |caller_line| stripped_caller_line = caller_line.sub("#{Rails.root}/", '') super(" ↳ #{stripped_caller_line}") @@ -117,14 +117,6 @@ module Gitlab end end - def self.clean_backtrace(backtrace) - return unless backtrace - - Array(Rails.backtrace_cleaner.clean(backtrace)).reject do |line| - line.match(Regexp.union(IGNORE_BACKTRACES)) - end - end - def self.with_custom_logger(logger) original_colorize_logging = ActiveSupport::LogSubscriber.colorize_logging original_activerecord_logger = ActiveRecord::Base.logger diff --git a/lib/gitlab/repo_path.rb b/lib/gitlab/repo_path.rb index 030e50dfbf6..1baa2a9e461 100644 --- a/lib/gitlab/repo_path.rb +++ b/lib/gitlab/repo_path.rb @@ -32,9 +32,12 @@ module Gitlab def self.find_project(project_path) project = Project.find_by_full_path(project_path, follow_redirects: true) - was_redirected = project && project.full_path.casecmp(project_path) != 0 - [project, was_redirected] + [project, redirected?(project, project_path)] + end + + def self.redirected?(project, project_path) + project && project.full_path.casecmp(project_path) != 0 end end end diff --git a/lib/gitlab/repository_cache.rb b/lib/gitlab/repository_cache.rb index 56007574b1b..fca8c43da2e 100644 --- a/lib/gitlab/repository_cache.rb +++ b/lib/gitlab/repository_cache.rb @@ -7,7 +7,8 @@ module Gitlab def initialize(repository, extra_namespace: nil, backend: Rails.cache) @repository = repository - @namespace = "#{repository.full_path}:#{repository.project.id}" + @namespace = "#{repository.full_path}" + @namespace += ":#{repository.project.id}" if repository.project @namespace = "#{@namespace}:#{extra_namespace}" if extra_namespace @backend = backend end diff --git a/lib/gitlab/repository_set_cache.rb b/lib/gitlab/repository_set_cache.rb index 6d3ac53a787..4797ec0b116 100644 --- a/lib/gitlab/repository_set_cache.rb +++ b/lib/gitlab/repository_set_cache.rb @@ -7,7 +7,8 @@ module Gitlab def initialize(repository, extra_namespace: nil, expires_in: 2.weeks) @repository = repository - @namespace = "#{repository.full_path}:#{repository.project.id}" + @namespace = "#{repository.full_path}" + @namespace += ":#{repository.project.id}" if repository.project @namespace = "#{@namespace}:#{extra_namespace}" if extra_namespace @expires_in = expires_in end diff --git a/lib/gitlab/sidekiq_logging/exception_handler.rb b/lib/gitlab/sidekiq_logging/exception_handler.rb index fba74b6c9ed..a6d6819bf8e 100644 --- a/lib/gitlab/sidekiq_logging/exception_handler.rb +++ b/lib/gitlab/sidekiq_logging/exception_handler.rb @@ -18,7 +18,7 @@ module Gitlab data.merge!(job_data) if job_data.present? end - data[:error_backtrace] = Gitlab::Profiler.clean_backtrace(job_exception.backtrace) if job_exception.backtrace.present? + data[:error_backtrace] = Gitlab::BacktraceCleaner.clean_backtrace(job_exception.backtrace) if job_exception.backtrace.present? Sidekiq.logger.warn(data) end diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb index 713ca31bbc5..29450a33289 100644 --- a/lib/gitlab/workhorse.rb +++ b/lib/gitlab/workhorse.rb @@ -22,18 +22,16 @@ module Gitlab def git_http_ok(repository, repo_type, user, action, show_all_refs: false) raise "Unsupported action: #{action}" unless ALLOWED_GIT_HTTP_ACTIONS.include?(action.to_s) - project = repository.project - attrs = { GL_ID: Gitlab::GlId.gl_id(user), - GL_REPOSITORY: repo_type.identifier_for_subject(project), + GL_REPOSITORY: repo_type.identifier_for_subject(repository.project), GL_USERNAME: user&.username, ShowAllRefs: show_all_refs, Repository: repository.gitaly_repository.to_h, GitConfigOptions: [], GitalyServer: { - address: Gitlab::GitalyClient.address(project.repository_storage), - token: Gitlab::GitalyClient.token(project.repository_storage), + address: Gitlab::GitalyClient.address(repository.storage), + token: Gitlab::GitalyClient.token(repository.storage), features: Feature::Gitaly.server_feature_flags } } diff --git a/lib/peek/views/active_record.rb b/lib/peek/views/active_record.rb index 1bb3ddb964a..ed3470f81f4 100644 --- a/lib/peek/views/active_record.rb +++ b/lib/peek/views/active_record.rb @@ -32,7 +32,7 @@ module Peek detail_store << { duration: finish - start, sql: data[:sql].strip, - backtrace: Gitlab::Profiler.clean_backtrace(caller) + backtrace: Gitlab::BacktraceCleaner.clean_backtrace(caller) } end end diff --git a/lib/peek/views/redis_detailed.rb b/lib/peek/views/redis_detailed.rb index 84041b6be73..14cabd62025 100644 --- a/lib/peek/views/redis_detailed.rb +++ b/lib/peek/views/redis_detailed.rb @@ -23,7 +23,7 @@ module Gitlab detail_store << { cmd: args.first, duration: duration, - backtrace: ::Gitlab::Profiler.clean_backtrace(caller) + backtrace: ::Gitlab::BacktraceCleaner.clean_backtrace(caller) } end diff --git a/qa/qa/resource/runner.rb b/qa/qa/resource/runner.rb index d1b4e8f7d54..f1f72c9cacd 100644 --- a/qa/qa/resource/runner.rb +++ b/qa/qa/resource/runner.rb @@ -54,7 +54,7 @@ module QA @id = this_runner[:id] super - + ensure Service::DockerRun::GitlabRunner.new(name).remove! end diff --git a/qa/qa/resource/sandbox.rb b/qa/qa/resource/sandbox.rb index 6c87fcb377a..54c13071cef 100644 --- a/qa/qa/resource/sandbox.rb +++ b/qa/qa/resource/sandbox.rb @@ -41,6 +41,14 @@ module QA resource_web_url(api_get) rescue ResourceNotFoundError super + + # If the group was just created the runners token might not be + # available via the API immediately. + Support::Retrier.retry_on_exception(sleep_interval: 5) do + resource = resource_web_url(api_get) + populate(:runners_token) + resource + end end def api_get_path diff --git a/spec/controllers/projects/git_http_controller_spec.rb b/spec/controllers/projects/git_http_controller_spec.rb index b756dd5662d..b7568346cdc 100644 --- a/spec/controllers/projects/git_http_controller_spec.rb +++ b/spec/controllers/projects/git_http_controller_spec.rb @@ -3,10 +3,17 @@ require 'spec_helper' describe Projects::GitHttpController do + let_it_be(:project) { create(:project, :public, :repository) } + let(:project_params) do + { + namespace_id: project.namespace.to_param, + project_id: project.path + '.git' + } + end + let(:params) { project_params } + describe 'HEAD #info_refs' do it 'returns 403' do - project = create(:project, :public, :repository) - head :info_refs, params: { namespace_id: project.namespace.to_param, project_id: project.path + '.git' } expect(response.status).to eq(403) @@ -14,18 +21,17 @@ describe Projects::GitHttpController do end describe 'GET #info_refs' do + let(:params) { project_params.merge(service: 'git-upload-pack') } + it 'returns 401 for unauthenticated requests to public repositories when http protocol is disabled' do stub_application_setting(enabled_git_access_protocol: 'ssh') - project = create(:project, :public, :repository) - get :info_refs, params: { service: 'git-upload-pack', namespace_id: project.namespace.to_param, project_id: project.path + '.git' } + get :info_refs, params: params expect(response.status).to eq(401) end context 'with exceptions' do - let(:project) { create(:project, :public, :repository) } - before do allow(controller).to receive(:verify_workhorse_api!).and_return(true) end @@ -33,7 +39,7 @@ describe Projects::GitHttpController do it 'returns 503 with GRPC Unavailable' do allow(controller).to receive(:access_check).and_raise(GRPC::Unavailable) - get :info_refs, params: { service: 'git-upload-pack', namespace_id: project.namespace.to_param, project_id: project.path + '.git' } + get :info_refs, params: params expect(response.status).to eq(503) end @@ -41,11 +47,27 @@ describe Projects::GitHttpController do it 'returns 503 with timeout error' do allow(controller).to receive(:access_check).and_raise(Gitlab::GitAccess::TimeoutError) - get :info_refs, params: { service: 'git-upload-pack', namespace_id: project.namespace.to_param, project_id: project.path + '.git' } + get :info_refs, params: params expect(response.status).to eq(503) expect(response.body).to eq 'Gitlab::GitAccess::TimeoutError' end end end + + describe 'POST #git_upload_pack' do + before do + allow(controller).to receive(:authenticate_user).and_return(true) + allow(controller).to receive(:verify_workhorse_api!).and_return(true) + allow(controller).to receive(:access_check).and_return(nil) + end + + after do + post :git_upload_pack, params: params + end + + it 'updates project statistics' do + expect(ProjectDailyStatisticsWorker).to receive(:perform_async) + end + end end diff --git a/spec/fixtures/api/schemas/public_api/v4/issue.json b/spec/fixtures/api/schemas/public_api/v4/issue.json index 147f53239e0..bf1b4a06f0b 100644 --- a/spec/fixtures/api/schemas/public_api/v4/issue.json +++ b/spec/fixtures/api/schemas/public_api/v4/issue.json @@ -84,6 +84,11 @@ "total_time_spent": { "type": "integer" }, "human_time_estimate": { "type": ["string", "null"] }, "human_total_time_spent": { "type": ["string", "null"] } + }, + "references": { + "short": {"type": "string"}, + "relative": {"type": "string"}, + "full": {"type": "string"} } }, "required": [ diff --git a/spec/fixtures/api/schemas/public_api/v4/merge_request.json b/spec/fixtures/api/schemas/public_api/v4/merge_request.json index a423bf70b69..3bf1299a1d8 100644 --- a/spec/fixtures/api/schemas/public_api/v4/merge_request.json +++ b/spec/fixtures/api/schemas/public_api/v4/merge_request.json @@ -113,7 +113,12 @@ "human_total_time_spent": { "type": ["string", "null"] } }, "allow_collaboration": { "type": ["boolean", "null"] }, - "allow_maintainer_to_push": { "type": ["boolean", "null"] } + "allow_maintainer_to_push": { "type": ["boolean", "null"] }, + "references": { + "short": {"type": "string"}, + "relative": {"type": "string"}, + "full": {"type": "string"} + } }, "required": [ "id", "iid", "project_id", "title", "description", diff --git a/spec/initializers/lograge_spec.rb b/spec/initializers/lograge_spec.rb index 5dd296b6040..65652468d93 100644 --- a/spec/initializers/lograge_spec.rb +++ b/spec/initializers/lograge_spec.rb @@ -112,7 +112,7 @@ describe 'lograge', type: :request do expect(log_data['exception.class']).to eq('RuntimeError') expect(log_data['exception.message']).to eq('bad request') - expect(log_data['exception.backtrace']).to eq(Gitlab::Profiler.clean_backtrace(backtrace)) + expect(log_data['exception.backtrace']).to eq(Gitlab::BacktraceCleaner.clean_backtrace(backtrace)) end end end diff --git a/spec/lib/gitlab/backtrace_cleaner_spec.rb b/spec/lib/gitlab/backtrace_cleaner_spec.rb new file mode 100644 index 00000000000..f3aded9faad --- /dev/null +++ b/spec/lib/gitlab/backtrace_cleaner_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::BacktraceCleaner do + describe '.clean_backtrace' do + it 'uses the Rails backtrace cleaner' do + backtrace = [] + + expect(Rails.backtrace_cleaner).to receive(:clean).with(backtrace) + + described_class.clean_backtrace(backtrace) + end + + it 'removes lines from IGNORE_BACKTRACES' do + backtrace = [ + "lib/gitlab/gitaly_client.rb:294:in `block (2 levels) in migrate'", + "lib/gitlab/gitaly_client.rb:331:in `allow_n_plus_1_calls'", + "lib/gitlab/gitaly_client.rb:280:in `block in migrate'", + "lib/gitlab/metrics/influx_db.rb:103:in `measure'", + "lib/gitlab/gitaly_client.rb:278:in `migrate'", + "lib/gitlab/git/repository.rb:1451:in `gitaly_migrate'", + "lib/gitlab/git/commit.rb:66:in `find'", + "app/models/repository.rb:1047:in `find_commit'", + "lib/gitlab/metrics/instrumentation.rb:159:in `block in find_commit'", + "lib/gitlab/metrics/method_call.rb:36:in `measure'", + "lib/gitlab/metrics/instrumentation.rb:159:in `find_commit'", + "app/models/repository.rb:113:in `commit'", + "lib/gitlab/i18n.rb:50:in `with_locale'", + "lib/gitlab/middleware/multipart.rb:95:in `call'", + "lib/gitlab/request_profiler/middleware.rb:14:in `call'", + "ee/lib/gitlab/database/load_balancing/rack_middleware.rb:37:in `call'", + "ee/lib/gitlab/jira/middleware.rb:15:in `call'" + ] + + expect(described_class.clean_backtrace(backtrace)) + .to eq([ + "lib/gitlab/gitaly_client.rb:294:in `block (2 levels) in migrate'", + "lib/gitlab/gitaly_client.rb:331:in `allow_n_plus_1_calls'", + "lib/gitlab/gitaly_client.rb:280:in `block in migrate'", + "lib/gitlab/gitaly_client.rb:278:in `migrate'", + "lib/gitlab/git/repository.rb:1451:in `gitaly_migrate'", + "lib/gitlab/git/commit.rb:66:in `find'", + "app/models/repository.rb:1047:in `find_commit'", + "app/models/repository.rb:113:in `commit'", + "ee/lib/gitlab/jira/middleware.rb:15:in `call'" + ]) + end + end +end diff --git a/spec/lib/gitlab/grape_logging/loggers/exception_logger_spec.rb b/spec/lib/gitlab/grape_logging/loggers/exception_logger_spec.rb index 0cfda80b854..c9021e2f436 100644 --- a/spec/lib/gitlab/grape_logging/loggers/exception_logger_spec.rb +++ b/spec/lib/gitlab/grape_logging/loggers/exception_logger_spec.rb @@ -39,7 +39,7 @@ describe Gitlab::GrapeLogging::Loggers::ExceptionLogger do before do current_backtrace = caller allow(exception).to receive(:backtrace).and_return(current_backtrace) - expected['exception.backtrace'] = Gitlab::Profiler.clean_backtrace(current_backtrace) + expected['exception.backtrace'] = Gitlab::BacktraceCleaner.clean_backtrace(current_backtrace) end it 'includes the backtrace' do diff --git a/spec/lib/gitlab/profiler_spec.rb b/spec/lib/gitlab/profiler_spec.rb index a19392f4bcb..c27e3ca7ace 100644 --- a/spec/lib/gitlab/profiler_spec.rb +++ b/spec/lib/gitlab/profiler_spec.rb @@ -120,51 +120,6 @@ describe Gitlab::Profiler do end end - describe '.clean_backtrace' do - it 'uses the Rails backtrace cleaner' do - backtrace = [] - - expect(Rails.backtrace_cleaner).to receive(:clean).with(backtrace) - - described_class.clean_backtrace(backtrace) - end - - it 'removes lines from IGNORE_BACKTRACES' do - backtrace = [ - "lib/gitlab/gitaly_client.rb:294:in `block (2 levels) in migrate'", - "lib/gitlab/gitaly_client.rb:331:in `allow_n_plus_1_calls'", - "lib/gitlab/gitaly_client.rb:280:in `block in migrate'", - "lib/gitlab/metrics/influx_db.rb:103:in `measure'", - "lib/gitlab/gitaly_client.rb:278:in `migrate'", - "lib/gitlab/git/repository.rb:1451:in `gitaly_migrate'", - "lib/gitlab/git/commit.rb:66:in `find'", - "app/models/repository.rb:1047:in `find_commit'", - "lib/gitlab/metrics/instrumentation.rb:159:in `block in find_commit'", - "lib/gitlab/metrics/method_call.rb:36:in `measure'", - "lib/gitlab/metrics/instrumentation.rb:159:in `find_commit'", - "app/models/repository.rb:113:in `commit'", - "lib/gitlab/i18n.rb:50:in `with_locale'", - "lib/gitlab/middleware/multipart.rb:95:in `call'", - "lib/gitlab/request_profiler/middleware.rb:14:in `call'", - "ee/lib/gitlab/database/load_balancing/rack_middleware.rb:37:in `call'", - "ee/lib/gitlab/jira/middleware.rb:15:in `call'" - ] - - expect(described_class.clean_backtrace(backtrace)) - .to eq([ - "lib/gitlab/gitaly_client.rb:294:in `block (2 levels) in migrate'", - "lib/gitlab/gitaly_client.rb:331:in `allow_n_plus_1_calls'", - "lib/gitlab/gitaly_client.rb:280:in `block in migrate'", - "lib/gitlab/gitaly_client.rb:278:in `migrate'", - "lib/gitlab/git/repository.rb:1451:in `gitaly_migrate'", - "lib/gitlab/git/commit.rb:66:in `find'", - "app/models/repository.rb:1047:in `find_commit'", - "app/models/repository.rb:113:in `commit'", - "ee/lib/gitlab/jira/middleware.rb:15:in `call'" - ]) - end - end - describe '.with_custom_logger' do context 'when the logger is set' do it 'uses the replacement logger for the duration of the block' do diff --git a/spec/lib/gitlab/repository_cache_spec.rb b/spec/lib/gitlab/repository_cache_spec.rb index 6a684595eb8..1b7dd1766da 100644 --- a/spec/lib/gitlab/repository_cache_spec.rb +++ b/spec/lib/gitlab/repository_cache_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe Gitlab::RepositoryCache do + let_it_be(:project) { create(:project) } let(:backend) { double('backend').as_null_object } - let(:project) { create(:project) } let(:repository) { project.repository } let(:namespace) { "#{repository.full_path}:#{project.id}" } let(:cache) { described_class.new(repository, backend: backend) } diff --git a/spec/lib/gitlab/repository_set_cache_spec.rb b/spec/lib/gitlab/repository_set_cache_spec.rb index 87e51f801e5..de0f3602346 100644 --- a/spec/lib/gitlab/repository_set_cache_spec.rb +++ b/spec/lib/gitlab/repository_set_cache_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::RepositorySetCache, :clean_gitlab_redis_cache do - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } let(:repository) { project.repository } let(:namespace) { "#{repository.full_path}:#{project.id}" } let(:cache) { described_class.new(repository) } diff --git a/spec/lib/gitlab/sidekiq_logging/exception_handler_spec.rb b/spec/lib/gitlab/sidekiq_logging/exception_handler_spec.rb index 24b6090cb19..a79a0678e2b 100644 --- a/spec/lib/gitlab/sidekiq_logging/exception_handler_spec.rb +++ b/spec/lib/gitlab/sidekiq_logging/exception_handler_spec.rb @@ -33,7 +33,7 @@ describe Gitlab::SidekiqLogging::ExceptionHandler do error_class: 'RuntimeError', error_message: exception_message, context: 'Test', - error_backtrace: Gitlab::Profiler.clean_backtrace(backtrace) + error_backtrace: Gitlab::BacktraceCleaner.clean_backtrace(backtrace) ) expect(logger).to receive(:warn).with(expected_data) diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index df55e9ad849..4fe0d4a5983 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -1000,6 +1000,22 @@ describe API::Internal::Base do it 'does not try to notify that project moved' do allow_any_instance_of(Gitlab::Identifier).to receive(:identify).and_return(nil) + expect(Gitlab::Checks::ProjectMoved).not_to receive(:fetch_message) + + post api('/internal/post_receive'), params: valid_params + + expect(response).to have_gitlab_http_status(200) + end + end + + context 'when project is nil' do + let(:gl_repository) { 'project-foo' } + + it 'does not try to notify that project moved' do + allow(Gitlab::GlRepository).to receive(:parse).and_return([nil, Gitlab::GlRepository::PROJECT]) + + expect(Gitlab::Checks::ProjectMoved).not_to receive(:fetch_message) + post api('/internal/post_receive'), params: valid_params expect(response).to have_gitlab_http_status(200) diff --git a/spec/requests/api/issues/get_group_issues_spec.rb b/spec/requests/api/issues/get_group_issues_spec.rb index 3ee08758f99..ef63902ffd7 100644 --- a/spec/requests/api/issues/get_group_issues_spec.rb +++ b/spec/requests/api/issues/get_group_issues_spec.rb @@ -688,5 +688,32 @@ describe API::Issues do end end end + + context "#to_reference" do + it 'exposes reference path in context of group' do + get api(base_url, user) + + expect(json_response.first['references']['short']).to eq("##{group_closed_issue.iid}") + expect(json_response.first['references']['relative']).to eq("#{group_closed_issue.project.path}##{group_closed_issue.iid}") + expect(json_response.first['references']['full']).to eq("#{group_closed_issue.project.full_path}##{group_closed_issue.iid}") + end + + context 'referencing from parent group' do + let(:parent_group) { create(:group) } + + before do + group.update(parent_id: parent_group.id) + group_closed_issue.reload + end + + it 'exposes reference path in context of parent group' do + get api("/groups/#{parent_group.id}/issues") + + expect(json_response.first['references']['short']).to eq("##{group_closed_issue.iid}") + expect(json_response.first['references']['relative']).to eq("#{group_closed_issue.project.full_path}##{group_closed_issue.iid}") + expect(json_response.first['references']['full']).to eq("#{group_closed_issue.project.full_path}##{group_closed_issue.iid}") + end + end + end end end diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index 50a0a80b542..a3538aa98b1 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -805,6 +805,17 @@ describe API::Issues do end end + describe 'GET /projects/:id/issues/:issue_iid' do + it 'exposes full reference path' do + get api("/projects/#{project.id}/issues/#{issue.iid}", user) + + expect(response).to have_gitlab_http_status(200) + expect(json_response['references']['short']).to eq("##{issue.iid}") + expect(json_response['references']['relative']).to eq("##{issue.iid}") + expect(json_response['references']['full']).to eq("#{project.parent.path}/#{project.path}##{issue.iid}") + end + end + describe 'DELETE /projects/:id/issues/:issue_iid' do it 'rejects a non member from deleting an issue' do delete api("/projects/#{project.id}/issues/#{issue.iid}", non_member) diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index e5ad1a6378e..582bf19820d 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -736,6 +736,33 @@ describe API::MergeRequests do it_behaves_like 'merge requests list' end + + context "#to_reference" do + it 'exposes reference path in context of group' do + get api("/groups/#{group.id}/merge_requests", user) + + expect(json_response.first['references']['short']).to eq("!#{merge_request_merged.iid}") + expect(json_response.first['references']['relative']).to eq("#{merge_request_merged.target_project.path}!#{merge_request_merged.iid}") + expect(json_response.first['references']['full']).to eq("#{merge_request_merged.target_project.full_path}!#{merge_request_merged.iid}") + end + + context 'referencing from parent group' do + let(:parent_group) { create(:group) } + + before do + group.update(parent_id: parent_group.id) + merge_request_merged.reload + end + + it 'exposes reference path in context of parent group' do + get api("/groups/#{parent_group.id}/merge_requests") + + expect(json_response.first['references']['short']).to eq("!#{merge_request_merged.iid}") + expect(json_response.first['references']['relative']).to eq("#{merge_request_merged.target_project.full_path}!#{merge_request_merged.iid}") + expect(json_response.first['references']['full']).to eq("#{merge_request_merged.target_project.full_path}!#{merge_request_merged.iid}") + end + end + end end describe "GET /projects/:id/merge_requests/:merge_request_iid" do @@ -783,6 +810,9 @@ describe API::MergeRequests do expect(json_response).not_to include('rebase_in_progress') expect(json_response['has_conflicts']).to be_falsy expect(json_response['blocking_discussions_resolved']).to be_truthy + expect(json_response['references']['short']).to eq("!#{merge_request.iid}") + expect(json_response['references']['relative']).to eq("!#{merge_request.iid}") + expect(json_response['references']['full']).to eq("#{merge_request.target_project.full_path}!#{merge_request.iid}") end it 'exposes description and title html when render_html is true' do diff --git a/spec/services/merge_requests/get_urls_service_spec.rb b/spec/services/merge_requests/get_urls_service_spec.rb index dcb8c8080a1..bb8a1873dac 100644 --- a/spec/services/merge_requests/get_urls_service_spec.rb +++ b/spec/services/merge_requests/get_urls_service_spec.rb @@ -45,6 +45,13 @@ describe MergeRequests::GetUrlsService do end end + context 'when project is nil' do + let(:project) { nil } + let(:changes) { default_branch_changes } + + it_behaves_like 'no_merge_request_url' + end + context 'pushing to default branch' do let(:changes) { default_branch_changes } diff --git a/spec/support/helpers/query_recorder.rb b/spec/support/helpers/query_recorder.rb index 9d47a0c23df..1d04014c9a6 100644 --- a/spec/support/helpers/query_recorder.rb +++ b/spec/support/helpers/query_recorder.rb @@ -16,7 +16,7 @@ module ActiveRecord def show_backtrace(values) Rails.logger.debug("QueryRecorder SQL: #{values[:sql]}") - Gitlab::Profiler.clean_backtrace(caller).each { |line| Rails.logger.debug(" --> #{line}") } + Gitlab::BacktraceCleaner.clean_backtrace(caller).each { |line| Rails.logger.debug(" --> #{line}") } end def callback(name, start, finish, message_id, values) diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index f2fa6af6402..bd945fe6409 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -245,8 +245,8 @@ module TestEnv end end - def copy_repo(project, bare_repo:, refs:) - target_repo_path = File.expand_path(repos_path + "/#{project.disk_path}.git") + def copy_repo(subject, bare_repo:, refs:) + target_repo_path = File.expand_path(repos_path + "/#{subject.disk_path}.git") FileUtils.mkdir_p(target_repo_path) FileUtils.cp_r("#{File.expand_path(bare_repo)}/.", target_repo_path) |