diff options
45 files changed, 364 insertions, 178 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ba36bbad7b..b4b672b55c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,24 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 11.1.3 (2018-07-27) + +### Fixed (8 changes, 1 of them is from the community) + +- Rework some projects table indexes around repository_storage field. !20377 +- Fix navigation to First and Next discussion on MR Changes tab. !20434 +- Fix showing outdated discussions on Changes tab. !20445 +- Fix autosave and ESC confirmation issues for MR discussions. !20569 +- Fix rendering of the context lines in MR diffs page. !20642 +- Don't overflow project/group dropdown results. !20704 (gfyoung) +- Fixed IDE not opening JSON files. !20798 +- Disable Gitaly timeouts when creating or restoring backups. !20810 + +### Performance (1 change) + +- Reduces the client side memory footprint on merge requests. !20744 + + ## 11.1.2 (2018-07-26) ### Security (4 changes) diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index 69adf3456f8..0ee843cc604 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -7.1.5 +7.2.0 diff --git a/app/assets/javascripts/pipelines/components/graph/dropdown_job_component.vue b/app/assets/javascripts/pipelines/components/graph/dropdown_job_component.vue index 14518f86dc7..8487c8036ee 100644 --- a/app/assets/javascripts/pipelines/components/graph/dropdown_job_component.vue +++ b/app/assets/javascripts/pipelines/components/graph/dropdown_job_component.vue @@ -14,7 +14,7 @@ import tooltip from '../../../vue_shared/directives/tooltip'; * "id": 4256, * "name": "test", * "status": { - * "icon": "icon_status_success", + * "icon": "status_success", * "text": "passed", * "label": "passed", * "group": "success", diff --git a/app/assets/javascripts/pipelines/components/graph/job_component.vue b/app/assets/javascripts/pipelines/components/graph/job_component.vue index 84a3d58b770..66f95147193 100644 --- a/app/assets/javascripts/pipelines/components/graph/job_component.vue +++ b/app/assets/javascripts/pipelines/components/graph/job_component.vue @@ -13,7 +13,7 @@ import tooltip from '../../../vue_shared/directives/tooltip'; * "id": 4256, * "name": "test", * "status": { - * "icon": "icon_status_success", + * "icon": "status_success", * "text": "passed", * "label": "passed", * "group": "success", diff --git a/app/finders/admin/projects_finder.rb b/app/finders/admin/projects_finder.rb index 53b77f5fed9..543bf1a1415 100644 --- a/app/finders/admin/projects_finder.rb +++ b/app/finders/admin/projects_finder.rb @@ -7,7 +7,7 @@ class Admin::ProjectsFinder end def execute - items = Project.without_deleted.with_statistics + items = Project.without_deleted.with_statistics.with_route items = by_namespace_id(items) items = by_visibilty_level(items) items = by_with_push(items) @@ -16,7 +16,7 @@ class Admin::ProjectsFinder items = by_archived(items) items = by_personal(items) items = by_name(items) - items = items.includes(namespace: [:owner]) + items = items.includes(namespace: [:owner, :route]) sort(items).page(params[:page]) end diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 1db1482d6b7..0e1e39501f5 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -124,7 +124,7 @@ class Notify < BaseMailer fallback_reply_message_id = "<reply-#{reply_key}@#{Gitlab.config.gitlab.host}>".freeze headers['References'] ||= [] - headers['References'] << fallback_reply_message_id + headers['References'].unshift(fallback_reply_message_id) @reply_by_email = true end @@ -158,7 +158,7 @@ class Notify < BaseMailer def mail_answer_thread(model, headers = {}) headers['Message-ID'] = "<#{SecureRandom.hex}@#{Gitlab.config.gitlab.host}>" headers['In-Reply-To'] = message_id(model) - headers['References'] = message_id(model) + headers['References'] = [message_id(model)] headers[:subject]&.prepend('Re: ') diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb index 0176a12a131..cb91f8fbac8 100644 --- a/app/models/concerns/routable.rb +++ b/app/models/concerns/routable.rb @@ -90,34 +90,17 @@ module Routable end def full_name - if route && route.name.present? - @full_name ||= route.name # rubocop:disable Gitlab/ModuleWithInstanceVariables - else - update_route if persisted? - - build_full_name - end + route&.name || build_full_name end - # Every time `project.namespace.becomes(Namespace)` is called for polymorphic_path, - # a new instance is instantiated, and we end up duplicating the same query to retrieve - # the route. Caching this per request ensures that even if we have multiple instances, - # we will not have to duplicate work, avoiding N+1 queries in some cases. def full_path - return uncached_full_path unless RequestStore.active? && persisted? - - RequestStore[full_path_key] ||= uncached_full_path + route&.path || build_full_path end def full_path_components full_path.split('/') end - def expires_full_path_cache - RequestStore.delete(full_path_key) if RequestStore.active? - @full_path = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables - end - def build_full_path if parent && path parent.full_path + '/' + path @@ -138,16 +121,6 @@ module Routable self.errors[:path].concat(route_path_errors) if route_path_errors end - def uncached_full_path - if route && route.path.present? - @full_path ||= route.path # rubocop:disable Gitlab/ModuleWithInstanceVariables - else - update_route if persisted? - - build_full_path - end - end - def full_name_changed? name_changed? || parent_changed? end @@ -156,10 +129,6 @@ module Routable path_changed? || parent_changed? end - def full_path_key - @full_path_key ||= "routable/full_path/#{self.class.name}/#{self.id}" - end - def build_full_name if parent && name parent.human_name + ' / ' + name @@ -168,18 +137,9 @@ module Routable end end - def update_route - return if Gitlab::Database.read_only? - - prepare_route - route.save - end - def prepare_route route || build_route(source: self) route.path = build_full_path route.name = build_full_name - @full_path = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables - @full_name = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables end end diff --git a/app/models/concerns/storage/legacy_namespace.rb b/app/models/concerns/storage/legacy_namespace.rb index ae6595320cb..f5225cd81ed 100644 --- a/app/models/concerns/storage/legacy_namespace.rb +++ b/app/models/concerns/storage/legacy_namespace.rb @@ -11,8 +11,6 @@ module Storage Namespace.find(parent_id_was) # raise NotFound early if needed end - expires_full_path_cache - move_repositories if parent_changed? diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 7034c633268..c1dc2f55346 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -304,7 +304,6 @@ class Namespace < ActiveRecord::Base def write_projects_repository_config all_projects.find_each do |project| - project.expires_full_path_cache # we need to clear cache to validate renames correctly project.write_repository_config end end diff --git a/app/models/project.rb b/app/models/project.rb index 32315dfaa01..b876270116e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1238,8 +1238,6 @@ class Project < ActiveRecord::Base return true if skip_disk_validation return false unless repository_storage - expires_full_path_cache # we need to clear cache to validate renames correctly - # Check if repository with same path already exists on disk we can # skip this for the hashed storage because the path does not change if legacy_storage? && repository_with_same_path_already_exists? @@ -1618,7 +1616,6 @@ class Project < ActiveRecord::Base # When we import a project overwriting the original project, there # is a move operation. In that case we don't want to send the instructions. send_move_instructions(full_path_was) unless import_started? - expires_full_path_cache self.old_path_with_namespace = full_path_was SystemHooksService.new.execute_hooks_for(self, :rename) diff --git a/app/serializers/pipeline_serializer.rb b/app/serializers/pipeline_serializer.rb index 4a33160afa1..3205578b83e 100644 --- a/app/serializers/pipeline_serializer.rb +++ b/app/serializers/pipeline_serializer.rb @@ -11,10 +11,15 @@ class PipelineSerializer < BaseSerializer :retryable_builds, :cancelable_statuses, :trigger_requests, - :project, :manual_actions, :artifacts, - { pending_builds: :project } + { + pending_builds: :project, + project: [:route, { namespace: :route }], + artifacts: { + project: [:route, { namespace: :route }] + } + } ]) end diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index a4a66330546..c2a0c5fa7f3 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -77,7 +77,6 @@ module Projects Gitlab::PagesTransfer.new.move_project(project.path, @old_namespace.full_path, @new_namespace.full_path) project.old_path_with_namespace = @old_path - project.expires_full_path_cache write_repository_config(@new_path) diff --git a/app/views/admin/projects/_projects.html.haml b/app/views/admin/projects/_projects.html.haml index 00933d726d9..fdaacc098e0 100644 --- a/app/views/admin/projects/_projects.html.haml +++ b/app/views/admin/projects/_projects.html.haml @@ -17,7 +17,7 @@ - if project.archived %span.badge.badge-warning archived .title - = link_to [:admin, project.namespace.becomes(Namespace), project] do + = link_to(admin_namespace_project_path(project.namespace, project)) do .dash-project-avatar .avatar-container.s40 = project_icon(project, alt: '', class: 'avatar project-avatar s40') diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml index 64c441492bc..759efd4e9d4 100644 --- a/app/views/projects/jobs/_sidebar.html.haml +++ b/app/views/projects/jobs/_sidebar.html.haml @@ -86,7 +86,7 @@ - HasStatus::ORDERED_STATUSES.each do |build_status| - builds.select{|build| build.status == build_status}.each do |build| .build-job{ class: sidebar_build_class(build, @build), data: { stage: build.stage } } - - tooltip = sanitize(build.tooltip_message) + - tooltip = sanitize(build.tooltip_message.dup) = link_to(project_job_path(@project, build), data: { toggle: 'tooltip', html: 'true', title: tooltip, container: 'body' }) do = sprite_icon('arrow-right', size:16, css_class: 'icon-arrow-right') %span{ class: "ci-status-icon-#{build.status}" } diff --git a/changelogs/unreleased/4525-fix-project-indexes.yml b/changelogs/unreleased/4525-fix-project-indexes.yml deleted file mode 100644 index 930e3b934c2..00000000000 --- a/changelogs/unreleased/4525-fix-project-indexes.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Rework some projects table indexes around repository_storage field -merge_request: 20377 -author: -type: fixed diff --git a/changelogs/unreleased/47728-mr-api-documentation-changes.yml b/changelogs/unreleased/47728-mr-api-documentation-changes.yml new file mode 100644 index 00000000000..12720f280a1 --- /dev/null +++ b/changelogs/unreleased/47728-mr-api-documentation-changes.yml @@ -0,0 +1,5 @@ +--- +title: Remove changes_count from MR API documentation where necessary +merge_request: 19745 +author: Jan Beckmann +type: fixed diff --git a/changelogs/unreleased/48817-fix-mr-changes-discussion-navigation.yml b/changelogs/unreleased/48817-fix-mr-changes-discussion-navigation.yml deleted file mode 100644 index ec4b843b863..00000000000 --- a/changelogs/unreleased/48817-fix-mr-changes-discussion-navigation.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix navigation to First and Next discussion on MR Changes tab -merge_request: 20434 -author: -type: fixed diff --git a/changelogs/unreleased/_acet-fix-expanding-context-lines.yml b/changelogs/unreleased/_acet-fix-expanding-context-lines.yml deleted file mode 100644 index 41b4dbca5d6..00000000000 --- a/changelogs/unreleased/_acet-fix-expanding-context-lines.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix rendering of the context lines in MR diffs page -merge_request: 20642 -author: -type: fixed diff --git a/changelogs/unreleased/_acet-fix-mr-autosave.yml b/changelogs/unreleased/_acet-fix-mr-autosave.yml deleted file mode 100644 index f87b32f68e2..00000000000 --- a/changelogs/unreleased/_acet-fix-mr-autosave.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix autosave and ESC confirmation issues for MR discussions -merge_request: 20569 -author: -type: fixed diff --git a/changelogs/unreleased/_acet-fix-outdated-discussions.yml b/changelogs/unreleased/_acet-fix-outdated-discussions.yml deleted file mode 100644 index d31483b4765..00000000000 --- a/changelogs/unreleased/_acet-fix-outdated-discussions.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix showing outdated discussions on Changes tab -merge_request: 20445 -author: -type: fixed diff --git a/changelogs/unreleased/ide-edit-json-files.yml b/changelogs/unreleased/ide-edit-json-files.yml deleted file mode 100644 index 2a6e6b80de1..00000000000 --- a/changelogs/unreleased/ide-edit-json-files.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fixed IDE not opening JSON files -merge_request: 20798 -author: -type: fixed diff --git a/changelogs/unreleased/project-dropdown-list-overflow.yml b/changelogs/unreleased/project-dropdown-list-overflow.yml deleted file mode 100644 index 9b74a68291b..00000000000 --- a/changelogs/unreleased/project-dropdown-list-overflow.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Don't overflow project/group dropdown results -merge_request: 20704 -author: gfyoung -type: fixed diff --git a/changelogs/unreleased/stop-dynamic-routable-creation.yml b/changelogs/unreleased/stop-dynamic-routable-creation.yml new file mode 100644 index 00000000000..8bfcb5b2d11 --- /dev/null +++ b/changelogs/unreleased/stop-dynamic-routable-creation.yml @@ -0,0 +1,5 @@ +--- +title: Stop dynamically creating project and namespace routes +merge_request: 20313 +author: +type: performance diff --git a/changelogs/unreleased/tc-reorder-mail-notify-references.yml b/changelogs/unreleased/tc-reorder-mail-notify-references.yml new file mode 100644 index 00000000000..689afda0259 --- /dev/null +++ b/changelogs/unreleased/tc-reorder-mail-notify-references.yml @@ -0,0 +1,5 @@ +--- +title: Put fallback reply-key address first in the References header +merge_request: 20871 +author: +type: changed diff --git a/changelogs/unreleased/tz-mr-refactor-memory-reduction.yml b/changelogs/unreleased/tz-mr-refactor-memory-reduction.yml deleted file mode 100644 index 16003fa9cad..00000000000 --- a/changelogs/unreleased/tz-mr-refactor-memory-reduction.yml +++ /dev/null @@ -1,5 +0,0 @@ ----
-title: Reduces the client side memory footprint on merge requests
-merge_request: 20744
-author:
-type: performance
diff --git a/changelogs/unreleased/zj-backup-timeout.yml b/changelogs/unreleased/zj-backup-timeout.yml deleted file mode 100644 index b2ad2ed8c63..00000000000 --- a/changelogs/unreleased/zj-backup-timeout.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Disable Gitaly timeouts when creating or restoring backups -merge_request: 20810 -author: -type: fixed diff --git a/db/migrate/20180702124358_remove_orphaned_routes.rb b/db/migrate/20180702124358_remove_orphaned_routes.rb new file mode 100644 index 00000000000..6f6e289ba87 --- /dev/null +++ b/db/migrate/20180702124358_remove_orphaned_routes.rb @@ -0,0 +1,49 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RemoveOrphanedRoutes < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + class Route < ActiveRecord::Base + self.table_name = 'routes' + include EachBatch + + def self.orphaned_namespace_routes + where(source_type: 'Namespace') + .where('NOT EXISTS ( SELECT 1 FROM namespaces WHERE namespaces.id = routes.source_id )') + end + + def self.orphaned_project_routes + where(source_type: 'Project') + .where('NOT EXISTS ( SELECT 1 FROM projects WHERE projects.id = routes.source_id )') + end + end + + def up + # Some of these queries can take up to 10 seconds to run on GitLab.com, + # which is pretty close to our 15 second statement timeout. To ensure a + # smooth deployment procedure we disable the statement timeouts for this + # migration, just in case. + disable_statement_timeout + + # On GitLab.com there are around 4000 orphaned project routes, and around + # 150 orphaned namespace routes. + [ + Route.orphaned_project_routes, + Route.orphaned_namespace_routes + ].each do |relation| + relation.each_batch(of: 1_000) do |batch| + batch.delete_all + end + end + end + + def down + # There is no way to restore orphaned routes, and this doesn't make any + # sense anyway. + end +end diff --git a/db/migrate/20180702134423_generate_missing_routes.rb b/db/migrate/20180702134423_generate_missing_routes.rb new file mode 100644 index 00000000000..994725f9bd1 --- /dev/null +++ b/db/migrate/20180702134423_generate_missing_routes.rb @@ -0,0 +1,143 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +# This migration generates missing routes for any projects and namespaces that +# don't already have a route. +# +# On GitLab.com this would insert 611 project routes, and 0 namespace routes. +# The exact number could vary per instance, so we take care of both just in +# case. +class GenerateMissingRoutes < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + class User < ActiveRecord::Base + self.table_name = 'users' + end + + class Route < ActiveRecord::Base + self.table_name = 'routes' + end + + module Routable + def build_full_path + if parent && path + parent.build_full_path + '/' + path + else + path + end + end + + def build_full_name + if parent && name + parent.human_name + ' / ' + name + else + name + end + end + + def human_name + build_full_name + end + + def attributes_for_insert + time = Time.zone.now + + { + # We can't use "self.class.name" here as that would include the + # migration namespace. + source_type: source_type_for_route, + source_id: id, + created_at: time, + updated_at: time, + name: build_full_name, + + # The route path might already be taken. Instead of trying to generate a + # new unique name on every conflict, we just append the row ID to the + # route path. + path: "#{build_full_path}-#{id}" + } + end + end + + class Project < ActiveRecord::Base + self.table_name = 'projects' + + include EachBatch + include GenerateMissingRoutes::Routable + + belongs_to :namespace, class_name: 'GenerateMissingRoutes::Namespace' + + has_one :route, + as: :source, + inverse_of: :source, + class_name: 'GenerateMissingRoutes::Route' + + alias_method :parent, :namespace + alias_attribute :parent_id, :namespace_id + + def self.without_routes + where( + 'NOT EXISTS ( + SELECT 1 + FROM routes + WHERE source_type = ? + AND source_id = projects.id + )', + 'Project' + ) + end + + def source_type_for_route + 'Project' + end + end + + class Namespace < ActiveRecord::Base + self.table_name = 'namespaces' + + include EachBatch + include GenerateMissingRoutes::Routable + + belongs_to :parent, class_name: 'GenerateMissingRoutes::Namespace' + belongs_to :owner, class_name: 'GenerateMissingRoutes::User' + + has_one :route, + as: :source, + inverse_of: :source, + class_name: 'GenerateMissingRoutes::Route' + + def self.without_routes + where( + 'NOT EXISTS ( + SELECT 1 + FROM routes + WHERE source_type = ? + AND source_id = namespaces.id + )', + 'Namespace' + ) + end + + def source_type_for_route + 'Namespace' + end + end + + def up + [Namespace, Project].each do |model| + model.without_routes.each_batch(of: 100) do |batch| + rows = batch.map(&:attributes_for_insert) + + Gitlab::Database.bulk_insert(:routes, rows) + end + end + end + + def down + # Removing routes we previously generated makes no sense. + end +end diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 34c2dd7b34d..58d05a70d05 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -15,11 +15,6 @@ given state (`opened`, `closed`, `locked`, or `merged`) or all of them (`all`). The pagination parameters `page` and `per_page` can be used to restrict the list of merge requests. -**Note**: the `changes_count` value in the response is a string, not an -integer. This is because when an MR has too many changes to display and store, -it will be capped at 1,000. In that case, the API will return the string -`"1000+"` for the changes count. - ``` GET /merge_requests GET /merge_requests?state=opened @@ -104,7 +99,6 @@ Parameters: "sha": "8888888888888888888888888888888888888888", "merge_commit_sha": null, "user_notes_count": 1, - "changes_count": "1", "should_remove_source_branch": true, "force_remove_source_branch": false, "squash": false, @@ -144,10 +138,6 @@ will be the same. In the case of a merge request from a fork, `target_project_id` and `project_id` will be the same and `source_project_id` will be the fork project's ID. -**Note**: the `changes_count` value in the response is a string, not an -integer. This is because when an MR has too many changes to display and store, -it will be capped at 1,000. In that case, the API will return the string -`"1000+"` for the changes count. Parameters: @@ -224,7 +214,6 @@ Parameters: "sha": "8888888888888888888888888888888888888888", "merge_commit_sha": null, "user_notes_count": 1, - "changes_count": "1", "should_remove_source_branch": true, "force_remove_source_branch": false, "squash": false, @@ -331,7 +320,6 @@ Parameters: "sha": "8888888888888888888888888888888888888888", "merge_commit_sha": null, "user_notes_count": 1, - "changes_count": "1", "should_remove_source_branch": true, "force_remove_source_branch": false, "web_url": "http://example.com/example/example/merge_requests/1", @@ -350,6 +338,11 @@ Parameters: Shows information about a single merge request. +**Note**: the `changes_count` value in the response is a string, not an +integer. This is because when an MR has too many changes to display and store, +it will be capped at 1,000. In that case, the API will return the string +`"1000+"` for the changes count. + ``` GET /projects/:id/merge_requests/:merge_request_iid ``` diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 290fcd4f8e6..d89716b1b50 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -55,10 +55,8 @@ describe Projects::PipelinesController do stub_feature_flags(ci_pipeline_persisted_stages: false) end - it 'returns JSON with serialized pipelines', :request_store do - queries = ActiveRecord::QueryRecorder.new do - get_pipelines_index_json - end + it 'returns JSON with serialized pipelines' do + get_pipelines_index_json expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('pipeline') @@ -73,8 +71,14 @@ describe Projects::PipelinesController do json_response.dig('pipelines', 0, 'details', 'stages').tap do |stages| expect(stages.count).to eq 3 end + end + + it 'does not execute N+1 queries' do + queries = ActiveRecord::QueryRecorder.new do + get_pipelines_index_json + end - expect(queries.count).to be_within(5).of(30) + expect(queries.count).to be <= 36 end end diff --git a/spec/fixtures/emails/commands_in_reply.eml b/spec/fixtures/emails/commands_in_reply.eml index 712f6f797b4..6a467ccbbc6 100644 --- a/spec/fixtures/emails/commands_in_reply.eml +++ b/spec/fixtures/emails/commands_in_reply.eml @@ -8,7 +8,7 @@ From: Jake the Dog <jake@adventuretime.ooo> To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> In-Reply-To: <issue_1@localhost> -References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> +References: <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> <issue_1@localhost> Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux' Mime-Version: 1.0 Content-Type: text/plain; diff --git a/spec/fixtures/emails/commands_only_reply.eml b/spec/fixtures/emails/commands_only_reply.eml index 2d2e2f94290..9b8d25c406e 100644 --- a/spec/fixtures/emails/commands_only_reply.eml +++ b/spec/fixtures/emails/commands_only_reply.eml @@ -8,7 +8,7 @@ From: Jake the Dog <jake@adventuretime.ooo> To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> In-Reply-To: <issue_1@localhost> -References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> +References: <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> <issue_1@localhost> Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux' Mime-Version: 1.0 Content-Type: text/plain; diff --git a/spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references.eml b/spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references.eml index 39d5cefbc2a..609d9d9e93d 100644 --- a/spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references.eml +++ b/spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references.eml @@ -8,7 +8,7 @@ From: Jake the Dog <jake@adventuretime.ooo> To: reply@appmail.adventuretime.ooo Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> In-Reply-To: <issue_1@localhost> -References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> +References: <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> <issue_1@localhost> Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux' Mime-Version: 1.0 Content-Type: text/plain; diff --git a/spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references_with_a_comma.eml b/spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references_with_a_comma.eml index 6823db0cfc8..7be1ed5bf44 100644 --- a/spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references_with_a_comma.eml +++ b/spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references_with_a_comma.eml @@ -8,7 +8,7 @@ From: Jake the Dog <jake@adventuretime.ooo> To: reply@appmail.adventuretime.ooo Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> In-Reply-To: <issue_1@localhost> -References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>,<exchange@microsoft.com> +References: <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> <issue_1@localhost>,<exchange@microsoft.com> Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux' Mime-Version: 1.0 Content-Type: text/plain; diff --git a/spec/fixtures/emails/update_commands_only_reply.eml b/spec/fixtures/emails/update_commands_only_reply.eml index bb0d2b0e03a..927a62f6475 100644 --- a/spec/fixtures/emails/update_commands_only_reply.eml +++ b/spec/fixtures/emails/update_commands_only_reply.eml @@ -8,7 +8,7 @@ From: Jake the Dog <jake@adventuretime.ooo> To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> In-Reply-To: <issue_1@localhost> -References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> +References: <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> <issue_1@localhost> Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux' Mime-Version: 1.0 Content-Type: text/plain; diff --git a/spec/fixtures/emails/valid_reply.eml b/spec/fixtures/emails/valid_reply.eml index 980e10a8812..5fbeebdb6b0 100644 --- a/spec/fixtures/emails/valid_reply.eml +++ b/spec/fixtures/emails/valid_reply.eml @@ -8,7 +8,7 @@ From: Jake the Dog <jake@adventuretime.ooo> To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> In-Reply-To: <issue_1@localhost> -References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> +References: <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> <issue_1@localhost> Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux' Mime-Version: 1.0 Content-Type: text/plain; diff --git a/spec/javascripts/pipelines/graph/dropdown_job_component_spec.js b/spec/javascripts/pipelines/graph/dropdown_job_component_spec.js index 608a0d4be67..ff584396d61 100644 --- a/spec/javascripts/pipelines/graph/dropdown_job_component_spec.js +++ b/spec/javascripts/pipelines/graph/dropdown_job_component_spec.js @@ -12,7 +12,7 @@ describe('dropdown job component', () => { id: 4256, name: '<img src=x onerror=alert(document.domain)>', status: { - icon: 'icon_status_success', + icon: 'status_success', text: 'passed', label: 'passed', tooltip: 'passed', @@ -31,7 +31,7 @@ describe('dropdown job component', () => { id: 4299, name: 'test', status: { - icon: 'icon_status_success', + icon: 'status_success', text: 'passed', label: 'passed', tooltip: 'passed', @@ -50,7 +50,7 @@ describe('dropdown job component', () => { name: 'rspec:linux', size: 2, status: { - icon: 'icon_status_success', + icon: 'status_success', text: 'passed', label: 'passed', tooltip: 'passed', diff --git a/spec/javascripts/pipelines/graph/job_component_spec.js b/spec/javascripts/pipelines/graph/job_component_spec.js index 59f18d9397d..215ce1e81b5 100644 --- a/spec/javascripts/pipelines/graph/job_component_spec.js +++ b/spec/javascripts/pipelines/graph/job_component_spec.js @@ -169,7 +169,7 @@ describe('pipeline graph job component', () => { id: 4259, name: '<img src=x onerror=alert(document.domain)>', status: { - icon: 'icon_status_success', + icon: 'status_success', label: 'success', tooltip: 'failed', }, diff --git a/spec/migrations/generate_missing_routes_spec.rb b/spec/migrations/generate_missing_routes_spec.rb new file mode 100644 index 00000000000..32515d353b0 --- /dev/null +++ b/spec/migrations/generate_missing_routes_spec.rb @@ -0,0 +1,84 @@ +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20180702134423_generate_missing_routes.rb') + +describe GenerateMissingRoutes, :migration do + describe '#up' do + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:routes) { table(:routes) } + + it 'creates routes for projects without a route' do + namespace = namespaces.create!(name: 'GitLab', path: 'gitlab') + + routes.create!( + path: 'gitlab', + source_type: 'Namespace', + source_id: namespace.id + ) + + project = projects.create!( + name: 'GitLab CE', + path: 'gitlab-ce', + namespace_id: namespace.id + ) + + described_class.new.up + + route = routes.where(source_type: 'Project').take + + expect(route.source_id).to eq(project.id) + expect(route.path).to eq("gitlab/gitlab-ce-#{project.id}") + end + + it 'creates routes for namespaces without a route' do + namespace = namespaces.create!(name: 'GitLab', path: 'gitlab') + + described_class.new.up + + route = routes.where(source_type: 'Namespace').take + + expect(route.source_id).to eq(namespace.id) + expect(route.path).to eq("gitlab-#{namespace.id}") + end + + it 'does not create routes for namespaces that already have a route' do + namespace = namespaces.create!(name: 'GitLab', path: 'gitlab') + + routes.create!( + path: 'gitlab', + source_type: 'Namespace', + source_id: namespace.id + ) + + described_class.new.up + + expect(routes.count).to eq(1) + end + + it 'does not create routes for projects that already have a route' do + namespace = namespaces.create!(name: 'GitLab', path: 'gitlab') + + routes.create!( + path: 'gitlab', + source_type: 'Namespace', + source_id: namespace.id + ) + + project = projects.create!( + name: 'GitLab CE', + path: 'gitlab-ce', + namespace_id: namespace.id + ) + + routes.create!( + path: 'gitlab/gitlab-ce', + source_type: 'Project', + source_id: project.id + ) + + described_class.new.up + + expect(routes.count).to eq(2) + end + end +end diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index ed3e28fbeca..565266321d3 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -12,16 +12,6 @@ describe Group, 'Routable' do it { is_expected.to have_many(:redirect_routes).dependent(:destroy) } end - describe 'GitLab read-only instance' do - it 'does not save route if route is not present' do - group.route.path = '' - allow(Gitlab::Database).to receive(:read_only?).and_return(true) - expect(group).to receive(:update_route).and_call_original - - expect { group.full_path }.to change { Route.count }.by(0) - end - end - describe 'Callbacks' do it 'creates route record on create' do expect(group.route.path).to eq(group.path) @@ -131,29 +121,6 @@ describe Group, 'Routable' do it { expect(group.full_path).to eq(group.path) } it { expect(nested_group.full_path).to eq("#{group.full_path}/#{nested_group.path}") } - - context 'with RequestStore active', :request_store do - it 'does not load the route table more than once' do - group.expires_full_path_cache - expect(group).to receive(:uncached_full_path).once.and_call_original - - 3.times { group.full_path } - expect(group.full_path).to eq(group.path) - end - end - end - - describe '#expires_full_path_cache' do - context 'with RequestStore active', :request_store do - it 'expires the full_path cache' do - expect(group.full_path).to eq('foo') - - group.route.update(path: 'bar', name: 'bar') - group.expires_full_path_cache - - expect(group.full_path).to eq('bar') - end - end end describe '#full_name' do diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 8ff16cbd7cc..9b7f932ec3a 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -323,6 +323,16 @@ describe Namespace do parent.update(path: 'mygroup_new') + # Routes are loaded when creating the projects, so we need to manually + # reload them for the below code to be aware of the above UPDATE. + [ + project_in_parent_group, + hashed_project_in_subgroup, + legacy_project_in_subgroup + ].each do |project| + project.route.reload + end + expect(project_rugged(project_in_parent_group).config['gitlab.fullpath']).to eq "mygroup_new/#{project_in_parent_group.path}" expect(project_rugged(hashed_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{hashed_project_in_subgroup.path}" expect(project_rugged(legacy_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{legacy_project_in_subgroup.path}" diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 88442247093..b75ca91b007 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2958,8 +2958,6 @@ describe Project do expect(project).to receive(:expire_caches_before_rename) - expect(project).to receive(:expires_full_path_cache) - project.rename_repo end @@ -3119,8 +3117,6 @@ describe Project do expect(project).to receive(:expire_caches_before_rename) - expect(project).to receive(:expires_full_path_cache) - project.rename_repo end diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index eb4235e3ee6..cf57776346a 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -125,7 +125,7 @@ describe PipelineSerializer do it 'verifies number of queries', :request_store do recorded = ActiveRecord::QueryRecorder.new { subject } - expect(recorded.count).to be_within(2).of(27) + expect(recorded.count).to be_within(2).of(31) expect(recorded.cached_count).to eq(0) end end @@ -144,7 +144,7 @@ describe PipelineSerializer do # pipeline. With the same ref this check is cached but if refs are # different then there is an extra query per ref # https://gitlab.com/gitlab-org/gitlab-ce/issues/46368 - expect(recorded.count).to be_within(2).of(30) + expect(recorded.count).to be_within(2).of(34) expect(recorded.cached_count).to eq(0) end end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 7e85f599afb..1a85c52fc97 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -31,12 +31,6 @@ describe Projects::TransferService do transfer_project(project, user, group) end - it 'expires full_path cache' do - expect(project).to receive(:expires_full_path_cache) - - transfer_project(project, user, group) - end - it 'invalidates the user\'s personal_project_count cache' do expect(user).to receive(:invalidate_personal_projects_count) diff --git a/spec/support/shared_examples/notify_shared_examples.rb b/spec/support/shared_examples/notify_shared_examples.rb index d176d3fa425..5beae005cf7 100644 --- a/spec/support/shared_examples/notify_shared_examples.rb +++ b/spec/support/shared_examples/notify_shared_examples.rb @@ -77,7 +77,7 @@ shared_examples 'a thread answer email with reply-by-email enabled' do aggregate_failures do is_expected.to have_header('Message-ID', /\A<.*@#{host}>\Z/) is_expected.to have_header('In-Reply-To', "<#{route_key}@#{host}>") - is_expected.to have_header('References', /\A<#{route_key}@#{host}> <reply\-.*@#{host}>\Z/ ) + is_expected.to have_header('References', /\A<reply\-.*@#{host}> <#{route_key}@#{host}>\Z/ ) is_expected.to have_subject(/^Re: /) end end |