diff options
| author | Winnie Hellmann <winnie@gitlab.com> | 2017-11-17 18:15:35 +0000 |
|---|---|---|
| committer | Winnie Hellmann <winnie@gitlab.com> | 2017-11-17 18:15:35 +0000 |
| commit | 9f40c578acef212734fae35f149b53d7434698f4 (patch) | |
| tree | 642f42a3cc53e1e5805de8895e9c513497c503ca | |
| parent | bd45973c5150847c45dedf784d4d5f976c8d07ed (diff) | |
| parent | ef22f0370a73fd7da08ba308250364b288e0a57b (diff) | |
| download | gitlab-ce-9f40c578acef212734fae35f149b53d7434698f4.tar.gz | |
Merge branch '10-2-stable-prepare-rc3' into '10-2-stable'
Prepare 10.2 RC3 release
See merge request gitlab-org/gitlab-ce!15404
118 files changed, 1693 insertions, 574 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6a5050b553f..e7b3662c009 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -581,7 +581,7 @@ codequality: script: - cp .rubocop.yml .rubocop.yml.bak - grep -v "rubocop-gitlab-security" .rubocop.yml.bak > .rubocop.yml - - docker run --env CODECLIMATE_CODE="$PWD" --volume "$PWD":/code --volume /var/run/docker.sock:/var/run/docker.sock --volume /tmp/cc:/tmp/cc codeclimate/codeclimate analyze -f json > raw_codeclimate.json + - docker run --env CODECLIMATE_CODE="$PWD" --volume "$PWD":/code --volume /var/run/docker.sock:/var/run/docker.sock --volume /tmp/cc:/tmp/cc codeclimate/codeclimate:0.69.0 analyze -f json > raw_codeclimate.json - cat raw_codeclimate.json | docker run -i stedolan/jq -c 'map({check_name,fingerprint,location})' > codeclimate.json - mv .rubocop.yml.bak .rubocop.yml artifacts: diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index 15a27998172..bea438e9ade 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -3.3.0 +3.3.1 diff --git a/app/assets/javascripts/issue_show/components/app.vue b/app/assets/javascripts/issue_show/components/app.vue index d1aa83ea57f..e8ac8d3b5bb 100644 --- a/app/assets/javascripts/issue_show/components/app.vue +++ b/app/assets/javascripts/issue_show/components/app.vue @@ -29,6 +29,11 @@ export default { required: false, default: false, }, + showDeleteButton: { + type: Boolean, + required: false, + default: true, + }, issuableRef: { type: String, required: true, @@ -92,6 +97,11 @@ export default { type: String, required: true, }, + issuableType: { + type: String, + required: false, + default: 'issue', + }, }, data() { const store = new Store({ @@ -157,21 +167,21 @@ export default { }) .catch(() => { eventHub.$emit('close.form'); - window.Flash('Error updating issue'); + window.Flash(`Error updating ${this.issuableType}`); }); }, deleteIssuable() { this.service.deleteIssuable() .then(res => res.json()) .then((data) => { - // Stop the poll so we don't get 404's with the issue not existing + // Stop the poll so we don't get 404's with the issuable not existing this.poll.stop(); gl.utils.visitUrl(data.web_url); }) .catch(() => { eventHub.$emit('close.form'); - window.Flash('Error deleting issue'); + window.Flash(`Error deleting ${this.issuableType}`); }); }, }, @@ -223,6 +233,7 @@ export default { :markdown-preview-path="markdownPreviewPath" :project-path="projectPath" :project-namespace="projectNamespace" + :show-delete-button="showDeleteButton" /> <div v-else> <title-component diff --git a/app/assets/javascripts/issue_show/components/edit_actions.vue b/app/assets/javascripts/issue_show/components/edit_actions.vue index 8c81575fe6f..a539506bce2 100644 --- a/app/assets/javascripts/issue_show/components/edit_actions.vue +++ b/app/assets/javascripts/issue_show/components/edit_actions.vue @@ -13,6 +13,11 @@ type: Object, required: true, }, + showDeleteButton: { + type: Boolean, + required: false, + default: true, + }, }, data() { return { @@ -23,6 +28,9 @@ isSubmitEnabled() { return this.formState.title.trim() !== ''; }, + shouldShowDeleteButton() { + return this.canDestroy && this.showDeleteButton; + }, }, methods: { closeForm() { @@ -62,7 +70,7 @@ Cancel </button> <button - v-if="canDestroy" + v-if="shouldShowDeleteButton" class="btn btn-danger pull-right append-right-default" :class="{ disabled: deleteLoading }" type="button" diff --git a/app/assets/javascripts/issue_show/components/form.vue b/app/assets/javascripts/issue_show/components/form.vue index 28bf6c67ea5..8bb5c86d567 100644 --- a/app/assets/javascripts/issue_show/components/form.vue +++ b/app/assets/javascripts/issue_show/components/form.vue @@ -36,6 +36,11 @@ type: String, required: true, }, + showDeleteButton: { + type: Boolean, + required: false, + default: true, + }, }, components: { lockedWarning, @@ -81,6 +86,7 @@ :markdown-docs-path="markdownDocsPath" /> <edit-actions :form-state="formState" - :can-destroy="canDestroy" /> + :can-destroy="canDestroy" + :show-delete-button="showDeleteButton" /> </form> </template> diff --git a/app/assets/javascripts/lib/utils/number_utils.js b/app/assets/javascripts/lib/utils/number_utils.js index 917a45eb06b..a02c79b787e 100644 --- a/app/assets/javascripts/lib/utils/number_utils.js +++ b/app/assets/javascripts/lib/utils/number_utils.js @@ -52,3 +52,31 @@ export function bytesToKiB(number) { export function bytesToMiB(number) { return number / (BYTES_IN_KIB * BYTES_IN_KIB); } + +/** + * Utility function that calculates GiB of the given bytes. + * @param {Number} number + * @returns {Number} + */ +export function bytesToGiB(number) { + return number / (BYTES_IN_KIB * BYTES_IN_KIB * BYTES_IN_KIB); +} + +/** + * Port of rails number_to_human_size + * Formats the bytes in number into a more understandable + * representation (e.g., giving it 1500 yields 1.5 KB). + * + * @param {Number} size + * @returns {String} + */ +export function numberToHumanSize(size) { + if (size < BYTES_IN_KIB) { + return `${size} bytes`; + } else if (size < BYTES_IN_KIB * BYTES_IN_KIB) { + return `${bytesToKiB(size).toFixed(2)} KiB`; + } else if (size < BYTES_IN_KIB * BYTES_IN_KIB * BYTES_IN_KIB) { + return `${bytesToMiB(size).toFixed(2)} MiB`; + } + return `${bytesToGiB(size).toFixed(2)} GiB`; +} diff --git a/app/assets/javascripts/lib/utils/url_utility.js b/app/assets/javascripts/lib/utils/url_utility.js index 1aa63216baf..17236c91490 100644 --- a/app/assets/javascripts/lib/utils/url_utility.js +++ b/app/assets/javascripts/lib/utils/url_utility.js @@ -100,6 +100,10 @@ export function visitUrl(url, external = false) { } } +export function redirectTo(url) { + return window.location.assign(url); +} + window.gl = window.gl || {}; window.gl.utils = { ...(window.gl.utils || {}), diff --git a/app/assets/javascripts/members.js b/app/assets/javascripts/members.js index 6264750a4fb..52315e969d1 100644 --- a/app/assets/javascripts/members.js +++ b/app/assets/javascripts/members.js @@ -5,7 +5,6 @@ export default class Members { } addListeners() { - $('.project_member, .group_member').off('ajax:success').on('ajax:success', this.removeRow); $('.js-member-update-control').off('change').on('change', this.formSubmit.bind(this)); $('.js-edit-member-form').off('ajax:success').on('ajax:success', this.formSuccess.bind(this)); gl.utils.disableButtonIfEmptyField('#user_ids', 'input[name=commit]', 'change'); @@ -33,17 +32,6 @@ export default class Members { }); }); } - // eslint-disable-next-line class-methods-use-this - removeRow(e) { - const $target = $(e.target); - - if ($target.hasClass('btn-remove')) { - $target.closest('.member') - .fadeOut(function fadeOutMemberRow() { - $(this).remove(); - }); - } - } formSubmit(e, $el = null) { const $this = e ? $(e.currentTarget) : $el; diff --git a/app/assets/javascripts/registry/components/table_registry.vue b/app/assets/javascripts/registry/components/table_registry.vue index e917279947e..14d43e135fe 100644 --- a/app/assets/javascripts/registry/components/table_registry.vue +++ b/app/assets/javascripts/registry/components/table_registry.vue @@ -8,6 +8,7 @@ import tooltip from '../../vue_shared/directives/tooltip'; import timeagoMixin from '../../vue_shared/mixins/timeago'; import { errorMessages, errorMessagesTypes } from '../constants'; + import { numberToHumanSize } from '../../lib/utils/number_utils'; export default { props: { @@ -41,6 +42,10 @@ return item.layers ? n__('%d layer', '%d layers', item.layers) : ''; }, + formatSize(size) { + return numberToHumanSize(size); + }, + handleDeleteRegistry(registry) { this.deleteRegistry(registry) .then(() => this.fetchList({ repo: this.repo })) @@ -97,7 +102,7 @@ </span> </td> <td> - {{item.size}} + {{formatSize(item.size)}} <template v-if="item.size && item.layers"> · </template> diff --git a/app/assets/javascripts/vue_shared/components/loading_button.vue b/app/assets/javascripts/vue_shared/components/loading_button.vue index 0cc2653761c..247943f83e6 100644 --- a/app/assets/javascripts/vue_shared/components/loading_button.vue +++ b/app/assets/javascripts/vue_shared/components/loading_button.vue @@ -35,6 +35,11 @@ export default { type: String, required: false, }, + containerClass: { + type: String, + required: false, + default: 'btn btn-align-content', + }, }, components: { loadingIcon, @@ -49,9 +54,9 @@ export default { <template> <button - class="btn btn-align-content" @click="onClick" type="button" + :class="containerClass" :disabled="loading || disabled" > <transition name="fade"> diff --git a/app/assets/stylesheets/framework.scss b/app/assets/stylesheets/framework.scss index c334f39f416..66212be1b8f 100644 --- a/app/assets/stylesheets/framework.scss +++ b/app/assets/stylesheets/framework.scss @@ -34,6 +34,7 @@ @import "framework/modal"; @import "framework/pagination"; @import "framework/panels"; +@import "framework/popup"; @import "framework/secondary-navigation-elements"; @import "framework/selects"; @import "framework/sidebar"; diff --git a/app/assets/stylesheets/framework/blank.scss b/app/assets/stylesheets/framework/blank.scss index 6bb096fc5bd..10f9e9b70b0 100644 --- a/app/assets/stylesheets/framework/blank.scss +++ b/app/assets/stylesheets/framework/blank.scss @@ -7,29 +7,67 @@ width: 100%; height: 100%; padding-bottom: 25px; - border: 1px solid $border-color; border-radius: $border-radius-default; } } -.blank-state { - padding-top: 20px; - padding-bottom: 20px; +.blank-state-row { + display: flex; + flex-wrap: wrap; + justify-content: space-around; + height: 100%; +} + +.blank-state-welcome { text-align: center; + padding: 20px 0 40px; + + .blank-state-welcome-title { + font-size: 24px; + } + + .blank-state-text { + margin-bottom: 0; + } +} - &.blank-state-welcome { - .blank-state-welcome-title { - font-size: 24px; +.blank-state-link { + display: block; + color: $gl-text-color; + flex: 0 0 100%; + margin-bottom: 15px; + + @media (min-width: $screen-sm-min) { + flex: 0 0 49%; + + &:nth-child(odd) { + margin-right: 5px; } - .blank-state-text { - margin-bottom: 0; + &:nth-child(even) { + margin-left: 5px; } } - .blank-state-icon { - padding-bottom: 20px; + &:hover { + background-color: $gray-light; + text-decoration: none; + color: $gl-text-color; + } +} + +.blank-state { + padding: 20px; + border: 1px solid $border-color; + border-radius: $border-radius-default; + + @media (min-width: $screen-sm-min) { + display: flex; + align-items: center; + padding: 50px 30px; + } + .blank-state-icon { svg { display: block; margin: auto; @@ -38,13 +76,17 @@ .blank-state-title { margin-top: 0; - margin-bottom: 10px; font-size: 18px; } - .blank-state-text { - max-width: $container-text-max-width; - margin: 0 auto $gl-padding; - font-size: 14px; + .blank-state-body { + @media (max-width: $screen-xs-max) { + text-align: center; + margin-top: 20px; + } + + @media (min-width: $screen-sm-min) { + padding-left: 20px; + } } } diff --git a/app/assets/stylesheets/framework/blocks.scss b/app/assets/stylesheets/framework/blocks.scss index 9c1439dfad5..91976ca1f56 100644 --- a/app/assets/stylesheets/framework/blocks.scss +++ b/app/assets/stylesheets/framework/blocks.scss @@ -353,3 +353,7 @@ display: -webkit-flex; display: flex; } + +.flex-right { + margin-left: auto; +} diff --git a/app/assets/stylesheets/framework/mixins.scss b/app/assets/stylesheets/framework/mixins.scss index 16d5edde61e..33012133b66 100644 --- a/app/assets/stylesheets/framework/mixins.scss +++ b/app/assets/stylesheets/framework/mixins.scss @@ -180,3 +180,31 @@ display: none; } } + +@mixin triangle($color, $border-color, $size, $border-size) { + &::before, + &::after { + bottom: 100%; + left: 50%; + border: solid transparent; + content: ''; + height: 0; + width: 0; + position: absolute; + pointer-events: none; + } + + &::before { + border-color: transparent; + border-bottom-color: $border-color; + border-width: ($size + $border-size); + margin-left: -($size + $border-size); + } + + &::after { + border-color: transparent; + border-bottom-color: $color; + border-width: $size; + margin-left: -$size; + } +} diff --git a/app/assets/stylesheets/framework/popup.scss b/app/assets/stylesheets/framework/popup.scss new file mode 100644 index 00000000000..5c76205095f --- /dev/null +++ b/app/assets/stylesheets/framework/popup.scss @@ -0,0 +1,15 @@ +.popup { + @include triangle( + $gray-lighter, + $gray-darker, + $popup-triangle-size, + $popup-triangle-border-size + ); + + padding: $gl-padding; + background-color: $gray-lighter; + border: 1px solid $gray-darker; + border-radius: $border-radius-default; + box-shadow: 0 5px 8px $popup-box-shadow-color; + position: relative; +} diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 8ab48e4844f..2dafd1ce47c 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -719,3 +719,10 @@ Image Commenting cursor */ $image-comment-cursor-left-offset: 12; $image-comment-cursor-top-offset: 30; + +/* +Popup +*/ +$popup-triangle-size: 15px; +$popup-triangle-border-size: 1px; +$popup-box-shadow-color: rgba(90, 90, 90, 0.05); diff --git a/app/assets/stylesheets/pages/tree.scss b/app/assets/stylesheets/pages/tree.scss index 50f0ef4414a..65b334662c2 100644 --- a/app/assets/stylesheets/pages/tree.scss +++ b/app/assets/stylesheets/pages/tree.scss @@ -125,7 +125,7 @@ color: $white-normal; } - &:hover { + &:hover:not(.tree-truncated-warning) { td { background-color: $row-hover; border-top: 1px solid $row-hover-border; @@ -198,6 +198,11 @@ } } + .tree-truncated-warning { + color: $orange-600; + background-color: $orange-100; + } + .tree-time-ago { min-width: 135px; color: $gl-text-color-secondary; diff --git a/app/controllers/concerns/lfs_request.rb b/app/controllers/concerns/lfs_request.rb index 6cca9f95618..4311f9d4db9 100644 --- a/app/controllers/concerns/lfs_request.rb +++ b/app/controllers/concerns/lfs_request.rb @@ -92,15 +92,7 @@ module LfsRequest end def storage_project - @storage_project ||= begin - result = project - - # TODO: Make this go to the fork_network root immeadiatly - # dependant on the discussion in: https://gitlab.com/gitlab-org/gitlab-ce/issues/39769 - result = result.fork_source while result.forked? - - result - end + @storage_project ||= project.lfs_storage_project end def objects diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb index 3c64fd964ff..be2e1b47feb 100644 --- a/app/controllers/concerns/notes_actions.rb +++ b/app/controllers/concerns/notes_actions.rb @@ -4,7 +4,7 @@ module NotesActions included do before_action :set_polling_interval_header, only: [:index] - before_action :noteable, only: :index + before_action :require_noteable!, only: [:index, :create] before_action :authorize_admin_note!, only: [:update, :destroy] before_action :note_project, only: [:create] end @@ -90,7 +90,7 @@ module NotesActions if note.persisted? attrs[:valid] = true - if noteable.nil? || noteable.discussions_rendered_on_frontend? + if noteable.discussions_rendered_on_frontend? attrs.merge!(note_serializer.represent(note)) else attrs.merge!( @@ -191,7 +191,11 @@ module NotesActions end def noteable - @noteable ||= notes_finder.target || render_404 + @noteable ||= notes_finder.target || @note&.noteable + end + + def require_noteable! + render_404 unless noteable end def last_fetched_at diff --git a/app/controllers/projects/commits_controller.rb b/app/controllers/projects/commits_controller.rb index 28920877635..5f4afd2cdee 100644 --- a/app/controllers/projects/commits_controller.rb +++ b/app/controllers/projects/commits_controller.rb @@ -57,6 +57,7 @@ class Projects::CommitsController < Projects::ApplicationController @repository.commits(@ref, path: @path, limit: @limit, offset: @offset) end + @commits = @commits.with_pipeline_status @commits = prepare_commits_for_rendering(@commits) end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 22de6680511..abe4e5245b1 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -80,7 +80,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo def commits # Get commits from repository # or from cache if already merged - @commits = prepare_commits_for_rendering(@merge_request.commits) + @commits = + prepare_commits_for_rendering(@merge_request.commits.with_pipeline_status) render json: { html: view_to_html_string('projects/merge_requests/_commits') } end diff --git a/app/controllers/snippets/notes_controller.rb b/app/controllers/snippets/notes_controller.rb index f9496787b15..c8b4682e6dc 100644 --- a/app/controllers/snippets/notes_controller.rb +++ b/app/controllers/snippets/notes_controller.rb @@ -20,6 +20,7 @@ class Snippets::NotesController < ApplicationController def snippet PersonalSnippet.find_by(id: params[:snippet_id]) end + alias_method :noteable, :snippet def note_params super.merge(noteable_id: params[:snippet_id]) diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb index c4ea0f5ac53..0e106e2c85d 100644 --- a/app/helpers/tree_helper.rb +++ b/app/helpers/tree_helper.rb @@ -1,14 +1,23 @@ module TreeHelper + FILE_LIMIT = 1_000 + # Sorts a repository's tree so that folders are before files and renders # their corresponding partials # - # contents - A Grit::Tree object for the current tree + # tree - A `Tree` object for the current tree def render_tree(tree) # Sort submodules and folders together by name ahead of files folders, files, submodules = tree.trees, tree.blobs, tree.submodules - tree = "" + tree = '' items = (folders + submodules).sort_by(&:name) + files - tree << render(partial: "projects/tree/tree_row", collection: items) if items.present? + + if items.size > FILE_LIMIT + tree << render(partial: 'projects/tree/truncated_notice_tree_row', + locals: { limit: FILE_LIMIT, total: items.size }) + items = items.take(FILE_LIMIT) + end + + tree << render(partial: 'projects/tree/tree_row', collection: items) if items.present? tree.html_safe end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index fcbe3d2b67b..797a12d4e63 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -149,34 +149,70 @@ module Ci end end - # ref can't be HEAD or SHA, can only be branch/tag name - scope :latest, ->(ref = nil) do - max_id = unscope(:select) - .select("max(#{quoted_table_name}.id)") - .group(:ref, :sha) - - if ref - where(ref: ref, id: max_id.where(ref: ref)) - else - where(id: max_id) - end - end scope :internal, -> { where(source: internal_sources) } + # Returns the pipelines in descending order (= newest first), optionally + # limited to a number of references. + # + # ref - The name (or names) of the branch(es)/tag(s) to limit the list of + # pipelines to. + def self.newest_first(ref = nil) + relation = order(id: :desc) + + ref ? relation.where(ref: ref) : relation + end + def self.latest_status(ref = nil) - latest(ref).status + newest_first(ref).pluck(:status).first end def self.latest_successful_for(ref) - success.latest(ref).order(id: :desc).first + newest_first(ref).success.take end def self.latest_successful_for_refs(refs) - success.latest(refs).order(id: :desc).each_with_object({}) do |pipeline, hash| + relation = newest_first(refs).success + + relation.each_with_object({}) do |pipeline, hash| hash[pipeline.ref] ||= pipeline end end + # Returns a Hash containing the latest pipeline status for every given + # commit. + # + # The keys of this Hash are the commit SHAs, the values the statuses. + # + # commits - The list of commit SHAs to get the status for. + # ref - The ref to scope the data to (e.g. "master"). If the ref is not + # given we simply get the latest status for the commits, regardless + # of what refs their pipelines belong to. + def self.latest_status_per_commit(commits, ref = nil) + p1 = arel_table + p2 = arel_table.alias + + # This LEFT JOIN will filter out all but the newest row for every + # combination of (project_id, sha) or (project_id, sha, ref) if a ref is + # given. + cond = p1[:sha].eq(p2[:sha]) + .and(p1[:project_id].eq(p2[:project_id])) + .and(p1[:id].lt(p2[:id])) + + cond = cond.and(p1[:ref].eq(p2[:ref])) if ref + join = p1.join(p2, Arel::Nodes::OuterJoin).on(cond) + + relation = select(:sha, :status) + .where(sha: commits) + .where(p2[:id].eq(nil)) + .joins(join.join_sources) + + relation = relation.where(ref: ref) if ref + + relation.each_with_object({}) do |row, hash| + hash[row[:sha]] = row[:status] + end + end + def self.truncate_sha(sha) sha[0...8] end diff --git a/app/models/commit.rb b/app/models/commit.rb index 6dba154a6ea..a31ebe9cc87 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -80,6 +80,7 @@ class Commit @raw = raw_commit @project = project + @statuses = {} end def id @@ -236,11 +237,13 @@ class Commit end def status(ref = nil) - @statuses ||= {} - return @statuses[ref] if @statuses.key?(ref) - @statuses[ref] = pipelines.latest_status(ref) + @statuses[ref] = project.pipelines.latest_status_per_commit(id, ref)[id] + end + + def set_status_for_ref(ref, status) + @statuses[ref] = status end def signature diff --git a/app/models/commit_collection.rb b/app/models/commit_collection.rb new file mode 100644 index 00000000000..dd93af9df64 --- /dev/null +++ b/app/models/commit_collection.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +# A collection of Commit instances for a specific project and Git reference. +class CommitCollection + include Enumerable + + attr_reader :project, :ref, :commits + + # project - The project the commits belong to. + # commits - The Commit instances to store. + # ref - The name of the ref (e.g. "master"). + def initialize(project, commits, ref = nil) + @project = project + @commits = commits + @ref = ref + end + + def each(&block) + commits.each(&block) + end + + # Sets the pipeline status for every commit. + # + # Setting this status ahead of time removes the need for running a query for + # every commit we're displaying. + def with_pipeline_status + statuses = project.pipelines.latest_status_per_commit(map(&:id), ref) + + each do |commit| + commit.set_status_for_ref(ref, statuses[commit.id]) + end + + self + end + + def respond_to_missing?(message, inc_private = false) + commits.respond_to?(message, inc_private) + end + + # rubocop:disable GitlabSecurity/PublicSend + def method_missing(message, *args, &block) + commits.public_send(message, *args, &block) + end +end diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb index 2ec70203710..10659030910 100644 --- a/app/models/concerns/avatarable.rb +++ b/app/models/concerns/avatarable.rb @@ -4,15 +4,26 @@ module Avatarable def avatar_path(only_path: true) return unless self[:avatar].present? - # If only_path is true then use the relative path of avatar. - # Otherwise use full path (including host). asset_host = ActionController::Base.asset_host - gitlab_host = only_path ? gitlab_config.relative_url_root : gitlab_config.url + use_asset_host = asset_host.present? - # If asset_host is set then it is expected that assets are handled by a standalone host. - # That means we do not want to get GitLab's relative_url_root option anymore. - host = (asset_host.present? && (!respond_to?(:public?) || public?)) ? asset_host : gitlab_host + # Avatars for private and internal groups and projects require authentication to be viewed, + # which means they can only be served by Rails, on the regular GitLab host. + # If an asset host is configured, we need to return the fully qualified URL + # instead of only the avatar path, so that Rails doesn't prefix it with the asset host. + if use_asset_host && respond_to?(:public?) && !public? + use_asset_host = false + only_path = false + end - [host, avatar.url].join + url_base = "" + if use_asset_host + url_base << asset_host unless only_path + else + url_base << gitlab_config.base_url unless only_path + url_base << gitlab_config.relative_url_root + end + + url_base + avatar.url end end diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index d88a92dc027..ae5f138a920 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -18,7 +18,8 @@ class DiffNote < Note validate :positions_complete validate :verify_supported - before_validation :set_original_position, :update_position, on: :create + before_validation :set_original_position, on: :create + before_validation :update_position, on: :create, if: :on_text? before_validation :set_line_code after_save :keep_around_commits diff --git a/app/models/key.rb b/app/models/key.rb index f119b15c737..815fd1de909 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -27,8 +27,10 @@ class Key < ActiveRecord::Base after_commit :add_to_shell, on: :create after_create :post_create_hook + after_create :refresh_user_cache after_commit :remove_from_shell, on: :destroy after_destroy :post_destroy_hook + after_destroy :refresh_user_cache def key=(value) value&.delete!("\n\r") @@ -76,6 +78,12 @@ class Key < ActiveRecord::Base ) end + def refresh_user_cache + return unless user + + Users::KeysCountService.new(user).refresh_cache + end + def post_destroy_hook SystemHooksService.new.execute_hooks_for(self, :destroy) end diff --git a/app/models/lfs_object.rb b/app/models/lfs_object.rb index b7cf96abe83..fc586fa216e 100644 --- a/app/models/lfs_object.rb +++ b/app/models/lfs_object.rb @@ -6,16 +6,8 @@ class LfsObject < ActiveRecord::Base mount_uploader :file, LfsObjectUploader - def storage_project(project) - if project && project.forked? - storage_project(project.forked_from_project) - else - project - end - end - def project_allowed_access?(project) - projects.exists?(storage_project(project).id) + projects.exists?(project.lfs_storage_project.id) end def self.destroy_unreferenced diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 1eda0f9cbbd..5382f5cc627 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -284,8 +284,10 @@ class MergeRequestDiff < ActiveRecord::Base def load_commits commits = st_commits.presence || merge_request_diff_commits + commits = commits.map { |commit| Commit.from_hash(commit.to_hash, project) } - commits.map { |commit| Commit.from_hash(commit.to_hash, project) } + CommitCollection + .new(merge_request.source_project, commits, merge_request.source_branch) end def save_diffs diff --git a/app/models/project.rb b/app/models/project.rb index bae16b6b2af..853f6bc504a 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1047,6 +1047,18 @@ class Project < ActiveRecord::Base forked_from_project || fork_network&.root_project end + def lfs_storage_project + @lfs_storage_project ||= begin + result = self + + # TODO: Make this go to the fork_network root immeadiatly + # dependant on the discussion in: https://gitlab.com/gitlab-org/gitlab-ce/issues/39769 + result = result.fork_source while result&.forked? + + result || self + end + end + def personal? !group end diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index 43de6809178..3eecbea8cbf 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -21,7 +21,7 @@ class ProjectWiki end delegate :empty?, to: :pages - delegate :repository_storage_path, to: :project + delegate :repository_storage_path, :hashed_storage?, to: :project def path @project.path + '.wiki' diff --git a/app/models/repository.rb b/app/models/repository.rb index 6bdee538172..b7a5839f6cd 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -132,7 +132,8 @@ class Repository commits = Gitlab::Git::Commit.where(options) commits = Commit.decorate(commits, @project) if commits.present? - commits + + CommitCollection.new(project, commits, ref) end def commits_between(from, to) @@ -148,11 +149,14 @@ class Repository end raw_repository.gitaly_migrate(:commits_by_message) do |is_enabled| - if is_enabled - find_commits_by_message_by_gitaly(query, ref, path, limit, offset) - else - find_commits_by_message_by_shelling_out(query, ref, path, limit, offset) - end + commits = + if is_enabled + find_commits_by_message_by_gitaly(query, ref, path, limit, offset) + else + find_commits_by_message_by_shelling_out(query, ref, path, limit, offset) + end + + CommitCollection.new(project, commits, ref) end end diff --git a/app/models/user.rb b/app/models/user.rb index f436efd604f..47c78d23591 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -170,6 +170,7 @@ class User < ActiveRecord::Base after_save :ensure_namespace_correct after_update :username_changed_hook, if: :username_changed? after_destroy :post_destroy_hook + after_destroy :remove_key_cache after_commit :update_emails_with_primary_email, on: :update, if: -> { previous_changes.key?('email') } after_commit :update_invalid_gpg_signatures, on: :update, if: -> { previous_changes.key?('email') } @@ -624,7 +625,9 @@ class User < ActiveRecord::Base end def require_ssh_key? - keys.count == 0 && Gitlab::ProtocolAccess.allowed?('ssh') + count = Users::KeysCountService.new(self).count + + count.zero? && Gitlab::ProtocolAccess.allowed?('ssh') end def require_password_creation? @@ -886,6 +889,10 @@ class User < ActiveRecord::Base system_hook_service.execute_hooks_for(self, :destroy) end + def remove_key_cache + Users::KeysCountService.new(self).delete_cache + end + def delete_async(deleted_by:, params: {}) block if params[:hard_delete] DeleteUserWorker.perform_async(deleted_by.id, id, params) diff --git a/app/services/base_count_service.rb b/app/services/base_count_service.rb new file mode 100644 index 00000000000..99cc9a196e6 --- /dev/null +++ b/app/services/base_count_service.rb @@ -0,0 +1,34 @@ +# Base class for services that count a single resource such as the number of +# issues for a project. +class BaseCountService + def relation_for_count + raise( + NotImplementedError, + '"relation_for_count" must be implemented and return an ActiveRecord::Relation' + ) + end + + def count + Rails.cache.fetch(cache_key, raw: raw?) { uncached_count }.to_i + end + + def refresh_cache + Rails.cache.write(cache_key, uncached_count, raw: raw?) + end + + def uncached_count + relation_for_count.count + end + + def delete_cache + Rails.cache.delete(cache_key) + end + + def raw? + false + end + + def cache_key + raise NotImplementedError, 'cache_key must be implemented and return a String' + end +end diff --git a/app/services/projects/count_service.rb b/app/services/projects/count_service.rb index aa034315280..7e575b2d6f3 100644 --- a/app/services/projects/count_service.rb +++ b/app/services/projects/count_service.rb @@ -1,7 +1,7 @@ module Projects # Base class for the various service classes that count project data (e.g. # issues or forks). - class CountService + class CountService < BaseCountService # The version of the cache format. This should be bumped whenever the # underlying logic changes. This removes the need for explicitly flushing # all caches. @@ -11,29 +11,6 @@ module Projects @project = project end - def relation_for_count - raise( - NotImplementedError, - '"relation_for_count" must be implemented and return an ActiveRecord::Relation' - ) - end - - def count - Rails.cache.fetch(cache_key) { uncached_count } - end - - def refresh_cache - Rails.cache.write(cache_key, uncached_count) - end - - def uncached_count - relation_for_count.count - end - - def delete_cache - Rails.cache.delete(cache_key) - end - def cache_key_name raise( NotImplementedError, diff --git a/app/services/projects/forks_count_service.rb b/app/services/projects/forks_count_service.rb index 3a0fa84b868..d9bdf3a8ad7 100644 --- a/app/services/projects/forks_count_service.rb +++ b/app/services/projects/forks_count_service.rb @@ -1,6 +1,6 @@ module Projects # Service class for getting and caching the number of forks of a project. - class ForksCountService < CountService + class ForksCountService < Projects::CountService def relation_for_count @project.forks end diff --git a/app/services/projects/open_issues_count_service.rb b/app/services/projects/open_issues_count_service.rb index 3c0d186a73c..25de97325e2 100644 --- a/app/services/projects/open_issues_count_service.rb +++ b/app/services/projects/open_issues_count_service.rb @@ -1,7 +1,7 @@ module Projects # Service class for counting and caching the number of open issues of a # project. - class OpenIssuesCountService < CountService + class OpenIssuesCountService < Projects::CountService def relation_for_count # We don't include confidential issues in this number since this would # expose the number of confidential issues to non project members. diff --git a/app/services/projects/open_merge_requests_count_service.rb b/app/services/projects/open_merge_requests_count_service.rb index 2a90f78b90d..77e6448fd5e 100644 --- a/app/services/projects/open_merge_requests_count_service.rb +++ b/app/services/projects/open_merge_requests_count_service.rb @@ -1,7 +1,7 @@ module Projects # Service class for counting and caching the number of open merge requests of # a project. - class OpenMergeRequestsCountService < CountService + class OpenMergeRequestsCountService < Projects::CountService def relation_for_count @project.merge_requests.opened end diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 5957f612e84..d3a8ae8d7c6 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -60,15 +60,8 @@ module Projects # Notifications project.send_move_instructions(@old_path) - # Move main repository - # TODO: check storage type and NOOP when not using Legacy - unless move_repo_folder(@old_path, @new_path) - raise TransferError.new('Cannot move project') - end - - # Move wiki repo also if present - # TODO: check storage type and NOOP when not using Legacy - move_repo_folder("#{@old_path}.wiki", "#{@new_path}.wiki") + # Directories on disk + move_project_folders(project) # Move missing group labels to project Labels::TransferService.new(current_user, @old_group, project).execute @@ -131,5 +124,20 @@ module Projects def execute_system_hooks SystemHooksService.new.execute_hooks_for(project, :transfer) end + + def move_project_folders(project) + return if project.hashed_storage?(:repository) + + # Move main repository + unless move_repo_folder(@old_path, @new_path) + raise TransferError.new("Cannot move project") + end + + # Disk path is changed; we need to ensure we reload it + project.reload_repository! + + # Move wiki repo also if present + move_repo_folder("#{@old_path}.wiki", "#{@new_path}.wiki") + end end end diff --git a/app/services/users/keys_count_service.rb b/app/services/users/keys_count_service.rb new file mode 100644 index 00000000000..f82d27eded9 --- /dev/null +++ b/app/services/users/keys_count_service.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Users + # Service class for getting the number of SSH keys that belong to a user. + class KeysCountService < BaseCountService + attr_reader :user + + # user - The User for which to get the number of SSH keys. + def initialize(user) + @user = user + end + + def relation_for_count + user.keys + end + + def raw? + # Since we're storing simple integers we don't need all of the additional + # Marshal data Rails includes by default. + true + end + + def cache_key + "users/key-count-service/#{user.id}" + end + end +end diff --git a/app/views/admin/runners/index.html.haml b/app/views/admin/runners/index.html.haml index 4965dffab9d..4f60be698e9 100644 --- a/app/views/admin/runners/index.html.haml +++ b/app/views/admin/runners/index.html.haml @@ -64,7 +64,7 @@ %th Projects %th Jobs %th Tags - %th Last contact + %th= link_to 'Last contact', admin_runners_path(params.slice(:search).merge(sort: 'contacted_asc')) %th - @runners.each do |runner| diff --git a/app/views/dashboard/projects/_blank_state_admin_welcome.html.haml b/app/views/dashboard/projects/_blank_state_admin_welcome.html.haml index 57544559824..573a4b93d67 100644 --- a/app/views/dashboard/projects/_blank_state_admin_welcome.html.haml +++ b/app/views/dashboard/projects/_blank_state_admin_welcome.html.haml @@ -1,33 +1,41 @@ -.blank-state - .blank-state-icon - = custom_icon("add_new_user", size: 50) - .blank-state-body - %h3.blank-state-title - Add user - %p.blank-state-text - Add your team members and others to GitLab. - = link_to new_admin_user_path, class: "btn btn-new" do - New user +.blank-state-row + = link_to new_project_path, class: "blank-state-link" do + .blank-state + .blank-state-icon + = custom_icon("add_new_project", size: 50) + .blank-state-body + %h3.blank-state-title + Create a project + %p.blank-state-text + Projects are where you store your code, access issues, wiki and other features of GitLab. -.blank-state - .blank-state-icon - = custom_icon("configure_server", size: 50) - .blank-state-body - %h3.blank-state-title - Configure GitLab - %p.blank-state-text - Make adjustments to how your GitLab instance is set up. - = link_to admin_root_path, class: "btn btn-new" do - Configure + - if current_user.can_create_group? + = link_to admin_root_path, class: "blank-state-link" do + .blank-state + .blank-state-icon + = custom_icon("add_new_group", size: 50) + .blank-state-body + %h3.blank-state-title + Create a group + %p.blank-state-text + Groups are a great way to organize projects and people. -- if current_user.can_create_group? - .blank-state - .blank-state-icon - = custom_icon("add_new_group", size: 50) - .blank-state-body - %h3.blank-state-title - Create a group - %p.blank-state-text - Groups are a great way to organize projects and people. - = link_to new_group_path, class: "btn btn-new" do - New group + = link_to new_admin_user_path, class: "blank-state-link" do + .blank-state + .blank-state-icon + = custom_icon("add_new_user", size: 50) + .blank-state-body + %h3.blank-state-title + Add people + %p.blank-state-text + Add your team members and others to GitLab. + + = link_to admin_root_path, class: "blank-state-link" do + .blank-state + .blank-state-icon + = custom_icon("configure_server", size: 50) + .blank-state-body + %h3.blank-state-title + Configure GitLab + %p.blank-state-text + Make adjustments to how your GitLab instance is set up. diff --git a/app/views/dashboard/projects/_blank_state_welcome.html.haml b/app/views/dashboard/projects/_blank_state_welcome.html.haml index a93a3415ee1..8d5bddbb288 100644 --- a/app/views/dashboard/projects/_blank_state_welcome.html.haml +++ b/app/views/dashboard/projects/_blank_state_welcome.html.haml @@ -1,48 +1,58 @@ - public_project_count = ProjectsFinder.new(current_user: current_user).execute.count -- if current_user.can_create_group? - .blank-state - .blank-state-icon - = custom_icon("add_new_group", size: 50) - .blank-state-body - %h3.blank-state-title - Create a group for several dependent projects. - %p.blank-state-text - Groups are the best way to manage projects and members. - = link_to new_group_path, class: "btn btn-new" do - New group +.blank-state-row + - if current_user.can_create_project? + = link_to new_project_path, class: "blank-state-link" do + .blank-state + .blank-state-icon + = custom_icon("add_new_project", size: 50) + .blank-state-body + %h3.blank-state-title + Create a project + %p.blank-state-text + Projects are where you store your code, access issues, wiki and other features of GitLab. + - else + .blank-state + .blank-state-icon + = custom_icon("add_new_project", size: 50) + .blank-state-body + %h3.blank-state-title + Create a project + %p.blank-state-text + If you are added to a project, it will be displayed here. -.blank-state - .blank-state-icon - = custom_icon("add_new_project", size: 50) - .blank-state-body - %h3.blank-state-title - Create a project - %p.blank-state-text - - if current_user.can_create_project? - You don't have access to any projects right now. - You can create up to - %strong= number_with_delimiter(current_user.projects_limit) - = succeed "." do - = "project".pluralize(current_user.projects_limit) - - else - If you are added to a project, it will be displayed here. - - if current_user.can_create_project? - = link_to new_project_path, class: "btn btn-new" do - New project + - if current_user.can_create_group? + = link_to new_group_path, class: "blank-state-link" do + .blank-state + .blank-state-icon + = custom_icon("add_new_group", size: 50) + .blank-state-body + %h3.blank-state-title + Create a group + %p.blank-state-text + Groups are the best way to manage projects and members. -- if public_project_count > 0 - .blank-state - .blank-state-icon - = custom_icon("globe", size: 50) - .blank-state-body - %h3.blank-state-title - Explore public projects - %p.blank-state-text - There are - = number_with_delimiter(public_project_count) - public projects on this server. - Public projects are an easy way to allow - everyone to have read-only access. - = link_to trending_explore_projects_path, class: "btn btn-new" do - Browse projects + - if public_project_count > 0 + = link_to trending_explore_projects_path, class: "blank-state-link" do + .blank-state + .blank-state-icon + = custom_icon("globe", size: 50) + .blank-state-body + %h3.blank-state-title + Explore public projects + %p.blank-state-text + There are + = number_with_delimiter(public_project_count) + public projects on this server. + Public projects are an easy way to allow + everyone to have read-only access. + + = link_to "https://docs.gitlab.com/", class: "blank-state-link" do + .blank-state + .blank-state-icon + = custom_icon("lightbulb", size: 50) + .blank-state-body + %h3.blank-state-title + Learn more about GitLab + %p.blank-state-text + Take a look at the documentation to discover all of GitLab's capabilities. diff --git a/app/views/dashboard/projects/_zero_authorized_projects.html.haml b/app/views/dashboard/projects/_zero_authorized_projects.html.haml index ad3fac6d164..18a82feb189 100644 --- a/app/views/dashboard/projects/_zero_authorized_projects.html.haml +++ b/app/views/dashboard/projects/_zero_authorized_projects.html.haml @@ -1,12 +1,13 @@ -.row.blank-state-parent-container +.blank-state-parent-container .section-container.section-welcome{ class: "#{ 'section-admin-welcome' if current_user.admin? }" } .container.section-body - .blank-state.blank-state-welcome - %h2.blank-state-welcome-title - Welcome to GitLab - %p.blank-state-text - Code, test, and deploy together - - if current_user.admin? - = render "blank_state_admin_welcome" - - else - = render "blank_state_welcome" + .row + .blank-state-welcome + %h2.blank-state-welcome-title + Welcome to GitLab + %p.blank-state-text + Code, test, and deploy together + - if current_user.admin? + = render "blank_state_admin_welcome" + - else + = render "blank_state_welcome" diff --git a/app/views/projects/commit/_ajax_signature.html.haml b/app/views/projects/commit/_ajax_signature.html.haml index 1d6a0fa38ca..36b28c731a1 100644 --- a/app/views/projects/commit/_ajax_signature.html.haml +++ b/app/views/projects/commit/_ajax_signature.html.haml @@ -1,2 +1,2 @@ - if commit.has_signature? - %a{ href: '#', tabindex: 0, class: commit_signature_badge_classes('js-loading-gpg-badge'), data: { toggle: 'tooltip', placement: 'auto top', title: 'GPG signature (loading...)', 'commit-sha' => commit.sha } } + %a{ href: 'javascript:void(0)', tabindex: 0, class: commit_signature_badge_classes('js-loading-gpg-badge'), data: { toggle: 'tooltip', placement: 'auto top', title: 'GPG signature (loading...)', 'commit-sha' => commit.sha } } diff --git a/app/views/projects/commit/_signature_badge.html.haml b/app/views/projects/commit/_signature_badge.html.haml index b6b7aae6f9a..44aa8002f12 100644 --- a/app/views/projects/commit/_signature_badge.html.haml +++ b/app/views/projects/commit/_signature_badge.html.haml @@ -24,5 +24,5 @@ = link_to('Learn more about signing commits', help_page_path('user/project/repository/gpg_signed_commits/index.md'), class: 'gpg-popover-help-link') -%a{ href: '#', tabindex: 0, class: css_classes, data: { toggle: 'popover', html: 'true', placement: 'auto top', title: title, content: content } } +%a{ href: 'javascript:void(0)', tabindex: 0, class: css_classes, data: { toggle: 'popover', html: 'true', placement: 'auto top', title: title, content: content } } = label diff --git a/app/views/projects/tree/_truncated_notice_tree_row.html.haml b/app/views/projects/tree/_truncated_notice_tree_row.html.haml new file mode 100644 index 00000000000..693b641888b --- /dev/null +++ b/app/views/projects/tree/_truncated_notice_tree_row.html.haml @@ -0,0 +1,7 @@ +%tr.tree-truncated-warning + %td{ colspan: '3' } + = icon('exclamation-triangle fw') + %span + Too many items to show. To preserve performance only + %strong #{number_with_delimiter(limit)} of #{number_with_delimiter(total)} + items are displayed. diff --git a/app/views/shared/icons/_add_new_project.svg b/app/views/shared/icons/_add_new_project.svg index 3c1e15453df..cf8762944ca 100644 --- a/app/views/shared/icons/_add_new_project.svg +++ b/app/views/shared/icons/_add_new_project.svg @@ -1 +1 @@ -<svg xmlns="http://www.w3.org/2000/svg" width="78" height="82" viewBox="0 0 78 82"><g fill="none" fill-rule="evenodd"><path fill="#F9F9F9" d="M2.12 42c-.08.99-.12 1.99-.12 3 0 20.435 16.565 37 37 37s37-16.565 37-37c0-1.01-.04-2.01-.12-3C74.353 61.032 58.425 76 39 76 19.575 76 3.647 61.032 2.12 42z"/><path fill="#EEE" fill-rule="nonzero" d="M39 78C17.46 78 0 60.54 0 39S17.46 0 39 0s39 17.46 39 39-17.46 39-39 39zm0-4c19.33 0 35-15.67 35-35S58.33 4 39 4 4 19.67 4 39s15.67 35 35 35z"/><path fill="#FEE1D3" fill-rule="nonzero" d="M30 24a4 4 0 0 0-4 4v22a4 4 0 0 0 4 4h18a4 4 0 0 0 4-4V28a4 4 0 0 0-4-4H30zm0-4h18a8 8 0 0 1 8 8v22a8 8 0 0 1-8 8H30a8 8 0 0 1-8-8V28a8 8 0 0 1 8-8z"/><path fill="#FC6D26" d="M33 30h8a2 2 0 1 1 0 4h-8a2 2 0 1 1 0-4zm0 7h12a2 2 0 1 1 0 4H33a2 2 0 1 1 0-4zm0 7h12a2 2 0 1 1 0 4H33a2 2 0 1 1 0-4z"/></g></svg>
\ No newline at end of file +<svg xmlns="http://www.w3.org/2000/svg" width="78" height="82" viewBox="0 0 78 82"><g fill="none" fill-rule="evenodd"><path fill="#F9F9F9" d="M2.12 42c-.08.99-.12 1.99-.12 3 0 20.435 16.565 37 37 37s37-16.565 37-37c0-1.01-.04-2.01-.12-3C74.353 61.032 58.425 76 39 76S3.647 61.032 2.12 42z"/><path fill="#EEE" fill-rule="nonzero" d="M39 78C17.46 78 0 60.54 0 39S17.46 0 39 0s39 17.46 39 39-17.46 39-39 39zm0-4c19.33 0 35-15.67 35-35S58.33 4 39 4 4 19.67 4 39s15.67 35 35 35z"/><path fill="#E1DBF2" fill-rule="nonzero" d="M30 24c-2.21 0-4 1.79-4 4v22c0 2.21 1.79 4 4 4h18c2.21 0 4-1.79 4-4V28c0-2.21-1.79-4-4-4H30zm0-4h18c4.418 0 8 3.582 8 8v22c0 4.418-3.582 8-8 8H30c-4.418 0-8-3.582-8-8V28c0-4.418 3.582-8 8-8z"/><path fill="#6B4FBB" d="M33 30h8c1.105 0 2 .895 2 2s-.895 2-2 2h-8c-1.105 0-2-.895-2-2s.895-2 2-2zm0 7h12c1.105 0 2 .895 2 2s-.895 2-2 2H33c-1.105 0-2-.895-2-2s.895-2 2-2zm0 7h12c1.105 0 2 .895 2 2s-.895 2-2 2H33c-1.105 0-2-.895-2-2s.895-2 2-2z"/></g></svg> diff --git a/app/views/shared/icons/_lightbulb.svg b/app/views/shared/icons/_lightbulb.svg new file mode 100644 index 00000000000..2fcc4c65f99 --- /dev/null +++ b/app/views/shared/icons/_lightbulb.svg @@ -0,0 +1 @@ +<svg xmlns="http://www.w3.org/2000/svg" width="78" height="82" viewBox="0 0 78 82"><g fill="none" fill-rule="evenodd"><path fill="#F9F9F9" d="M2.12 42c-.08.99-.12 1.99-.12 3 0 20.435 16.565 37 37 37s37-16.565 37-37c0-1.01-.04-2.01-.12-3C74.353 61.032 58.425 76 39 76S3.647 61.032 2.12 42z"/><path fill="#EEE" fill-rule="nonzero" d="M39 78C17.46 78 0 60.54 0 39S17.46 0 39 0s39 17.46 39 39-17.46 39-39 39zm0-4c19.33 0 35-15.67 35-35S58.33 4 39 4 4 19.67 4 39s15.67 35 35 35z"/><path fill="#6B4FBB" d="M33 52h12c1.105 0 2 .895 2 2s-.895 2-2 2H33c-1.105 0-2-.895-2-2s.895-2 2-2zm1 5h10c1.105 0 2 .895 2 2s-.895 2-2 2H34c-1.105 0-2-.895-2-2s.895-2 2-2z"/><path fill="#E1DBF2" fill-rule="nonzero" d="M45.542 46.932l.346-2.36c.198-1.348.737-2.623 1.566-3.705 3.025-3.946 4.485-7.29 4.547-9.96C52.153 24.41 46.843 20 39 20c-7.777 0-13 4.374-13 11 0 2.4 1.462 5.73 4.573 9.846.815 1.08 1.343 2.345 1.536 3.683l.353 2.456 13.08-.054zm-17.038.624L28.15 45.1c-.097-.67-.36-1.303-.768-1.842C23.794 38.51 22 34.424 22 31c0-9.39 7.61-15 17-15s17.218 5.614 17 15c-.085 3.64-1.875 7.74-5.37 12.3-.416.54-.685 1.18-.784 1.853l-.346 2.36c-.288 1.958-1.963 3.41-3.942 3.42l-13.08.053c-1.994.008-3.69-1.455-3.974-3.43z"/><path fill="#6B4FBB" d="M41 38.732c-.598-.345-1-.992-1-1.732 0-1.105.895-2 2-2s2 .895 2 2c0 .74-.402 1.387-1 1.732V42c0 .552-.448 1-1 1s-1-.448-1-1v-3.268zm-6 0c-.598-.345-1-.992-1-1.732 0-1.105.895-2 2-2s2 .895 2 2c0 .74-.402 1.387-1 1.732V42c0 .552-.448 1-1 1s-1-.448-1-1v-3.268z"/></g></svg> diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index 951b4dd7b36..2c27dd638a7 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -104,7 +104,6 @@ class: 'btn btn-remove prepend-left-10' - else = link_to member, - remote: true, method: :delete, data: { confirm: remove_member_message(member) }, class: 'btn btn-remove prepend-left-10', diff --git a/changelogs/unreleased/3615-improve-welcome-screen.yml b/changelogs/unreleased/3615-improve-welcome-screen.yml new file mode 100644 index 00000000000..862efddb162 --- /dev/null +++ b/changelogs/unreleased/3615-improve-welcome-screen.yml @@ -0,0 +1,5 @@ +--- +title: Reorganize welcome page for new users +merge_request: +author: +type: other diff --git a/changelogs/unreleased/39573-hashed-storage-backup.yml b/changelogs/unreleased/39573-hashed-storage-backup.yml new file mode 100644 index 00000000000..40ee589c8cc --- /dev/null +++ b/changelogs/unreleased/39573-hashed-storage-backup.yml @@ -0,0 +1,5 @@ +--- +title: Fix gitlab:backup rake for hashed storage based repositories +merge_request: 15400 +author: +type: fixed diff --git a/changelogs/unreleased/40068-runner-sorting-regression.yml b/changelogs/unreleased/40068-runner-sorting-regression.yml new file mode 100644 index 00000000000..6a2bd59d6d6 --- /dev/null +++ b/changelogs/unreleased/40068-runner-sorting-regression.yml @@ -0,0 +1,5 @@ +--- +title: Revert a regression on runners sorting (!15134) +merge_request: 15341 +author: Takuya Noguchi +type: fixed diff --git a/changelogs/unreleased/40092-fix-cluster-size.yml b/changelogs/unreleased/40092-fix-cluster-size.yml new file mode 100644 index 00000000000..e7a16c6920c --- /dev/null +++ b/changelogs/unreleased/40092-fix-cluster-size.yml @@ -0,0 +1,5 @@ +--- +title: Formats bytes to human reabale number in registry table +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/40198-fix-gpg-badge-links.yml b/changelogs/unreleased/40198-fix-gpg-badge-links.yml new file mode 100644 index 00000000000..62b962acefa --- /dev/null +++ b/changelogs/unreleased/40198-fix-gpg-badge-links.yml @@ -0,0 +1,6 @@ +--- +title: Fix issue where clicking a GPG verification badge would scroll to the top of + the page +merge_request: 15407 +author: +type: fixed diff --git a/changelogs/unreleased/bvl-fork-network-memberships-for-deleted-source.yml b/changelogs/unreleased/bvl-fork-network-memberships-for-deleted-source.yml new file mode 100644 index 00000000000..ae1eb3aedd5 --- /dev/null +++ b/changelogs/unreleased/bvl-fork-network-memberships-for-deleted-source.yml @@ -0,0 +1,5 @@ +--- +title: Don't try to create fork network memberships for forks with a missing source +merge_request: 15366 +author: +type: fixed diff --git a/changelogs/unreleased/bvl-refresh-member-listing-on-removal.yml b/changelogs/unreleased/bvl-refresh-member-listing-on-removal.yml new file mode 100644 index 00000000000..48b4051711c --- /dev/null +++ b/changelogs/unreleased/bvl-refresh-member-listing-on-removal.yml @@ -0,0 +1,5 @@ +--- +title: Don't use JS to delete memberships from projects and groups +merge_request: 15344 +author: +type: fixed diff --git a/changelogs/unreleased/cache-user-keys-count.yml b/changelogs/unreleased/cache-user-keys-count.yml new file mode 100644 index 00000000000..181be95622c --- /dev/null +++ b/changelogs/unreleased/cache-user-keys-count.yml @@ -0,0 +1,5 @@ +--- +title: Cache the number of user SSH keys +merge_request: +author: +type: performance diff --git a/changelogs/unreleased/ci-pipeline-status-query.yml b/changelogs/unreleased/ci-pipeline-status-query.yml new file mode 100644 index 00000000000..a464e501418 --- /dev/null +++ b/changelogs/unreleased/ci-pipeline-status-query.yml @@ -0,0 +1,5 @@ +--- +title: Optimise getting the pipeline status of commits +merge_request: +author: +type: performance diff --git a/changelogs/unreleased/dm-avatarable-with-asset-host.yml b/changelogs/unreleased/dm-avatarable-with-asset-host.yml new file mode 100644 index 00000000000..6cf8d719afb --- /dev/null +++ b/changelogs/unreleased/dm-avatarable-with-asset-host.yml @@ -0,0 +1,6 @@ +--- +title: Always return full avatar URL for private/internal groups/projects when asset + host is set +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/dm-notes-actions-noteable-for-update.yml b/changelogs/unreleased/dm-notes-actions-noteable-for-update.yml new file mode 100644 index 00000000000..1d2f58bc765 --- /dev/null +++ b/changelogs/unreleased/dm-notes-actions-noteable-for-update.yml @@ -0,0 +1,5 @@ +--- +title: Make sure NotesActions#noteable returns a Noteable in the update action +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/fix-gb-update-registry-path-reference-regexp.yml b/changelogs/unreleased/fix-gb-update-registry-path-reference-regexp.yml new file mode 100644 index 00000000000..55c1089ade5 --- /dev/null +++ b/changelogs/unreleased/fix-gb-update-registry-path-reference-regexp.yml @@ -0,0 +1,5 @@ +--- +title: Update container repository path reference and allow using double underscore +merge_request: 15417 +author: +type: fixed diff --git a/changelogs/unreleased/issue_40058.yml b/changelogs/unreleased/issue_40058.yml new file mode 100644 index 00000000000..46e83d947ba --- /dev/null +++ b/changelogs/unreleased/issue_40058.yml @@ -0,0 +1,5 @@ +--- +title: Prevents position update for image diff notes +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/sh-port-hashed-storage-transfer-fix.yml b/changelogs/unreleased/sh-port-hashed-storage-transfer-fix.yml new file mode 100644 index 00000000000..c32afc90f64 --- /dev/null +++ b/changelogs/unreleased/sh-port-hashed-storage-transfer-fix.yml @@ -0,0 +1,5 @@ +--- +title: Fix hashed storage with project transfers to another namespace +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/tree_item_limit.yml b/changelogs/unreleased/tree_item_limit.yml new file mode 100644 index 00000000000..d95c5776075 --- /dev/null +++ b/changelogs/unreleased/tree_item_limit.yml @@ -0,0 +1,5 @@ +--- +title: Truncate tree to max 1,000 items and display notice to users +merge_request: +author: +type: performance diff --git a/config/prometheus/additional_metrics.yml b/config/prometheus/additional_metrics.yml index 190eeb59a2c..601a86490d4 100644 --- a/config/prometheus/additional_metrics.yml +++ b/config/prometheus/additional_metrics.yml @@ -145,7 +145,7 @@ - container_memory_usage_bytes weight: 1 queries: - - query_range: '(sum(container_memory_usage_bytes{container_name!="POD",environment="%{ci_environment_slug}"}) / count(container_memory_usage_bytes{container_name!="POD",environment="%{ci_environment_slug}"})) /1024/1024' + - query_range: '(sum(avg(container_memory_usage_bytes{container_name!="POD",environment="%{ci_environment_slug}"}) without (job))) / count(avg(container_memory_usage_bytes{container_name!="POD",environment="%{ci_environment_slug}"}) without (job)) /1024/1024' label: Average unit: MB - title: "CPU Utilization" @@ -154,8 +154,6 @@ - container_cpu_usage_seconds_total weight: 1 queries: - - query_range: 'sum(rate(container_cpu_usage_seconds_total{container_name!="POD",environment="%{ci_environment_slug}"}[2m])) * 100' - label: CPU - unit: "%" - series: - - label: cpu + - query_range: 'sum(avg(rate(container_cpu_usage_seconds_total{container_name!="POD",environment="%{ci_environment_slug}"}[2m])) without (job)) * 100' + label: Average + unit: "%"
\ No newline at end of file diff --git a/db/post_migrate/20171026082505_populate_merge_requests_latest_merge_request_diff_id.rb b/db/post_migrate/20171026082505_populate_merge_requests_latest_merge_request_diff_id.rb deleted file mode 100644 index a7ebbbf34c0..00000000000 --- a/db/post_migrate/20171026082505_populate_merge_requests_latest_merge_request_diff_id.rb +++ /dev/null @@ -1,27 +0,0 @@ -class PopulateMergeRequestsLatestMergeRequestDiffId < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - BATCH_SIZE = 1_000 - - class MergeRequest < ActiveRecord::Base - self.table_name = 'merge_requests' - - include ::EachBatch - end - - disable_ddl_transaction! - - def up - update = ' - latest_merge_request_diff_id = ( - SELECT MAX(id) - FROM merge_request_diffs - WHERE merge_requests.id = merge_request_diffs.merge_request_id - )'.squish - - MergeRequest.where(latest_merge_request_diff_id: nil).each_batch(of: BATCH_SIZE) do |relation| - relation.update_all(update) - end - end -end diff --git a/db/post_migrate/20171026082505_schedule_merge_request_latest_merge_request_diff_id_migrations.rb b/db/post_migrate/20171026082505_schedule_merge_request_latest_merge_request_diff_id_migrations.rb new file mode 100644 index 00000000000..7a63382cc6d --- /dev/null +++ b/db/post_migrate/20171026082505_schedule_merge_request_latest_merge_request_diff_id_migrations.rb @@ -0,0 +1,29 @@ +class ScheduleMergeRequestLatestMergeRequestDiffIdMigrations < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + BATCH_SIZE = 50_000 + MIGRATION = 'PopulateMergeRequestsLatestMergeRequestDiffId' + + disable_ddl_transaction! + + class MergeRequest < ActiveRecord::Base + self.table_name = 'merge_requests' + + include ::EachBatch + end + + # On GitLab.com, we saw that we generated about 500,000 dead tuples over 5 minutes. + # To keep replication lag from ballooning, we'll aim for 50,000 updates over 5 minutes. + # + # Assuming that there are 5 million rows affected (which is more than on + # GitLab.com), and that each batch of 50,000 rows takes up to 5 minutes, then + # we can migrate all the rows in 8.5 hours. + def up + MergeRequest.where(latest_merge_request_diff_id: nil).each_batch(of: BATCH_SIZE) do |relation, index| + range = relation.pluck('MIN(id)', 'MAX(id)').first + + BackgroundMigrationWorker.perform_in(index * 5.minutes, MIGRATION, range) + end + end +end diff --git a/doc/user/profile/preferences.md b/doc/user/profile/preferences.md index f2ad42f21fd..022d6317555 100644 --- a/doc/user/profile/preferences.md +++ b/doc/user/profile/preferences.md @@ -55,9 +55,10 @@ You have 6 options here that you can use for your default dashboard view: The project home page content setting allows you to choose what content you want to see on a project’s home page. -You can choose between 2 options: +You can choose between 3 options: - Show the files and the readme (default) +- Show the readme - Show the project’s activity [rouge]: http://rouge.jneen.net/ "Rouge website" diff --git a/doc/user/project/clusters/img/cluster-applications.png b/doc/user/project/clusters/img/cluster-applications.png Binary files differdeleted file mode 100644 index 7c82d19297e..00000000000 --- a/doc/user/project/clusters/img/cluster-applications.png +++ /dev/null diff --git a/doc/user/project/clusters/index.md b/doc/user/project/clusters/index.md index 27b4b49c207..cf0c7c109a8 100644 --- a/doc/user/project/clusters/index.md +++ b/doc/user/project/clusters/index.md @@ -1,14 +1,15 @@ -# Connecting GitLab with GKE +# Connecting GitLab with a Kubernetes cluster > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/35954) in 10.1. CAUTION: **Warning:** The Cluster integration is currently in **Beta**. -Connect your project to Google Container Engine (GKE) in a few steps. - With a cluster associated to your project, you can use Review Apps, deploy your -applications, run your pipelines, and much more in an easy way. +applications, run your pipelines, and much more, in an easy way. + +Connect your project to Google Kubernetes Engine (GKE) or your own Kubernetes +cluster in a few steps. NOTE: **Note:** The Cluster integration will eventually supersede the @@ -30,36 +31,58 @@ prerequisites must be met: - You must have Master [permissions] in order to be able to access the **Cluster** page. -If all of the above requirements are met, you can proceed to add a new cluster. +If all of the above requirements are met, you can proceed to add a new GKE +cluster. ## Adding a cluster NOTE: **Note:** You need Master [permissions] and above to add a cluster. +There are two options when adding a new cluster; either use Google Kubernetes +Engine (GKE) or provide the credentials to your own Kubernetes cluster. + To add a new cluster: -1. Navigate to your project's **CI/CD > Cluster** page. -1. Connect your Google account if you haven't done already by clicking the - "Sign-in with Google" button. -1. Fill in the requested values: - - **Cluster name** (required) - The name you wish to give the cluster. - - **GCP project ID** (required) - The ID of the project you created in your GCP - console that will host the Kubernetes cluster. This must **not** be confused - with the project name. Learn more about [Google Cloud Platform projects](https://cloud.google.com/resource-manager/docs/creating-managing-projects). - - **Zone** - The zone under which the cluster will be created. Read more about - [the available zones](https://cloud.google.com/compute/docs/regions-zones/). - - **Number of nodes** - The number of nodes you wish the cluster to have. - - **Machine type** - The machine type of the Virtual Machine instance that - the cluster will be based on. Read more about [the available machine types](https://cloud.google.com/compute/docs/machine-types). - - **Project namespace** - The unique namespace for this project. By default you - don't have to fill it in; by leaving it blank, GitLab will create one for you. -1. Click the **Create cluster** button. - -After a few moments your cluster should be created. If something goes wrong, +1. Navigate to your project's **CI/CD > Cluster** page +1. If you want to let GitLab create a cluster on GKE for you, go through the + following steps, otherwise skip to the next one. + 1. Click on **Create with GKE** + 1. Connect your Google account if you haven't done already by clicking the + **Sign in with Google** button + 1. Fill in the requested values: + - **Cluster name** (required) - The name you wish to give the cluster. + - **GCP project ID** (required) - The ID of the project you created in your GCP + console that will host the Kubernetes cluster. This must **not** be confused + with the project name. Learn more about [Google Cloud Platform projects](https://cloud.google.com/resource-manager/docs/creating-managing-projects). + - **Zone** - The [zone](https://cloud.google.com/compute/docs/regions-zones/) + under which the cluster will be created. + - **Number of nodes** - The number of nodes you wish the cluster to have. + - **Machine type** - The [machine type](https://cloud.google.com/compute/docs/machine-types) + of the Virtual Machine instance that the cluster will be based on. + - **Project namespace** - The unique namespace for this project. By default you + don't have to fill it in; by leaving it blank, GitLab will create one for you. +1. If you want to use your own existing Kubernetes cluster, click on + **Add an existing cluster** and fill in the details as described in the + [Kubernetes integration](../integrations/kubernetes.md) documentation. +1. Finally, click the **Create cluster** button + +After a few moments, your cluster should be created. If something goes wrong, you will be notified. -Now, you can proceed to [enable the Cluster integration](#enabling-or-disabling-the-cluster-integration). +You can now proceed to install some pre-defined applications and then +enable the Cluster integration. + +## Installing applications + +GitLab provides a one-click install for various applications which will be +added directly to your configured cluster. Those applications are needed for +[Review Apps](../../../ci/review_apps/index.md) and [deployments](../../../ci/environments.md). + +| Application | GitLab version | Description | +| ----------- | :------------: | ----------- | +| [Helm Tiller](https://docs.helm.sh/) | 10.2+ | Helm is a package manager for Kubernetes and is required to install all the other applications. It will be automatically installed as a dependency when you try to install a different app. It is installed in its own pod inside the cluster which can run the `helm` CLI in a safe environment. | +| [Ingress](https://kubernetes.io/docs/concepts/services-networking/ingress/) | 10.2+ | Ingress can provide load balancing, SSL termination and name-based virtual hosting. It acts as a web proxy for your applications and is useful if you want to use [Auto DevOps](../../../topics/autodevops/index.md) or deploy your own web apps. | ## Enabling or disabling the Cluster integration @@ -88,12 +111,3 @@ To remove the Cluster integration from your project, simply click on the and [add a cluster](#adding-a-cluster) again. [permissions]: ../../permissions.md - -## Installing applications - -GitLab provides a one-click install for -[Helm Tiller](https://docs.helm.sh/) and -[Ingress](https://kubernetes.io/docs/concepts/services-networking/ingress/) -which will be added directly to your configured cluster. - - diff --git a/doc/user/project/integrations/prometheus_library/kubernetes.md b/doc/user/project/integrations/prometheus_library/kubernetes.md index 518683965e8..a6673fa2a00 100644 --- a/doc/user/project/integrations/prometheus_library/kubernetes.md +++ b/doc/user/project/integrations/prometheus_library/kubernetes.md @@ -13,8 +13,8 @@ integration services must be enabled. | Name | Query | | ---- | ----- | -| Average Memory Usage (MB) | (sum(container_memory_usage_bytes{container_name!="POD",%{environment_filter}}) / count(container_memory_usage_bytes{container_name!="POD",%{environment_filter}})) /1024/1024 | -| Average CPU Utilization (%) | sum(rate(container_cpu_usage_seconds_total{container_name!="POD",%{environment_filter}}[2m])) by (cpu) * 100 | +| Average Memory Usage (MB) | (sum(avg(container_memory_usage_bytes{container_name!="POD",environment="%{ci_environment_slug}"}) without (job))) / count(avg(container_memory_usage_bytes{container_name!="POD",environment="%{ci_environment_slug}"}) without (job)) /1024/1024 | +| Average CPU Utilization (%) | sum(avg(rate(container_cpu_usage_seconds_total{container_name!="POD",environment="%{ci_environment_slug}"}[2m])) without (job)) * 100 | ## Configuring Prometheus to monitor for Kubernetes node metrics diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb index 4c0db4d42b1..4b3c473b0bb 100644 --- a/lib/api/helpers/internal_helpers.rb +++ b/lib/api/helpers/internal_helpers.rb @@ -36,6 +36,18 @@ module API {} end + def fix_git_env_repository_paths(env, repository_path) + if obj_dir_relative = env['GIT_OBJECT_DIRECTORY_RELATIVE'].presence + env['GIT_OBJECT_DIRECTORY'] = File.join(repository_path, obj_dir_relative) + end + + if alt_obj_dirs_relative = env['GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE'].presence + env['GIT_ALTERNATE_OBJECT_DIRECTORIES'] = alt_obj_dirs_relative.map { |dir| File.join(repository_path, dir) } + end + + env + end + def log_user_activity(actor) commands = Gitlab::GitAccess::DOWNLOAD_COMMANDS diff --git a/lib/api/internal.rb b/lib/api/internal.rb index 6e78ac2c903..451121a4cea 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -19,7 +19,9 @@ module API status 200 # Stores some Git-specific env thread-safely - Gitlab::Git::Env.set(parse_env) + env = parse_env + env = fix_git_env_repository_paths(env, repository_path) if project + Gitlab::Git::Env.set(env) actor = if params[:key_id] diff --git a/lib/backup/repository.rb b/lib/backup/repository.rb index 3ad09a1b421..b6d273b98c2 100644 --- a/lib/backup/repository.rb +++ b/lib/backup/repository.rb @@ -7,12 +7,16 @@ module Backup prepare Project.find_each(batch_size: 1000) do |project| - progress.print " * #{project.full_path} ... " + progress.print " * #{display_repo_path(project)} ... " path_to_project_repo = path_to_repo(project) path_to_project_bundle = path_to_bundle(project) - # Create namespace dir if missing - FileUtils.mkdir_p(File.join(backup_repos_path, project.namespace.full_path)) if project.namespace + # Create namespace dir or hashed path if missing + if project.hashed_storage?(:repository) + FileUtils.mkdir_p(File.dirname(File.join(backup_repos_path, project.disk_path))) + else + FileUtils.mkdir_p(File.join(backup_repos_path, project.namespace.full_path)) if project.namespace + end if empty_repo?(project) progress.puts "[SKIPPED]".color(:cyan) @@ -42,7 +46,7 @@ module Backup path_to_wiki_bundle = path_to_bundle(wiki) if File.exist?(path_to_wiki_repo) - progress.print " * #{wiki.full_path} ... " + progress.print " * #{display_repo_path(wiki)} ... " if empty_repo?(wiki) progress.puts " [SKIPPED]".color(:cyan) else @@ -71,7 +75,7 @@ module Backup end Project.find_each(batch_size: 1000) do |project| - progress.print " * #{project.full_path} ... " + progress.print " * #{display_repo_path(project)} ... " path_to_project_repo = path_to_repo(project) path_to_project_bundle = path_to_bundle(project) @@ -104,7 +108,7 @@ module Backup path_to_wiki_bundle = path_to_bundle(wiki) if File.exist?(path_to_wiki_bundle) - progress.print " * #{wiki.full_path} ... " + progress.print " * #{display_repo_path(wiki)} ... " # If a wiki bundle exists, first remove the empty repo # that was initialized with ProjectWiki.new() and then @@ -185,14 +189,14 @@ module Backup def progress_warn(project, cmd, output) progress.puts "[WARNING] Executing #{cmd}".color(:orange) - progress.puts "Ignoring error on #{project.full_path} - #{output}".color(:orange) + progress.puts "Ignoring error on #{display_repo_path(project)} - #{output}".color(:orange) end def empty_repo?(project_or_wiki) project_or_wiki.repository.expire_exists_cache # protect backups from stale cache project_or_wiki.repository.empty_repo? rescue => e - progress.puts "Ignoring repository error and continuing backing up project: #{project_or_wiki.full_path} - #{e.message}".color(:orange) + progress.puts "Ignoring repository error and continuing backing up project: #{display_repo_path(project_or_wiki)} - #{e.message}".color(:orange) false end @@ -204,5 +208,9 @@ module Backup def progress $progress end + + def display_repo_path(project) + project.hashed_storage?(:repository) ? "#{project.full_path} (#{project.disk_path})" : project.full_path + end end end diff --git a/lib/gitlab/background_migration/create_fork_network_memberships_range.rb b/lib/gitlab/background_migration/create_fork_network_memberships_range.rb index c88eb9783ed..67a39d28944 100644 --- a/lib/gitlab/background_migration/create_fork_network_memberships_range.rb +++ b/lib/gitlab/background_migration/create_fork_network_memberships_range.rb @@ -51,10 +51,20 @@ module Gitlab FROM projects WHERE forked_project_links.forked_from_project_id = projects.id ) + AND NOT EXISTS ( + SELECT true + FROM forked_project_links AS parent_links + WHERE parent_links.forked_to_project_id = forked_project_links.forked_from_project_id + AND NOT EXISTS ( + SELECT true + FROM projects + WHERE parent_links.forked_from_project_id = projects.id + ) + ) AND forked_project_links.id BETWEEN #{start_id} AND #{end_id} MISSING_MEMBERS - ForkNetworkMember.count_by_sql(count_sql) > 0 + ForkedProjectLink.count_by_sql(count_sql) > 0 end def log(message) diff --git a/lib/gitlab/background_migration/populate_merge_requests_latest_merge_request_diff_id.rb b/lib/gitlab/background_migration/populate_merge_requests_latest_merge_request_diff_id.rb new file mode 100644 index 00000000000..7e109e96e73 --- /dev/null +++ b/lib/gitlab/background_migration/populate_merge_requests_latest_merge_request_diff_id.rb @@ -0,0 +1,30 @@ +module Gitlab + module BackgroundMigration + class PopulateMergeRequestsLatestMergeRequestDiffId + BATCH_SIZE = 1_000 + + class MergeRequest < ActiveRecord::Base + self.table_name = 'merge_requests' + + include ::EachBatch + end + + def perform(start_id, stop_id) + update = ' + latest_merge_request_diff_id = ( + SELECT MAX(id) + FROM merge_request_diffs + WHERE merge_requests.id = merge_request_diffs.merge_request_id + )'.squish + + MergeRequest + .where(id: start_id..stop_id) + .where(latest_merge_request_diff_id: nil) + .each_batch(of: BATCH_SIZE) do |relation| + + relation.update_all(update) + end + end + end + end +end diff --git a/lib/gitlab/checks/lfs_integrity.rb b/lib/gitlab/checks/lfs_integrity.rb index 27a95764dc1..f7276a380dc 100644 --- a/lib/gitlab/checks/lfs_integrity.rb +++ b/lib/gitlab/checks/lfs_integrity.rb @@ -15,7 +15,10 @@ module Gitlab return false unless new_lfs_pointers.present? - existing_count = @project.lfs_objects.where(oid: new_lfs_pointers.map(&:lfs_oid)).count + existing_count = @project.lfs_storage_project + .lfs_objects + .where(oid: new_lfs_pointers.map(&:lfs_oid)) + .count existing_count != new_lfs_pointers.count end diff --git a/lib/gitlab/middleware/go.rb b/lib/gitlab/middleware/go.rb index cfc6b2a2029..1edda50e4b0 100644 --- a/lib/gitlab/middleware/go.rb +++ b/lib/gitlab/middleware/go.rb @@ -42,12 +42,11 @@ module Gitlab project_url = URI.join(config.gitlab.url, path) import_prefix = strip_url(project_url.to_s) - repository_url = case current_application_settings.enabled_git_access_protocol - when 'ssh' + repository_url = if current_application_settings.enabled_git_access_protocol == 'ssh' shell = config.gitlab_shell port = ":#{shell.ssh_port}" unless shell.ssh_port == 22 "ssh://#{shell.ssh_user}@#{shell.ssh_host}#{port}/#{path}.git" - when 'http', nil + else "#{project_url}.git" end diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index bd677ec4bf3..2c7b8af83f2 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -25,7 +25,7 @@ module Gitlab # See https://github.com/docker/distribution/blob/master/reference/regexp.go. # def container_repository_name_regex - @container_repository_regex ||= %r{\A[a-z0-9]+(?:[-._/][a-z0-9]+)*\Z} + @container_repository_regex ||= %r{\A[a-z0-9]+((?:[._/]|__|[-])[a-z0-9]+)*\Z} end ## diff --git a/lib/gitlab/routing.rb b/lib/gitlab/routing.rb index 910533076b0..2c994536060 100644 --- a/lib/gitlab/routing.rb +++ b/lib/gitlab/routing.rb @@ -46,10 +46,10 @@ module Gitlab # Only replace the last occurence of `path`. # # `request.fullpath` includes the querystring - path = request.path.sub(%r{/#{path}/*(?!.*#{path})}, "/-/#{path}/") - path << "?#{request.query_string}" if request.query_string.present? + new_path = request.path.sub(%r{/#{path}(/*)(?!.*#{path})}, "/-/#{path}\\1") + new_path << "?#{request.query_string}" if request.query_string.present? - path + new_path end paths.each do |path| diff --git a/qa/qa/specs/config.rb b/qa/qa/specs/config.rb index 79c681168cc..7e5fbe6cdde 100644 --- a/qa/qa/specs/config.rb +++ b/qa/qa/specs/config.rb @@ -59,7 +59,7 @@ module QA config.app_host = @address config.default_driver = :chrome config.javascript_driver = :chrome - config.default_max_wait_time = 4 + config.default_max_wait_time = 10 # https://github.com/mattheworiordan/capybara-screenshot/issues/164 config.save_path = 'tmp' diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 5f5a789d5cc..37e9f863fc4 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -336,6 +336,29 @@ describe Projects::NotesController do end end + describe 'PUT update' do + let(:request_params) do + { + namespace_id: project.namespace, + project_id: project, + id: note, + format: :json, + note: { + note: "New comment" + } + } + end + + before do + sign_in(note.author) + project.team << [note.author, :developer] + end + + it "updates the note" do + expect { put :update, request_params }.to change { note.reload.note } + end + end + describe 'DELETE destroy' do let(:request_params) do { diff --git a/spec/features/groups/members/manage_members.rb b/spec/features/groups/members/manage_members.rb index 9039b283393..da1e17225db 100644 --- a/spec/features/groups/members/manage_members.rb +++ b/spec/features/groups/members/manage_members.rb @@ -44,7 +44,11 @@ feature 'Groups > Members > Manage members' do visit group_group_members_path(group) - find(:css, '.project-members-page li', text: user2.name).find(:css, 'a.btn-remove').click + accept_confirm do + find(:css, '.project-members-page li', text: user2.name).find(:css, 'a.btn-remove').click + end + + wait_for_requests expect(page).not_to have_content(user2.name) expect(group.users).not_to include(user2) diff --git a/spec/features/projects/members/list_spec.rb b/spec/features/projects/members/list_spec.rb index 237c059e595..65b11a1d9e7 100644 --- a/spec/features/projects/members/list_spec.rb +++ b/spec/features/projects/members/list_spec.rb @@ -55,6 +55,22 @@ feature 'Project members list' do end end + scenario 'remove user from project', :js do + other_user = create(:user) + project.add_developer(other_user) + + visit_members_page + + accept_confirm do + find(:css, 'li.project_member', text: other_user.name).find(:css, 'a.btn-remove').click + end + + wait_for_requests + + expect(page).not_to have_content(other_user.name) + expect(project.users).not_to include(other_user) + end + scenario 'invite user to project', :js do visit_members_page diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 7a241b02d28..5c5d53877a6 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -4,8 +4,6 @@ require 'spec_helper' describe ApplicationHelper do include UploadHelpers - let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" } - describe 'current_controller?' do it 'returns true when controller matches argument' do stub_controller_name('foo') @@ -57,30 +55,11 @@ describe ApplicationHelper do end describe 'project_icon' do - let(:asset_host) { 'http://assets' } - it 'returns an url for the avatar' do project = create(:project, :public, avatar: File.open(uploaded_image_temp_path)) - avatar_url = "/uploads/-/system/project/avatar/#{project.id}/banana_sample.gif" - - expect(helper.project_icon(project.full_path).to_s) - .to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />" - - allow(ActionController::Base).to receive(:asset_host).and_return(asset_host) - avatar_url = "#{asset_host}/uploads/-/system/project/avatar/#{project.id}/banana_sample.gif" - - expect(helper.project_icon(project.full_path).to_s) - .to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />" - end - - it 'gives uploaded icon when present' do - project = create(:project) - allow_any_instance_of(Project).to receive(:avatar_in_git).and_return(true) - - avatar_url = "#{gitlab_host}#{project_avatar_path(project)}" expect(helper.project_icon(project.full_path).to_s) - .to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />" + .to eq "<img data-src=\"#{project.avatar.url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />" end end @@ -91,40 +70,7 @@ describe ApplicationHelper do context 'when there is a matching user' do it 'returns a relative URL for the avatar' do expect(helper.avatar_icon(user.email).to_s) - .to eq("/uploads/-/system/user/avatar/#{user.id}/banana_sample.gif") - end - - context 'when an asset_host is set in the config' do - let(:asset_host) { 'http://assets' } - - before do - allow(ActionController::Base).to receive(:asset_host).and_return(asset_host) - end - - it 'returns an absolute URL on that asset host' do - expect(helper.avatar_icon(user.email, only_path: false).to_s) - .to eq("#{asset_host}/uploads/-/system/user/avatar/#{user.id}/banana_sample.gif") - end - end - - context 'when only_path is set to false' do - it 'returns an absolute URL for the avatar' do - expect(helper.avatar_icon(user.email, only_path: false).to_s) - .to eq("#{gitlab_host}/uploads/-/system/user/avatar/#{user.id}/banana_sample.gif") - end - end - - context 'when the GitLab instance is at a relative URL' do - before do - stub_config_setting(relative_url_root: '/gitlab') - # Must be stubbed after the stub above, and separately - stub_config_setting(url: Settings.send(:build_gitlab_url)) - end - - it 'returns a relative URL with the correct prefix' do - expect(helper.avatar_icon(user.email).to_s) - .to eq("/gitlab/uploads/-/system/user/avatar/#{user.id}/banana_sample.gif") - end + .to eq(user.avatar.url) end end @@ -138,18 +84,9 @@ describe ApplicationHelper do end describe 'using a user' do - context 'when only_path is true' do - it 'returns a relative URL for the avatar' do - expect(helper.avatar_icon(user, only_path: true).to_s) - .to eq("/uploads/-/system/user/avatar/#{user.id}/banana_sample.gif") - end - end - - context 'when only_path is false' do - it 'returns an absolute URL for the avatar' do - expect(helper.avatar_icon(user, only_path: false).to_s) - .to eq("#{gitlab_host}/uploads/-/system/user/avatar/#{user.id}/banana_sample.gif") - end + it 'returns a relative URL for the avatar' do + expect(helper.avatar_icon(user).to_s) + .to eq(user.avatar.url) end end end diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb index 97f0ed4904e..32432ee1e81 100644 --- a/spec/helpers/groups_helper_spec.rb +++ b/spec/helpers/groups_helper_spec.rb @@ -3,8 +3,6 @@ require 'spec_helper' describe GroupsHelper do include ApplicationHelper - let(:asset_host) { 'http://assets' } - describe 'group_icon' do avatar_file_path = File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif') @@ -13,16 +11,8 @@ describe GroupsHelper do group.avatar = fixture_file_upload(avatar_file_path) group.save! - avatar_url = "/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif" - - expect(helper.group_icon(group).to_s) - .to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />" - - allow(ActionController::Base).to receive(:asset_host).and_return(asset_host) - avatar_url = "#{asset_host}/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif" - expect(helper.group_icon(group).to_s) - .to eq "<img data-src=\"#{avatar_url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />" + .to eq "<img data-src=\"#{group.avatar.url}\" class=\" lazy\" src=\"#{LazyImageTagHelper.placeholder_image}\" />" end end @@ -34,25 +24,7 @@ describe GroupsHelper do group.avatar = fixture_file_upload(avatar_file_path) group.save! expect(group_icon_url(group.path).to_s) - .to match("/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif") - end - - it 'returns an CDN url for the avatar' do - allow(ActionController::Base).to receive(:asset_host).and_return(asset_host) - group = create(:group) - group.avatar = fixture_file_upload(avatar_file_path) - group.save! - expect(group_icon_url(group.path).to_s) - .to match("#{asset_host}/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif") - end - - it 'returns an based url for the avatar if private' do - allow(ActionController::Base).to receive(:asset_host).and_return(asset_host) - group = create(:group, :private) - group.avatar = fixture_file_upload(avatar_file_path) - group.save! - expect(group_icon_url(group.path).to_s) - .to match("/uploads/-/system/group/avatar/#{group.id}/banana_sample.gif") + .to match(group.avatar.url) end it 'gives default avatar_icon when no avatar is present' do diff --git a/spec/helpers/tree_helper_spec.rb b/spec/helpers/tree_helper_spec.rb index d7b66e6f078..c358ccae9c3 100644 --- a/spec/helpers/tree_helper_spec.rb +++ b/spec/helpers/tree_helper_spec.rb @@ -1,10 +1,36 @@ require 'spec_helper' describe TreeHelper do + let(:project) { create(:project, :repository) } + let(:repository) { project.repository } + let(:sha) { 'ce369011c189f62c815f5971d096b26759bab0d1' } + + describe '.render_tree' do + before do + @id = sha + @project = project + end + + it 'displays all entries without a warning' do + tree = repository.tree(sha, 'files') + + html = render_tree(tree) + + expect(html).not_to have_selector('.tree-truncated-warning') + end + + it 'truncates entries and adds a warning' do + stub_const('TreeHelper::FILE_LIMIT', 1) + tree = repository.tree(sha, 'files') + + html = render_tree(tree) + + expect(html).to have_selector('.tree-truncated-warning', count: 1) + expect(html).to have_selector('.tree-item-file-name', count: 1) + end + end + describe 'flatten_tree' do - let(:project) { create(:project, :repository) } - let(:repository) { project.repository } - let(:sha) { 'ce369011c189f62c815f5971d096b26759bab0d1' } let(:tree) { repository.tree(sha, 'files') } let(:root_path) { 'files' } let(:tree_item) { tree.entries.find { |entry| entry.path == path } } diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js index 2ea290108a4..5662c7387fb 100644 --- a/spec/javascripts/issue_show/components/app_spec.js +++ b/spec/javascripts/issue_show/components/app_spec.js @@ -223,23 +223,46 @@ describe('Issuable output', () => { }); }); - it('closes form on error', (done) => { - spyOn(window, 'Flash').and.callThrough(); - spyOn(vm.service, 'updateIssuable').and.callFake(() => new Promise((resolve, reject) => { - reject(); - })); + describe('error when updating', () => { + beforeEach(() => { + spyOn(window, 'Flash').and.callThrough(); + spyOn(vm.service, 'updateIssuable').and.callFake(() => new Promise((resolve, reject) => { + reject(); + })); + }); - vm.updateIssuable(); + it('closes form on error', (done) => { + vm.updateIssuable(); - setTimeout(() => { - expect( - eventHub.$emit, - ).toHaveBeenCalledWith('close.form'); - expect( - window.Flash, - ).toHaveBeenCalledWith('Error updating issue'); + setTimeout(() => { + expect( + eventHub.$emit, + ).toHaveBeenCalledWith('close.form'); + expect( + window.Flash, + ).toHaveBeenCalledWith('Error updating issue'); - done(); + done(); + }); + }); + + it('returns the correct error message for issuableType', (done) => { + vm.issuableType = 'merge request'; + + Vue.nextTick(() => { + vm.updateIssuable(); + + setTimeout(() => { + expect( + eventHub.$emit, + ).toHaveBeenCalledWith('close.form'); + expect( + window.Flash, + ).toHaveBeenCalledWith('Error updating merge request'); + + done(); + }); + }); }); }); }); diff --git a/spec/javascripts/issue_show/components/edit_actions_spec.js b/spec/javascripts/issue_show/components/edit_actions_spec.js index f6625b748b6..d779ab7bb31 100644 --- a/spec/javascripts/issue_show/components/edit_actions_spec.js +++ b/spec/javascripts/issue_show/components/edit_actions_spec.js @@ -61,6 +61,15 @@ describe('Edit Actions components', () => { }); }); + it('should not show delete button if showDeleteButton is false', (done) => { + vm.showDeleteButton = false; + + Vue.nextTick(() => { + expect(vm.$el.querySelector('.btn-danger')).toBeNull(); + done(); + }); + }); + describe('updateIssuable', () => { it('sends update.issauble event when clicking save button', () => { vm.$el.querySelector('.btn-save').click(); diff --git a/spec/javascripts/lib/utils/number_utility_spec.js b/spec/javascripts/lib/utils/number_utility_spec.js index 83c92deccdc..fcf27f6805f 100644 --- a/spec/javascripts/lib/utils/number_utility_spec.js +++ b/spec/javascripts/lib/utils/number_utility_spec.js @@ -1,4 +1,4 @@ -import { formatRelevantDigits, bytesToKiB, bytesToMiB } from '~/lib/utils/number_utils'; +import { formatRelevantDigits, bytesToKiB, bytesToMiB, bytesToGiB, numberToHumanSize } from '~/lib/utils/number_utils'; describe('Number Utils', () => { describe('formatRelevantDigits', () => { @@ -52,4 +52,29 @@ describe('Number Utils', () => { expect(bytesToMiB(1000000)).toEqual(0.95367431640625); }); }); + + describe('bytesToGiB', () => { + it('calculates GiB for the given bytes', () => { + expect(bytesToGiB(1073741824)).toEqual(1); + expect(bytesToGiB(10737418240)).toEqual(10); + }); + }); + + describe('numberToHumanSize', () => { + it('should return bytes', () => { + expect(numberToHumanSize(654)).toEqual('654 bytes'); + }); + + it('should return KiB', () => { + expect(numberToHumanSize(1079)).toEqual('1.05 KiB'); + }); + + it('should return MiB', () => { + expect(numberToHumanSize(10485764)).toEqual('10.00 MiB'); + }); + + it('should return GiB', () => { + expect(numberToHumanSize(10737418240)).toEqual('10.00 GiB'); + }); + }); }); diff --git a/spec/javascripts/vue_shared/components/loading_button_spec.js b/spec/javascripts/vue_shared/components/loading_button_spec.js index 97c8a08fcdd..c1eabdede00 100644 --- a/spec/javascripts/vue_shared/components/loading_button_spec.js +++ b/spec/javascripts/vue_shared/components/loading_button_spec.js @@ -66,6 +66,23 @@ describe('LoadingButton', function () { }); }); + describe('container class', () => { + it('should default to btn btn-align-content', () => { + vm = mountComponent(LoadingButton, {}); + expect(vm.$el.classList.contains('btn')).toEqual(true); + expect(vm.$el.classList.contains('btn-align-content')).toEqual(true); + }); + + it('should be configurable through props', () => { + vm = mountComponent(LoadingButton, { + containerClass: 'test-class', + }); + expect(vm.$el.classList.contains('btn')).toEqual(false); + expect(vm.$el.classList.contains('btn-align-content')).toEqual(false); + expect(vm.$el.classList.contains('test-class')).toEqual(true); + }); + }); + describe('click callback prop', () => { it('calls given callback when normal', () => { vm = mountComponent(LoadingButton, { diff --git a/spec/lib/container_registry/path_spec.rb b/spec/lib/container_registry/path_spec.rb index 84cacdd3f0d..010deae822c 100644 --- a/spec/lib/container_registry/path_spec.rb +++ b/spec/lib/container_registry/path_spec.rb @@ -86,6 +86,24 @@ describe ContainerRegistry::Path do it { is_expected.to be_valid } end + + context 'when path contains double underscore' do + let(:path) { 'my/repository__name' } + + it { is_expected.to be_valid } + end + + context 'when path contains invalid separator with dot' do + let(:path) { 'some/registry-.name' } + + it { is_expected.not_to be_valid } + end + + context 'when path contains invalid separator with underscore' do + let(:path) { 'some/registry._name' } + + it { is_expected.not_to be_valid } + end end describe '#has_repository?' do diff --git a/spec/lib/gitlab/background_migration/create_fork_network_memberships_range_spec.rb b/spec/lib/gitlab/background_migration/create_fork_network_memberships_range_spec.rb index 1a4ea2bac48..79d2c071446 100644 --- a/spec/lib/gitlab/background_migration/create_fork_network_memberships_range_spec.rb +++ b/spec/lib/gitlab/background_migration/create_fork_network_memberships_range_spec.rb @@ -93,7 +93,14 @@ describe Gitlab::BackgroundMigration::CreateForkNetworkMembershipsRange, :migrat end it 'knows it is finished for this range' do - expect(migration.missing_members?(1, 7)).to be_falsy + expect(migration.missing_members?(1, 8)).to be_falsy + end + + it 'does not miss members for forks of forks for which the root was deleted' do + forked_project_links.create(id: 9, forked_from_project_id: base1_fork1.id, forked_to_project_id: create(:project).id) + base1.destroy + + expect(migration.missing_members?(7, 10)).to be_falsy end context 'with more forks' do diff --git a/spec/lib/gitlab/background_migration/populate_fork_networks_range_spec.rb b/spec/lib/gitlab/background_migration/populate_fork_networks_range_spec.rb index 2c2684a6fc9..994992f79d4 100644 --- a/spec/lib/gitlab/background_migration/populate_fork_networks_range_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_fork_networks_range_spec.rb @@ -3,12 +3,9 @@ require 'spec_helper' describe Gitlab::BackgroundMigration::PopulateForkNetworksRange, :migration, schema: 20170929131201 do let(:migration) { described_class.new } let(:base1) { create(:project) } - let(:base1_fork1) { create(:project) } - let(:base1_fork2) { create(:project) } let(:base2) { create(:project) } let(:base2_fork1) { create(:project) } - let(:base2_fork2) { create(:project) } let!(:forked_project_links) { table(:forked_project_links) } let!(:fork_networks) { table(:fork_networks) } @@ -21,21 +18,24 @@ describe Gitlab::BackgroundMigration::PopulateForkNetworksRange, :migration, sch # A normal fork link forked_project_links.create(id: 1, forked_from_project_id: base1.id, - forked_to_project_id: base1_fork1.id) + forked_to_project_id: create(:project).id) forked_project_links.create(id: 2, forked_from_project_id: base1.id, - forked_to_project_id: base1_fork2.id) - + forked_to_project_id: create(:project).id) forked_project_links.create(id: 3, forked_from_project_id: base2.id, forked_to_project_id: base2_fork1.id) + + # create a fork of a fork forked_project_links.create(id: 4, forked_from_project_id: base2_fork1.id, forked_to_project_id: create(:project).id) - forked_project_links.create(id: 5, - forked_from_project_id: base2.id, - forked_to_project_id: base2_fork2.id) + forked_from_project_id: create(:project).id, + forked_to_project_id: create(:project).id) + + # Stub out the calls to the other migrations + allow(BackgroundMigrationWorker).to receive(:perform_in) migration.perform(1, 3) end @@ -80,11 +80,11 @@ describe Gitlab::BackgroundMigration::PopulateForkNetworksRange, :migration, sch end it 'only processes a single batch of links at a time' do - expect(fork_network_members.count).to eq(5) + expect(fork_networks.count).to eq(2) migration.perform(3, 5) - expect(fork_network_members.count).to eq(7) + expect(fork_networks.count).to eq(3) end it 'can be repeated without effect' do diff --git a/spec/migrations/populate_merge_requests_latest_merge_request_diff_id_spec.rb b/spec/lib/gitlab/background_migration/populate_merge_requests_latest_merge_request_diff_id_spec.rb index 4ea7f441f7c..0cb753c5853 100644 --- a/spec/migrations/populate_merge_requests_latest_merge_request_diff_id_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_merge_requests_latest_merge_request_diff_id_spec.rb @@ -1,7 +1,6 @@ require 'spec_helper' -require Rails.root.join('db', 'post_migrate', '20171026082505_populate_merge_requests_latest_merge_request_diff_id') -describe PopulateMergeRequestsLatestMergeRequestDiffId, :migration do +describe Gitlab::BackgroundMigration::PopulateMergeRequestsLatestMergeRequestDiffId, :migration, schema: 20171026082505 do let(:projects_table) { table(:projects) } let(:merge_requests_table) { table(:merge_requests) } let(:merge_request_diffs_table) { table(:merge_request_diffs) } @@ -27,30 +26,32 @@ describe PopulateMergeRequestsLatestMergeRequestDiffId, :migration do merge_request_diffs_table.where(merge_request_id: merge_request.id) end - describe '#up' do + describe '#perform' do it 'ignores MRs without diffs' do merge_request_without_diff = create_mr!('without_diff') + mr_id = merge_request_without_diff.id expect(merge_request_without_diff.latest_merge_request_diff_id).to be_nil - expect { migrate! } + expect { subject.perform(mr_id, mr_id) } .not_to change { merge_request_without_diff.reload.latest_merge_request_diff_id } end it 'ignores MRs that have a diff ID already set' do merge_request_with_multiple_diffs = create_mr!('with_multiple_diffs', diffs: 3) diff_id = diffs_for(merge_request_with_multiple_diffs).minimum(:id) + mr_id = merge_request_with_multiple_diffs.id merge_request_with_multiple_diffs.update!(latest_merge_request_diff_id: diff_id) - expect { migrate! } + expect { subject.perform(mr_id, mr_id) } .not_to change { merge_request_with_multiple_diffs.reload.latest_merge_request_diff_id } end it 'migrates multiple MR diffs to the correct values' do merge_requests = Array.new(3).map.with_index { |_, i| create_mr!(i, diffs: 3) } - migrate! + subject.perform(merge_requests.first.id, merge_requests.last.id) merge_requests.each do |merge_request| expect(merge_request.reload.latest_merge_request_diff_id) diff --git a/spec/lib/gitlab/checks/change_access_spec.rb b/spec/lib/gitlab/checks/change_access_spec.rb index 74a24a4424b..c2bca816aae 100644 --- a/spec/lib/gitlab/checks/change_access_spec.rb +++ b/spec/lib/gitlab/checks/change_access_spec.rb @@ -165,47 +165,16 @@ describe Gitlab::Checks::ChangeAccess do end context 'LFS integrity check' do - let(:blob_object) { project.repository.blob_at_branch('lfs', 'files/lfs/lfs_object.iso') } + it 'fails if any LFS blobs are missing' do + allow_any_instance_of(Gitlab::Checks::LfsIntegrity).to receive(:objects_missing?).and_return(true) - before do - allow_any_instance_of(Gitlab::Git::RevList).to receive(:new_objects) do |&lazy_block| - lazy_block.call([blob_object.id]) - end - end - - context 'with LFS not enabled' do - it 'skips integrity check' do - expect_any_instance_of(Gitlab::Git::RevList).not_to receive(:new_objects) - - subject.exec - end + expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /LFS objects are missing/) end - context 'with LFS enabled' do - before do - allow(project).to receive(:lfs_enabled?).and_return(true) - end - - context 'deletion' do - let(:changes) { { oldrev: oldrev, ref: ref } } - - it 'skips integrity check' do - expect_any_instance_of(Gitlab::Git::RevList).not_to receive(:new_objects) + it 'succeeds if LFS objects have already been uploaded' do + allow_any_instance_of(Gitlab::Checks::LfsIntegrity).to receive(:objects_missing?).and_return(false) - subject.exec - end - end - - it 'fails if any LFS blobs are missing' do - expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /LFS objects are missing/) - end - - it 'succeeds if LFS objects have already been uploaded' do - lfs_object = create(:lfs_object, oid: blob_object.lfs_oid) - create(:lfs_objects_project, project: project, lfs_object: lfs_object) - - expect { subject.exec }.not_to raise_error - end + expect { subject.exec }.not_to raise_error end end end diff --git a/spec/lib/gitlab/checks/lfs_integrity_spec.rb b/spec/lib/gitlab/checks/lfs_integrity_spec.rb new file mode 100644 index 00000000000..17756621221 --- /dev/null +++ b/spec/lib/gitlab/checks/lfs_integrity_spec.rb @@ -0,0 +1,74 @@ +require 'spec_helper' + +describe Gitlab::Checks::LfsIntegrity do + include ProjectForksHelper + let(:project) { create(:project, :repository) } + let(:newrev) { '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51' } + + subject { described_class.new(project, newrev) } + + describe '#objects_missing?' do + let(:blob_object) { project.repository.blob_at_branch('lfs', 'files/lfs/lfs_object.iso') } + + before do + allow_any_instance_of(Gitlab::Git::RevList).to receive(:new_objects) do |&lazy_block| + lazy_block.call([blob_object.id]) + end + end + + context 'with LFS not enabled' do + it 'skips integrity check' do + expect_any_instance_of(Gitlab::Git::RevList).not_to receive(:new_objects) + + subject.objects_missing? + end + end + + context 'with LFS enabled' do + before do + allow(project).to receive(:lfs_enabled?).and_return(true) + end + + context 'deletion' do + let(:newrev) { nil } + + it 'skips integrity check' do + expect_any_instance_of(Gitlab::Git::RevList).not_to receive(:new_objects) + + expect(subject.objects_missing?).to be_falsey + end + end + + it 'is true if any LFS blobs are missing' do + expect(subject.objects_missing?).to be_truthy + end + + it 'is false if LFS objects have already been uploaded' do + lfs_object = create(:lfs_object, oid: blob_object.lfs_oid) + create(:lfs_objects_project, project: project, lfs_object: lfs_object) + + expect(subject.objects_missing?).to be_falsey + end + end + + context 'for forked project' do + let(:parent_project) { create(:project, :repository) } + let(:project) { fork_project(parent_project, nil, repository: true) } + + before do + allow(project).to receive(:lfs_enabled?).and_return(true) + end + + it 'is true parent project is missing LFS objects' do + expect(subject.objects_missing?).to be_truthy + end + + it 'is false parent project already conatins LFS objects for the fork' do + lfs_object = create(:lfs_object, oid: blob_object.lfs_oid) + create(:lfs_objects_project, project: parent_project, lfs_object: lfs_object) + + expect(subject.objects_missing?).to be_falsey + end + end + end +end diff --git a/spec/lib/gitlab/middleware/go_spec.rb b/spec/lib/gitlab/middleware/go_spec.rb index 67121937398..60a134be939 100644 --- a/spec/lib/gitlab/middleware/go_spec.rb +++ b/spec/lib/gitlab/middleware/go_spec.rb @@ -127,6 +127,14 @@ describe Gitlab::Middleware::Go do include_examples 'go-get=1', enabled_protocol: nil end + + context 'with nothing disabled (blank string)' do + before do + stub_application_setting(enabled_git_access_protocol: '') + end + + include_examples 'go-get=1', enabled_protocol: nil + end end def go diff --git a/spec/migrations/schedule_merge_request_latest_merge_request_diff_id_migrations_spec.rb b/spec/migrations/schedule_merge_request_latest_merge_request_diff_id_migrations_spec.rb new file mode 100644 index 00000000000..158d0bc02ed --- /dev/null +++ b/spec/migrations/schedule_merge_request_latest_merge_request_diff_id_migrations_spec.rb @@ -0,0 +1,64 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20171026082505_schedule_merge_request_latest_merge_request_diff_id_migrations') + +describe ScheduleMergeRequestLatestMergeRequestDiffIdMigrations, :migration, :sidekiq do + let(:projects_table) { table(:projects) } + let(:merge_requests_table) { table(:merge_requests) } + let(:merge_request_diffs_table) { table(:merge_request_diffs) } + + let(:project) { projects_table.create!(name: 'gitlab', path: 'gitlab-org/gitlab-ce') } + + let!(:merge_request_1) { create_mr!('mr_1', diffs: 1) } + let!(:merge_request_2) { create_mr!('mr_2', diffs: 2) } + let!(:merge_request_migrated) { create_mr!('merge_request_migrated', diffs: 3) } + let!(:merge_request_4) { create_mr!('mr_4', diffs: 3) } + + def create_mr!(name, diffs: 0) + merge_request = + merge_requests_table.create!(target_project_id: project.id, + target_branch: 'master', + source_project_id: project.id, + source_branch: name, + title: name) + + diffs.times do + merge_request_diffs_table.create!(merge_request_id: merge_request.id) + end + + merge_request + end + + def diffs_for(merge_request) + merge_request_diffs_table.where(merge_request_id: merge_request.id) + end + + before do + stub_const("#{described_class.name}::BATCH_SIZE", 1) + + diff_id = diffs_for(merge_request_migrated).minimum(:id) + merge_request_migrated.update!(latest_merge_request_diff_id: diff_id) + end + + it 'correctly schedules background migrations' do + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, merge_request_1.id, merge_request_1.id) + expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, merge_request_2.id, merge_request_2.id) + expect(described_class::MIGRATION).to be_scheduled_migration(15.minutes, merge_request_4.id, merge_request_4.id) + expect(BackgroundMigrationWorker.jobs.size).to eq 3 + end + end + end + + it 'schedules background migrations' do + Sidekiq::Testing.inline! do + expect(merge_requests_table.where(latest_merge_request_diff_id: nil).count).to eq 3 + + migrate! + + expect(merge_requests_table.where(latest_merge_request_diff_id: nil).count).to eq 0 + end + end +end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 2c9e7013b77..b89b0e555d9 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -625,38 +625,29 @@ describe Ci::Pipeline, :mailer do shared_context 'with some outdated pipelines' do before do - create_pipeline(:canceled, 'ref', 'A') - create_pipeline(:success, 'ref', 'A') - create_pipeline(:failed, 'ref', 'B') - create_pipeline(:skipped, 'feature', 'C') + create_pipeline(:canceled, 'ref', 'A', project) + create_pipeline(:success, 'ref', 'A', project) + create_pipeline(:failed, 'ref', 'B', project) + create_pipeline(:skipped, 'feature', 'C', project) end - def create_pipeline(status, ref, sha) - create(:ci_empty_pipeline, status: status, ref: ref, sha: sha) + def create_pipeline(status, ref, sha, project) + create( + :ci_empty_pipeline, + status: status, + ref: ref, + sha: sha, + project: project + ) end end - describe '.latest' do + describe '.newest_first' do include_context 'with some outdated pipelines' - context 'when no ref is specified' do - let(:pipelines) { described_class.latest.all } - - it 'returns the latest pipeline for the same ref and different sha' do - expect(pipelines.map(&:sha)).to contain_exactly('A', 'B', 'C') - expect(pipelines.map(&:status)) - .to contain_exactly('success', 'failed', 'skipped') - end - end - - context 'when ref is specified' do - let(:pipelines) { described_class.latest('ref').all } - - it 'returns the latest pipeline for ref and different sha' do - expect(pipelines.map(&:sha)).to contain_exactly('A', 'B') - expect(pipelines.map(&:status)) - .to contain_exactly('success', 'failed') - end + it 'returns the pipelines from new to old' do + expect(described_class.newest_first.pluck(:status)) + .to eq(%w[skipped failed success canceled]) end end @@ -664,20 +655,14 @@ describe Ci::Pipeline, :mailer do include_context 'with some outdated pipelines' context 'when no ref is specified' do - let(:latest_status) { described_class.latest_status } - - it 'returns the latest status for the same ref and different sha' do - expect(latest_status).to eq(described_class.latest.status) - expect(latest_status).to eq('failed') + it 'returns the status of the latest pipeline' do + expect(described_class.latest_status).to eq('skipped') end end context 'when ref is specified' do - let(:latest_status) { described_class.latest_status('ref') } - - it 'returns the latest status for ref and different sha' do - expect(latest_status).to eq(described_class.latest_status('ref')) - expect(latest_status).to eq('failed') + it 'returns the status of the latest pipeline for the given ref' do + expect(described_class.latest_status('ref')).to eq('failed') end end end @@ -686,7 +671,7 @@ describe Ci::Pipeline, :mailer do include_context 'with some outdated pipelines' let!(:latest_successful_pipeline) do - create_pipeline(:success, 'ref', 'D') + create_pipeline(:success, 'ref', 'D', project) end it 'returns the latest successful pipeline' do @@ -698,8 +683,13 @@ describe Ci::Pipeline, :mailer do describe '.latest_successful_for_refs' do include_context 'with some outdated pipelines' - let!(:latest_successful_pipeline1) { create_pipeline(:success, 'ref1', 'D') } - let!(:latest_successful_pipeline2) { create_pipeline(:success, 'ref2', 'D') } + let!(:latest_successful_pipeline1) do + create_pipeline(:success, 'ref1', 'D', project) + end + + let!(:latest_successful_pipeline2) do + create_pipeline(:success, 'ref2', 'D', project) + end it 'returns the latest successful pipeline for both refs' do refs = %w(ref1 ref2 ref3) @@ -708,6 +698,62 @@ describe Ci::Pipeline, :mailer do end end + describe '.latest_status_per_commit' do + let(:project) { create(:project) } + + before do + pairs = [ + %w[success ref1 123], + %w[manual master 123], + %w[failed ref 456] + ] + + pairs.each do |(status, ref, sha)| + create( + :ci_empty_pipeline, + status: status, + ref: ref, + sha: sha, + project: project + ) + end + end + + context 'without a ref' do + it 'returns a Hash containing the latest status per commit for all refs' do + expect(described_class.latest_status_per_commit(%w[123 456])) + .to eq({ '123' => 'manual', '456' => 'failed' }) + end + + it 'only includes the status of the given commit SHAs' do + expect(described_class.latest_status_per_commit(%w[123])) + .to eq({ '123' => 'manual' }) + end + + context 'when there are two pipelines for a ref and SHA' do + it 'returns the status of the latest pipeline' do + create( + :ci_empty_pipeline, + status: 'failed', + ref: 'master', + sha: '123', + project: project + ) + + expect(described_class.latest_status_per_commit(%w[123])) + .to eq({ '123' => 'failed' }) + end + end + end + + context 'with a ref' do + it 'only includes the pipelines for the given ref' do + expect(described_class.latest_status_per_commit(%w[123 456], 'master')) + .to eq({ '123' => 'manual' }) + end + end + end + describe '.internal_sources' do subject { described_class.internal_sources } diff --git a/spec/models/commit_collection_spec.rb b/spec/models/commit_collection_spec.rb new file mode 100644 index 00000000000..066fe7d154e --- /dev/null +++ b/spec/models/commit_collection_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +describe CommitCollection do + let(:project) { create(:project, :repository) } + let(:commit) { project.commit } + + describe '#each' do + it 'yields every commit' do + collection = described_class.new(project, [commit]) + + expect { |b| collection.each(&b) }.to yield_with_args(commit) + end + end + + describe '#with_pipeline_status' do + it 'sets the pipeline status for every commit so no additional queries are necessary' do + create( + :ci_empty_pipeline, + ref: 'master', + sha: commit.id, + status: 'success', + project: project + ) + + collection = described_class.new(project, [commit]) + collection.with_pipeline_status + + recorder = ActiveRecord::QueryRecorder.new do + expect(commit.status).to eq('success') + end + + expect(recorder.count).to be_zero + end + end + + describe '#respond_to_missing?' do + it 'returns true when the underlying Array responds to the message' do + collection = described_class.new(project, []) + + expect(collection.respond_to?(:last)).to eq(true) + end + + it 'returns false when the underlying Array does not respond to the message' do + collection = described_class.new(project, []) + + expect(collection.respond_to?(:foo)).to eq(false) + end + end + + describe '#method_missing' do + it 'delegates undefined methods to the underlying Array' do + collection = described_class.new(project, [commit]) + + expect(collection.length).to eq(1) + expect(collection.last).to eq(commit) + expect(collection).not_to be_empty + end + end +end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index e3cfa149e3a..d18a5c9dfa6 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -351,12 +351,19 @@ eos end it 'gives compound status from latest pipelines if ref is nil' do - expect(commit.status(nil)).to eq(Ci::Pipeline.latest_status) - expect(commit.status(nil)).to eq('failed') + expect(commit.status(nil)).to eq(pipeline_from_fix.status) end end end + describe '#set_status_for_ref' do + it 'sets the status for a given reference' do + commit.set_status_for_ref('master', 'failed') + + expect(commit.status('master')).to eq('failed') + end + end + describe '#participants' do let(:user1) { build(:user) } let(:user2) { build(:user) } diff --git a/spec/models/concerns/avatarable_spec.rb b/spec/models/concerns/avatarable_spec.rb new file mode 100644 index 00000000000..cbdc438be0b --- /dev/null +++ b/spec/models/concerns/avatarable_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper' + +describe Avatarable do + subject { create(:project, avatar: fixture_file_upload(File.join(Rails.root, 'spec/fixtures/dk.png'))) } + + let(:gitlab_host) { "https://gitlab.example.com" } + let(:relative_url_root) { "/gitlab" } + let(:asset_host) { "https://gitlab-assets.example.com" } + + before do + stub_config_setting(base_url: gitlab_host) + stub_config_setting(relative_url_root: relative_url_root) + end + + describe '#avatar_path' do + using RSpec::Parameterized::TableSyntax + + where(:has_asset_host, :visibility_level, :only_path, :avatar_path) do + true | Project::PRIVATE | true | [gitlab_host, relative_url_root, subject.avatar.url] + true | Project::PRIVATE | false | [gitlab_host, relative_url_root, subject.avatar.url] + true | Project::INTERNAL | true | [gitlab_host, relative_url_root, subject.avatar.url] + true | Project::INTERNAL | false | [gitlab_host, relative_url_root, subject.avatar.url] + true | Project::PUBLIC | true | [subject.avatar.url] + true | Project::PUBLIC | false | [asset_host, subject.avatar.url] + false | Project::PRIVATE | true | [relative_url_root, subject.avatar.url] + false | Project::PRIVATE | false | [gitlab_host, relative_url_root, subject.avatar.url] + false | Project::INTERNAL | true | [relative_url_root, subject.avatar.url] + false | Project::INTERNAL | false | [gitlab_host, relative_url_root, subject.avatar.url] + false | Project::PUBLIC | true | [relative_url_root, subject.avatar.url] + false | Project::PUBLIC | false | [gitlab_host, relative_url_root, subject.avatar.url] + end + + with_them do + before do + allow(ActionController::Base).to receive(:asset_host).and_return(has_asset_host ? asset_host : nil) + subject.visibility_level = visibility_level + end + + it 'returns the expected avatar path' do + expect(subject.avatar_path(only_path: only_path)).to eq(avatar_path.join) + end + end + end +end diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb index da972d2d86a..8389d5c5430 100644 --- a/spec/models/diff_note_spec.rb +++ b/spec/models/diff_note_spec.rb @@ -283,6 +283,12 @@ describe DiffNote do expect(diff_line).to be nil expect(subject).to be_valid end + + it "does not update the position" do + expect(subject).not_to receive(:update_position) + + subject.save + end end it "returns true for on_image?" do diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index d4052a64570..5e82a2988ce 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -247,8 +247,6 @@ describe Group do describe '#avatar_url' do let!(:group) { create(:group, :access_requestable, :with_avatar) } let(:user) { create(:user) } - let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" } - let(:avatar_path) { "/uploads/-/system/group/avatar/#{group.id}/dk.png" } context 'when avatar file is uploaded' do before do @@ -256,12 +254,8 @@ describe Group do end it 'shows correct avatar url' do - expect(group.avatar_url).to eq(avatar_path) - expect(group.avatar_url(only_path: false)).to eq([gitlab_host, avatar_path].join) - - allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host) - - expect(group.avatar_url).to eq([gitlab_host, avatar_path].join) + expect(group.avatar_url).to eq(group.avatar.url) + expect(group.avatar_url(only_path: false)).to eq([Gitlab.config.gitlab.url, group.avatar.url].join) end end end diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index 81c2057e175..4cd9e3f4f1d 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -166,4 +166,27 @@ describe Key, :mailer do expect(key.public_key.key_text).to eq(valid_key) end end + + describe '#refresh_user_cache', :use_clean_rails_memory_store_caching do + context 'when the key belongs to a user' do + it 'refreshes the keys count cache for the user' do + expect_any_instance_of(Users::KeysCountService) + .to receive(:refresh_cache) + .and_call_original + + key = create(:personal_key) + + expect(Users::KeysCountService.new(key.user).count).to eq(1) + end + end + + context 'when the key does not belong to a user' do + it 'does nothing' do + expect_any_instance_of(Users::KeysCountService) + .not_to receive(:refresh_cache) + + create(:key) + end + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e684389af5d..f7f19d464d1 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -883,20 +883,14 @@ describe Project do context 'when avatar file is uploaded' do let(:project) { create(:project, :public, :with_avatar) } - let(:avatar_path) { "/uploads/-/system/project/avatar/#{project.id}/dk.png" } - let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" } it 'shows correct url' do - expect(project.avatar_url).to eq(avatar_path) - expect(project.avatar_url(only_path: false)).to eq([gitlab_host, avatar_path].join) - - allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host) - - expect(project.avatar_url).to eq([gitlab_host, avatar_path].join) + expect(project.avatar_url).to eq(project.avatar.url) + expect(project.avatar_url(only_path: false)).to eq([Gitlab.config.gitlab.url, project.avatar.url].join) end end - context 'When avatar file in git' do + context 'when avatar file in git' do before do allow(project).to receive(:avatar_in_git) { true } end @@ -1944,6 +1938,24 @@ describe Project do expect(second_fork.fork_source).to eq(project) end end + + describe '#lfs_storage_project' do + it 'returns self for non-forks' do + expect(project.lfs_storage_project).to eq project + end + + it 'returns the fork network root for forks' do + second_fork = fork_project(forked_project) + + expect(second_fork.lfs_storage_project).to eq project + end + + it 'returns self when fork_source is nil' do + expect(forked_project).to receive(:fork_source).and_return(nil) + + expect(forked_project.lfs_storage_project).to eq forked_project + end + end end describe '#pushes_since_gc' do diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 3d46434fc27..929086305ba 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -10,6 +10,10 @@ describe ProjectWiki do subject { project_wiki } + it { is_expected.to delegate_method(:empty?).to :pages } + it { is_expected.to delegate_method(:repository_storage_path).to :project } + it { is_expected.to delegate_method(:hashed_storage?).to :project } + describe "#path_with_namespace" do it "returns the project path with namespace with the .wiki extension" do expect(subject.path_with_namespace).to eq(project.full_path + '.wiki') diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a3ebf649aa8..3b5ee70ae7c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -804,7 +804,7 @@ describe User do end end - describe '#require_ssh_key?' do + describe '#require_ssh_key?', :use_clean_rails_memory_store_caching do protocol_and_expectation = { 'http' => false, 'ssh' => true, @@ -819,6 +819,12 @@ describe User do expect(user.require_ssh_key?).to eq(expected) end end + + it 'returns false when the user has 1 or more SSH keys' do + key = create(:personal_key) + + expect(key.user.require_ssh_key?).to eq(false) + end end end @@ -1149,16 +1155,9 @@ describe User do let(:user) { create(:user, :with_avatar) } context 'when avatar file is uploaded' do - let(:gitlab_host) { "http://#{Gitlab.config.gitlab.host}" } - let(:avatar_path) { "/uploads/-/system/user/avatar/#{user.id}/dk.png" } - it 'shows correct avatar url' do - expect(user.avatar_url).to eq(avatar_path) - expect(user.avatar_url(only_path: false)).to eq([gitlab_host, avatar_path].join) - - allow(ActionController::Base).to receive(:asset_host).and_return(gitlab_host) - - expect(user.avatar_url).to eq([gitlab_host, avatar_path].join) + expect(user.avatar_url).to eq(user.avatar.url) + expect(user.avatar_url(only_path: false)).to eq([Gitlab.config.gitlab.url, user.avatar.url].join) end end end diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index d919899282d..34ecdd1e164 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -203,18 +203,44 @@ describe API::Internal do end context 'with env passed as a JSON' do - it 'sets env in RequestStore' do - expect(Gitlab::Git::Env).to receive(:set).with({ - 'GIT_OBJECT_DIRECTORY' => 'foo', - 'GIT_ALTERNATE_OBJECT_DIRECTORIES' => 'bar' - }) + context 'when relative path envs are not set' do + it 'sets env in RequestStore' do + expect(Gitlab::Git::Env).to receive(:set).with({ + 'GIT_OBJECT_DIRECTORY' => 'foo', + 'GIT_ALTERNATE_OBJECT_DIRECTORIES' => 'bar' + }) + + push(key, project.wiki, env: { + GIT_OBJECT_DIRECTORY: 'foo', + GIT_ALTERNATE_OBJECT_DIRECTORIES: 'bar' + }.to_json) - push(key, project.wiki, env: { - GIT_OBJECT_DIRECTORY: 'foo', - GIT_ALTERNATE_OBJECT_DIRECTORIES: 'bar' - }.to_json) + expect(response).to have_gitlab_http_status(200) + end + end - expect(response).to have_gitlab_http_status(200) + context 'when relative path envs are set' do + it 'sets env in RequestStore' do + obj_dir_relative = './objects' + alt_obj_dirs_relative = ['./alt-objects-1', './alt-objects-2'] + repo_path = project.wiki.repository.path_to_repo + + expect(Gitlab::Git::Env).to receive(:set).with({ + 'GIT_OBJECT_DIRECTORY' => File.join(repo_path, obj_dir_relative), + 'GIT_ALTERNATE_OBJECT_DIRECTORIES' => alt_obj_dirs_relative.map { |d| File.join(repo_path, d) }, + 'GIT_OBJECT_DIRECTORY_RELATIVE' => obj_dir_relative, + 'GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE' => alt_obj_dirs_relative + }) + + push(key, project.wiki, env: { + GIT_OBJECT_DIRECTORY: 'foo', + GIT_ALTERNATE_OBJECT_DIRECTORIES: 'bar', + GIT_OBJECT_DIRECTORY_RELATIVE: obj_dir_relative, + GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE: alt_obj_dirs_relative + }.to_json) + + expect(response).to have_gitlab_http_status(200) + end end end diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index 0b1f8ce6f6d..1a5ad9b04e4 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -107,6 +107,15 @@ describe 'OpenID Connect requests' do end end + # These 2 calls shouldn't actually throw, they should be handled as an + # unauthorized request, so we should be able to check the response. + # + # This was not possible due to an issue with Warden: + # https://github.com/hassox/warden/pull/162 + # + # When the patch gets merged and we update Warden, these specs will need to + # updated to check the response instead of a raised exception. + # https://gitlab.com/gitlab-org/gitlab-ce/issues/40218 context 'when user is blocked' do it 'returns authentication error' do access_grant @@ -114,7 +123,7 @@ describe 'OpenID Connect requests' do expect do request_access_token - end.to throw_symbol :warden + end.to raise_error UncaughtThrowError end end @@ -125,7 +134,7 @@ describe 'OpenID Connect requests' do expect do request_access_token - end.to throw_symbol :warden + end.to raise_error UncaughtThrowError end end end diff --git a/spec/routing/group_routing_spec.rb b/spec/routing/group_routing_spec.rb index 7a4c8304e62..71788028cbf 100644 --- a/spec/routing/group_routing_spec.rb +++ b/spec/routing/group_routing_spec.rb @@ -39,13 +39,19 @@ describe "Groups", "routing" do describe 'legacy redirection' do describe 'labels' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/labels", "/groups/complex.group-namegit/-/labels/" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/labels", "/groups/complex.group-namegit/-/labels" do let(:resource) { create(:group, parent: group, path: 'labels') } end + + context 'when requesting JSON' do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/labels.json", "/groups/complex.group-namegit/-/labels.json" do + let(:resource) { create(:group, parent: group, path: 'labels') } + end + end end describe 'group_members' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/group_members", "/groups/complex.group-namegit/-/group_members/" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/group_members", "/groups/complex.group-namegit/-/group_members" do let(:resource) { create(:group, parent: group, path: 'group_members') } end end @@ -60,7 +66,7 @@ describe "Groups", "routing" do end describe 'milestones' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones", "/groups/complex.group-namegit/-/milestones/" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones", "/groups/complex.group-namegit/-/milestones" do let(:resource) { create(:group, parent: group, path: 'milestones') } end @@ -76,18 +82,18 @@ describe "Groups", "routing" do end context 'with a query string' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones?hello=world", "/groups/complex.group-namegit/-/milestones/?hello=world" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones?hello=world", "/groups/complex.group-namegit/-/milestones?hello=world" do let(:resource) { create(:group, parent: group, path: 'milestones') } end - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones?milestones=/milestones", "/groups/complex.group-namegit/-/milestones/?milestones=/milestones" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/milestones?milestones=/milestones", "/groups/complex.group-namegit/-/milestones?milestones=/milestones" do let(:resource) { create(:group, parent: group, path: 'milestones') } end end end describe 'edit' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/edit", "/groups/complex.group-namegit/-/edit/" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/edit", "/groups/complex.group-namegit/-/edit" do let(:resource) do pending('still rejected because of the wildcard reserved word') create(:group, parent: group, path: 'edit') @@ -96,29 +102,29 @@ describe "Groups", "routing" do end describe 'issues' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/issues", "/groups/complex.group-namegit/-/issues/" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/issues", "/groups/complex.group-namegit/-/issues" do let(:resource) { create(:group, parent: group, path: 'issues') } end end describe 'merge_requests' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/merge_requests", "/groups/complex.group-namegit/-/merge_requests/" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/merge_requests", "/groups/complex.group-namegit/-/merge_requests" do let(:resource) { create(:group, parent: group, path: 'merge_requests') } end end describe 'projects' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/projects", "/groups/complex.group-namegit/-/projects/" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/projects", "/groups/complex.group-namegit/-/projects" do let(:resource) { create(:group, parent: group, path: 'projects') } end end describe 'activity' do - it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/activity", "/groups/complex.group-namegit/-/activity/" do + it_behaves_like 'redirecting a legacy path', "/groups/complex.group-namegit/activity", "/groups/complex.group-namegit/-/activity" do let(:resource) { create(:group, parent: group, path: 'activity') } end - it_behaves_like 'redirecting a legacy path', "/groups/activity/activity", "/groups/activity/-/activity/" do + it_behaves_like 'redirecting a legacy path', "/groups/activity/activity", "/groups/activity/-/activity" do let!(:parent) { create(:group, path: 'activity') } let(:resource) { create(:group, parent: parent, path: 'activity') } end diff --git a/spec/services/base_count_service_spec.rb b/spec/services/base_count_service_spec.rb new file mode 100644 index 00000000000..5ec8ed0976d --- /dev/null +++ b/spec/services/base_count_service_spec.rb @@ -0,0 +1,80 @@ +require 'spec_helper' + +describe BaseCountService, :use_clean_rails_memory_store_caching do + let(:service) { described_class.new } + + describe '#relation_for_count' do + it 'raises NotImplementedError' do + expect { service.relation_for_count }.to raise_error(NotImplementedError) + end + end + + describe '#count' do + it 'returns the number of values' do + expect(service) + .to receive(:cache_key) + .and_return('foo') + + expect(service) + .to receive(:uncached_count) + .and_return(5) + + expect(service.count).to eq(5) + end + end + + describe '#uncached_count' do + it 'returns the uncached number of values' do + expect(service) + .to receive(:relation_for_count) + .and_return(double(:relation, count: 5)) + + expect(service.uncached_count).to eq(5) + end + end + + describe '#refresh_cache' do + it 'refreshes the cache' do + allow(service) + .to receive(:cache_key) + .and_return('foo') + + allow(service) + .to receive(:uncached_count) + .and_return(4) + + service.refresh_cache + + expect(Rails.cache.fetch(service.cache_key, raw: service.raw?)).to eq(4) + end + end + + describe '#delete_cache' do + it 'deletes the cache' do + allow(service) + .to receive(:cache_key) + .and_return('foo') + + allow(service) + .to receive(:uncached_count) + .and_return(4) + + service.refresh_cache + service.delete_cache + + expect(Rails.cache.fetch(service.cache_key, raw: service.raw?)).to be_nil + end + end + + describe '#raw?' do + it 'returns false' do + expect(service.raw?).to eq(false) + end + end + + describe '#cache_key' do + it 'raises NotImplementedError' do + expect { service.cache_key }.to raise_error(NotImplementedError) + end + end +end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 2459f371a91..2b1337bee7e 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -42,6 +42,18 @@ describe Projects::TransferService do expect(service).to receive(:execute_system_hooks) end end + + it 'disk path has moved' do + old_path = project.repository.disk_path + old_full_path = project.repository.full_path + + transfer_project(project, user, group) + + expect(project.repository.disk_path).not_to eq(old_path) + expect(project.repository.full_path).not_to eq(old_full_path) + expect(project.disk_path).not_to eq(old_path) + expect(project.disk_path).to start_with(group.path) + end end context 'when transfer fails' do @@ -188,6 +200,26 @@ describe Projects::TransferService do end end + context 'when hashed storage in use' do + let(:hashed_project) { create(:project, :repository, :hashed, namespace: user.namespace) } + + before do + group.add_owner(user) + end + + it 'does not move the directory' do + old_path = hashed_project.repository.disk_path + old_full_path = hashed_project.repository.full_path + + transfer_project(hashed_project, user, group) + project.reload + + expect(hashed_project.repository.disk_path).to eq(old_path) + expect(hashed_project.repository.full_path).to eq(old_full_path) + expect(hashed_project.disk_path).to eq(old_path) + end + end + describe 'refreshing project authorizations' do let(:group) { create(:group) } let(:owner) { project.namespace.owner } diff --git a/spec/services/users/keys_count_service_spec.rb b/spec/services/users/keys_count_service_spec.rb new file mode 100644 index 00000000000..a188cf86772 --- /dev/null +++ b/spec/services/users/keys_count_service_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Users::KeysCountService, :use_clean_rails_memory_store_caching do + let(:user) { create(:user) } + let(:service) { described_class.new(user) } + + describe '#count' do + before do + create(:personal_key, user: user) + end + + it 'returns the number of SSH keys as an Integer' do + expect(service.count).to eq(1) + end + + it 'caches the number of keys in Redis' do + service.delete_cache + + recorder = ActiveRecord::QueryRecorder.new do + 2.times { service.count } + end + + expect(recorder.count).to eq(1) + end + end + + describe '#refresh_cache' do + it 'refreshes the Redis cache' do + Rails.cache.write(service.cache_key, 10) + service.refresh_cache + + expect(Rails.cache.fetch(service.cache_key, raw: true)).to be_zero + end + end + + describe '#delete_cache' do + it 'removes the cache' do + service.count + service.delete_cache + + expect(Rails.cache.fetch(service.cache_key, raw: true)).to be_nil + end + end + + describe '#uncached_count' do + it 'returns the number of SSH keys' do + expect(service.uncached_count).to be_zero + end + + it 'does not cache the number of keys' do + recorder = ActiveRecord::QueryRecorder.new do + 2.times { service.uncached_count } + end + + expect(recorder.count).to be > 0 + end + end + + describe '#cache_key' do + it 'returns the cache key' do + expect(service.cache_key).to eq("users/key-count-service/#{user.id}") + end + end +end |
