diff options
41 files changed, 1259 insertions, 494 deletions
diff --git a/Dangerfile b/Dangerfile index 7879c14b31e..c428b55f33a 100644 --- a/Dangerfile +++ b/Dangerfile @@ -7,8 +7,12 @@ danger.import_plugin('danger/plugins/helper.rb') danger.import_plugin('danger/plugins/roulette.rb') danger.import_plugin('danger/plugins/changelog.rb') -unless helper.release_automation? - GitlabDanger.new(helper.gitlab_helper).rule_names.each do |file| - danger.import_dangerfile(path: File.join('danger', file)) - end +return if helper.release_automation? + +gitlab_danger = GitlabDanger.new(helper.gitlab_helper) + +gitlab_danger.rule_names.each do |file| + danger.import_dangerfile(path: File.join('danger', file)) end + +markdown("**If needed, you can retry the [`danger-review` job](#{ENV['CI_JOB_URL']}) that generated this comment.**") if gitlab_danger.ci? diff --git a/app/assets/javascripts/ide/components/repo_editor.vue b/app/assets/javascripts/ide/components/repo_editor.vue index 2a8ed076ae2..55290ba7779 100644 --- a/app/assets/javascripts/ide/components/repo_editor.vue +++ b/app/assets/javascripts/ide/components/repo_editor.vue @@ -14,7 +14,6 @@ import Editor from '../lib/editor'; import FileTemplatesBar from './file_templates/bar.vue'; import { __ } from '~/locale'; import { extractMarkdownImagesFromEntries } from '../stores/utils'; -import { addFinalNewline } from '../utils'; export default { components: { @@ -32,7 +31,6 @@ export default { return { content: '', images: {}, - addFinalNewline: true, }; }, computed: { @@ -253,10 +251,8 @@ export default { const monacoModel = model.getModel(); const content = monacoModel.getValue(); - this.changeFileContent({ - path: file.path, - content: this.addFinalNewline ? addFinalNewline(content, monacoModel.getEOL()) : content, - }); + this.changeFileContent({ path: file.path, content }); + this.setFileEOL({ eol: this.model.eol }); }); // Handle Cursor Position diff --git a/app/assets/javascripts/ide/lib/common/model.js b/app/assets/javascripts/ide/lib/common/model.js index a15f04075d9..4c743076fb2 100644 --- a/app/assets/javascripts/ide/lib/common/model.js +++ b/app/assets/javascripts/ide/lib/common/model.js @@ -1,6 +1,8 @@ import { editor as monacoEditor, Uri } from 'monaco-editor'; import Disposable from './disposable'; import eventHub from '../../eventhub'; +import { trimTrailingWhitespace, insertFinalNewline } from '../../utils'; +import { defaultModelOptions } from '../editor_options'; export default class Model { constructor(file, head = null) { @@ -8,6 +10,7 @@ export default class Model { this.file = file; this.head = head; this.content = file.content !== '' || file.deleted ? file.content : file.raw; + this.options = { ...defaultModelOptions }; this.disposable.add( (this.originalModel = monacoEditor.createModel( @@ -94,8 +97,32 @@ export default class Model { this.getModel().setValue(content); } + updateOptions(obj = {}) { + Object.assign(this.options, obj); + this.model.updateOptions(obj); + this.applyCustomOptions(); + } + + applyCustomOptions() { + this.updateNewContent( + Object.entries(this.options).reduce((content, [key, value]) => { + switch (key) { + case 'endOfLine': + this.model.pushEOL(value); + return this.model.getValue(); + case 'insertFinalNewline': + return value ? insertFinalNewline(content) : content; + case 'trimTrailingWhitespace': + return value ? trimTrailingWhitespace(content) : content; + default: + return content; + } + }, this.model.getValue()), + ); + } + dispose() { - this.disposable.dispose(); + if (!this.model.isDisposed()) this.applyCustomOptions(); this.events.forEach(cb => { if (typeof cb === 'function') cb(); @@ -106,5 +133,7 @@ export default class Model { eventHub.$off(`editor.update.model.dispose.${this.file.key}`, this.dispose); eventHub.$off(`editor.update.model.content.${this.file.key}`, this.updateContent); eventHub.$off(`editor.update.model.new.content.${this.file.key}`, this.updateNewContent); + + this.disposable.dispose(); } } diff --git a/app/assets/javascripts/ide/lib/diff/controller.js b/app/assets/javascripts/ide/lib/diff/controller.js index 234a7f903a1..35fcda6a6c5 100644 --- a/app/assets/javascripts/ide/lib/diff/controller.js +++ b/app/assets/javascripts/ide/lib/diff/controller.js @@ -50,10 +50,15 @@ export default class DirtyDiffController { } computeDiff(model) { + const originalModel = model.getOriginalModel(); + const newModel = model.getModel(); + + if (originalModel.isDisposed() || newModel.isDisposed()) return; + this.dirtyDiffWorker.postMessage({ path: model.path, - originalContent: model.getOriginalModel().getValue(), - newContent: model.getModel().getValue(), + originalContent: originalModel.getValue(), + newContent: newModel.getValue(), }); } diff --git a/app/assets/javascripts/ide/lib/diff/diff.js b/app/assets/javascripts/ide/lib/diff/diff.js index 29e29d7fcd3..3a456b7c4d6 100644 --- a/app/assets/javascripts/ide/lib/diff/diff.js +++ b/app/assets/javascripts/ide/lib/diff/diff.js @@ -1,8 +1,15 @@ import { diffLines } from 'diff'; +import { defaultDiffOptions } from '../editor_options'; +// See: https://gitlab.com/gitlab-org/frontend/rfcs/-/issues/20 // eslint-disable-next-line import/prefer-default-export export const computeDiff = (originalContent, newContent) => { - const changes = diffLines(originalContent, newContent); + // prevent EOL changes from highlighting the entire file + const changes = diffLines( + originalContent.replace(/\r\n/g, '\n'), + newContent.replace(/\r\n/g, '\n'), + defaultDiffOptions, + ); let lineNumber = 1; return changes.reduce((acc, change) => { diff --git a/app/assets/javascripts/ide/lib/editor.js b/app/assets/javascripts/ide/lib/editor.js index 25224abd77c..141efd86884 100644 --- a/app/assets/javascripts/ide/lib/editor.js +++ b/app/assets/javascripts/ide/lib/editor.js @@ -5,7 +5,7 @@ import DecorationsController from './decorations/controller'; import DirtyDiffController from './diff/controller'; import Disposable from './common/disposable'; import ModelManager from './common/model_manager'; -import editorOptions, { defaultEditorOptions } from './editor_options'; +import { editorOptions, defaultEditorOptions, defaultDiffEditorOptions } from './editor_options'; import { themes } from './themes'; import languages from './languages'; import keymap from './keymap.json'; @@ -73,8 +73,7 @@ export default class Editor { this.disposable.add( (this.instance = monacoEditor.createDiffEditor(domElement, { ...this.options, - quickSuggestions: false, - occurrencesHighlight: false, + ...defaultDiffEditorOptions, renderSideBySide: Editor.renderSideBySide(domElement), readOnly, renderLineHighlight: readOnly ? 'all' : 'none', diff --git a/app/assets/javascripts/ide/lib/editor_options.js b/app/assets/javascripts/ide/lib/editor_options.js index dac2a8e8b51..422b2b9776c 100644 --- a/app/assets/javascripts/ide/lib/editor_options.js +++ b/app/assets/javascripts/ide/lib/editor_options.js @@ -9,7 +9,23 @@ export const defaultEditorOptions = { wordWrap: 'on', }; -export default [ +export const defaultDiffOptions = { + ignoreWhitespace: false, +}; + +export const defaultDiffEditorOptions = { + quickSuggestions: false, + occurrencesHighlight: false, + ignoreTrimWhitespace: false, +}; + +export const defaultModelOptions = { + endOfLine: 0, + insertFinalNewline: true, + trimTrailingWhitespace: false, +}; + +export const editorOptions = [ { readOnly: model => Boolean(model.file.file_lock), quickSuggestions: model => !(model.language === 'markdown'), diff --git a/app/assets/javascripts/ide/utils.js b/app/assets/javascripts/ide/utils.js index 64be1f59887..2d1a7c24c59 100644 --- a/app/assets/javascripts/ide/utils.js +++ b/app/assets/javascripts/ide/utils.js @@ -77,7 +77,11 @@ export function registerLanguages(def, ...defs) { export const otherSide = side => (side === SIDE_RIGHT ? SIDE_LEFT : SIDE_RIGHT); -export function addFinalNewline(content, eol = '\n') { +export function trimTrailingWhitespace(content) { + return content.replace(/[^\S\r\n]+$/gm, ''); +} + +export function insertFinalNewline(content, eol = '\n') { return content.slice(-eol.length) !== eol ? `${content}${eol}` : content; } diff --git a/app/controllers/admin/runners_controller.rb b/app/controllers/admin/runners_controller.rb index 4639d8adfe0..2449fa3128c 100644 --- a/app/controllers/admin/runners_controller.rb +++ b/app/controllers/admin/runners_controller.rb @@ -4,7 +4,7 @@ class Admin::RunnersController < Admin::ApplicationController before_action :runner, except: [:index, :tag_list] def index - finder = Admin::RunnersFinder.new(params: params) + finder = Ci::RunnersFinder.new(current_user: current_user, params: params) @runners = finder.execute @active_runners_count = Ci::Runner.online.count @sort = finder.sort_key diff --git a/app/finders/admin/runners_finder.rb b/app/finders/admin/runners_finder.rb deleted file mode 100644 index b2799565f57..00000000000 --- a/app/finders/admin/runners_finder.rb +++ /dev/null @@ -1,71 +0,0 @@ -# frozen_string_literal: true - -class Admin::RunnersFinder < UnionFinder - NUMBER_OF_RUNNERS_PER_PAGE = 30 - - def initialize(params:) - @params = params - end - - def execute - search! - filter_by_status! - filter_by_runner_type! - filter_by_tag_list! - sort! - paginate! - - @runners.with_tags - end - - def sort_key - if @params[:sort] == 'contacted_asc' - 'contacted_asc' - else - 'created_date' - end - end - - private - - def search! - @runners = - if @params[:search].present? - Ci::Runner.search(@params[:search]) - else - Ci::Runner.all - end - end - - def filter_by_status! - filter_by!(:status_status, Ci::Runner::AVAILABLE_STATUSES) - end - - def filter_by_runner_type! - filter_by!(:type_type, Ci::Runner::AVAILABLE_TYPES) - end - - def filter_by_tag_list! - tag_list = @params[:tag_name].presence - - if tag_list - @runners = @runners.tagged_with(tag_list) - end - end - - def sort! - @runners = @runners.order_by(sort_key) - end - - def paginate! - @runners = @runners.page(@params[:page]).per(NUMBER_OF_RUNNERS_PER_PAGE) - end - - def filter_by!(scope_name, available_scopes) - scope = @params[scope_name] - - if scope.present? && available_scopes.include?(scope) - @runners = @runners.public_send(scope) # rubocop:disable GitlabSecurity/PublicSend - end - end -end diff --git a/app/finders/ci/runners_finder.rb b/app/finders/ci/runners_finder.rb new file mode 100644 index 00000000000..54d9d9522e0 --- /dev/null +++ b/app/finders/ci/runners_finder.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +module Ci + class RunnersFinder < UnionFinder + include Gitlab::Allowable + + NUMBER_OF_RUNNERS_PER_PAGE = 30 + + def initialize(current_user:, group: nil, params:) + @params = params + @group = group + @current_user = current_user + end + + def execute + search! + filter_by_status! + filter_by_runner_type! + filter_by_tag_list! + sort! + paginate! + + @runners.with_tags + + rescue Gitlab::Access::AccessDeniedError + Ci::Runner.none + end + + def sort_key + if @params[:sort] == 'contacted_asc' + 'contacted_asc' + else + 'created_date' + end + end + + private + + def search! + @group ? group_runners : all_runners + + @runners = @runners.search(@params[:search]) if @params[:search].present? + end + + def all_runners + raise Gitlab::Access::AccessDeniedError unless @current_user&.admin? + + @runners = Ci::Runner.all + end + + def group_runners + raise Gitlab::Access::AccessDeniedError unless can?(@current_user, :admin_group, @group) + + # Getting all runners from the group itself and all its descendants + descentant_projects = Project.for_group_and_its_subgroups(@group) + + @runners = Ci::Runner.belonging_to_group_or_project(@group.self_and_descendants, descentant_projects) + end + + def filter_by_status! + filter_by!(:status_status, Ci::Runner::AVAILABLE_STATUSES) + end + + def filter_by_runner_type! + filter_by!(:type_type, Ci::Runner::AVAILABLE_TYPES) + end + + def filter_by_tag_list! + tag_list = @params[:tag_name].presence + + if tag_list + @runners = @runners.tagged_with(tag_list) + end + end + + def sort! + @runners = @runners.order_by(sort_key) + end + + def paginate! + @runners = @runners.page(@params[:page]).per(NUMBER_OF_RUNNERS_PER_PAGE) + end + + def filter_by!(scope_name, available_scopes) + scope = @params[scope_name] + + if scope.present? && available_scopes.include?(scope) + @runners = @runners.public_send(scope) # rubocop:disable GitlabSecurity/PublicSend + end + end + end +end diff --git a/app/finders/uploader_finder.rb b/app/finders/uploader_finder.rb new file mode 100644 index 00000000000..0d1de0d56fd --- /dev/null +++ b/app/finders/uploader_finder.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class UploaderFinder + # Instantiates a a new FileUploader + # FileUploader can be opened via .open agnostic of storage type + # Arguments correspond to Upload.secret, Upload.model_type and Upload.file_path + # Returns a FileUploader with uploaded file retrieved into the object state + def initialize(project, secret, file_path) + @project = project + @secret = secret + @file_path = file_path + end + + def execute + prevent_path_traversal_attack! + retrieve_file_state! + + uploader + rescue ::Gitlab::Utils::PathTraversalAttackError + nil # no-op if for incorrect files + end + + def prevent_path_traversal_attack! + Gitlab::Utils.check_path_traversal!(@file_path) + end + + def retrieve_file_state! + uploader.retrieve_from_store!(@file_path) + end + + def uploader + @uploader ||= FileUploader.new(@project, secret: @secret) + end +end diff --git a/app/models/ci/instance_variable.rb b/app/models/ci/instance_variable.rb index c674f76d229..557f3a63280 100644 --- a/app/models/ci/instance_variable.rb +++ b/app/models/ci/instance_variable.rb @@ -3,6 +3,7 @@ module Ci class InstanceVariable < ApplicationRecord extend Gitlab::Ci::Model + extend Gitlab::ProcessMemoryCache::Helper include Ci::NewHasVariable include Ci::Maskable @@ -13,7 +14,8 @@ module Ci } scope :unprotected, -> { where(protected: false) } - after_commit { self.class.touch_redis_cache_timestamp } + + after_commit { self.class.invalidate_memory_cache(:ci_instance_variable_data) } class << self def all_cached @@ -24,10 +26,6 @@ module Ci cached_data[:unprotected] end - def touch_redis_cache_timestamp(time = Time.current.to_f) - shared_backend.write(:ci_instance_variable_changed_at, time) - end - private def cached_data @@ -37,40 +35,6 @@ module Ci { all: all_records, unprotected: all_records.reject(&:protected?) } end end - - def fetch_memory_cache(key, &payload) - cache = process_backend.read(key) - - if cache && !stale_cache?(cache) - cache[:data] - else - store_cache(key, &payload) - end - end - - def stale_cache?(cache_info) - shared_timestamp = shared_backend.read(:ci_instance_variable_changed_at) - return true unless shared_timestamp - - shared_timestamp.to_f > cache_info[:cached_at].to_f - end - - def store_cache(key) - data = yield - time = Time.current.to_f - - process_backend.write(key, data: data, cached_at: time) - touch_redis_cache_timestamp(time) - data - end - - def shared_backend - Rails.cache - end - - def process_backend - Gitlab::ProcessMemoryCache.cache_backend - end end end end diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 03ff952b435..30ea383bd73 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -81,6 +81,17 @@ module Ci joins(:runner_namespaces).where(ci_runner_namespaces: { namespace_id: groups }) } + scope :belonging_to_group_or_project, -> (group_id, project_id) { + groups = ::Group.where(id: group_id) + + group_runners = joins(:runner_namespaces).where(ci_runner_namespaces: { namespace_id: groups }) + project_runners = joins(:runner_projects).where(ci_runner_projects: { project_id: project_id }) + + union_sql = ::Gitlab::SQL::Union.new([group_runners, project_runners]).to_sql + + from("(#{union_sql}) #{table_name}") + } + scope :belonging_to_parent_group_of_project, -> (project_id) { project_groups = ::Group.joins(:projects).where(projects: { id: project_id }) hierarchy_groups = Gitlab::ObjectHierarchy.new(project_groups).base_and_ancestors diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index c738493507f..409e602f306 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -319,7 +319,7 @@ - if project_nav_tab? :snippets = nav_link(controller: :snippets) do - = link_to project_snippets_path(@project), class: 'shortcuts-snippets' do + = link_to project_snippets_path(@project), class: 'shortcuts-snippets', data: { qa_selector: 'snippets_link' } do .nav-icon-container = sprite_icon('snippet') %span.nav-item-name diff --git a/app/views/shared/empty_states/_snippets.html.haml b/app/views/shared/empty_states/_snippets.html.haml index efd9bceedc5..db8da50d868 100644 --- a/app/views/shared/empty_states/_snippets.html.haml +++ b/app/views/shared/empty_states/_snippets.html.haml @@ -3,7 +3,7 @@ .row.empty-state.mt-0 .col-12 .svg-content - = image_tag 'illustrations/snippets_empty.svg' + = image_tag 'illustrations/snippets_empty.svg', data: { qa_selector: 'svg_content' } .text-content.text-center.pt-0 - if current_user %h4 @@ -12,7 +12,7 @@ = s_('SnippetsEmptyState|Store, share, and embed small pieces of code and text.') .mt-2< - if button_path - = link_to s_('SnippetsEmptyState|New snippet'), button_path, class: 'btn btn-success', title: s_('SnippetsEmptyState|New snippet'), id: 'new_snippet_link' + = link_to s_('SnippetsEmptyState|New snippet'), button_path, class: 'btn btn-success', title: s_('SnippetsEmptyState|New snippet'), id: 'new_snippet_link', data: { qa_selector: 'create_first_snippet_link' } = link_to s_('SnippetsEmptyState|Documentation'), help_page_path('user/snippets.md'), class: 'btn btn-default', title: s_('SnippetsEmptyState|Documentation') - else %h4.text-center= s_('SnippetsEmptyState|There are no snippets to show.') diff --git a/changelogs/unreleased/add-group-runners-finder.yml b/changelogs/unreleased/add-group-runners-finder.yml new file mode 100644 index 00000000000..a90eb2c2e42 --- /dev/null +++ b/changelogs/unreleased/add-group-runners-finder.yml @@ -0,0 +1,5 @@ +--- +title: Add finder for group-level runners +merge_request: 29283 +author: Arthur de Lapertosa Lisboa +type: added diff --git a/doc/user/project/status_page/index.md b/doc/user/project/status_page/index.md index 8ebfb638894..c4d8a71320e 100644 --- a/doc/user/project/status_page/index.md +++ b/doc/user/project/status_page/index.md @@ -71,7 +71,8 @@ The incident detail page shows detailed information about a particular incident. - Status on the incident, including when the incident was last updated. - The incident title, including any emojis. -- The description of the incident, including emojis and static images. +- The description of the incident, including emojis. +- Any file attachments provided in the incident description or comments with a valid image extension. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/205166) in GitLab 13.1. - A chronological ordered list of updates to the incident.  @@ -108,3 +109,15 @@ Anyone with access to view the Issue can add an Emoji Award to a comment, so you ### Changing the Incident status To change the incident status from `open` to `closed`, close the incident issue within GitLab. This will then be updated shortly on the Status Page website. + +## Attachment storage + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/205166) in GitLab 13.1. + +Beginning with GitLab 13.1, files attached to incident issue descriptions or +comments are published and unpublished to the status page storage as part of +the [publication flow](#how-it-works). + +### Limit + +Only 5000 attachments per issue will be transferred to the status page. diff --git a/lib/api/validations/validators/file_path.rb b/lib/api/validations/validators/file_path.rb index 93a20e5bf7d..fee71373170 100644 --- a/lib/api/validations/validators/file_path.rb +++ b/lib/api/validations/validators/file_path.rb @@ -8,7 +8,7 @@ module API path = params[attr_name] Gitlab::Utils.check_path_traversal!(path) - rescue StandardError + rescue ::Gitlab::Utils::PathTraversalAttackError raise Grape::Exceptions::Validation, params: [@scope.full_name(attr_name)], message: "should be a valid file path" end diff --git a/lib/gitlab/gfm/uploads_rewriter.rb b/lib/gitlab/gfm/uploads_rewriter.rb index 23af0a9bb18..08321d5fda6 100644 --- a/lib/gitlab/gfm/uploads_rewriter.rb +++ b/lib/gitlab/gfm/uploads_rewriter.rb @@ -22,9 +22,10 @@ module Gitlab return @text unless needs_rewrite? @text.gsub(@pattern) do |markdown| - Gitlab::Utils.check_path_traversal!($~[:file]) + file = find_file($~[:secret], $~[:file]) + # No file will be returned for a path traversal + next if file.nil? - file = find_file(@source_project, $~[:secret], $~[:file]) break markdown unless file.try(:exists?) klass = target_parent.is_a?(Namespace) ? NamespaceFileUploader : FileUploader @@ -47,7 +48,7 @@ module Gitlab def files referenced_files = @text.scan(@pattern).map do - find_file(@source_project, $~[:secret], $~[:file]) + find_file($~[:secret], $~[:file]) end referenced_files.compact.select(&:exists?) @@ -57,12 +58,8 @@ module Gitlab markdown.starts_with?("!") end - private - - def find_file(project, secret, file) - uploader = FileUploader.new(project, secret: secret) - uploader.retrieve_from_store!(file) - uploader + def find_file(secret, file_name) + UploaderFinder.new(@source_project, secret, file_name).execute end end end diff --git a/lib/gitlab/process_memory_cache/helper.rb b/lib/gitlab/process_memory_cache/helper.rb new file mode 100644 index 00000000000..ee4b81a9a19 --- /dev/null +++ b/lib/gitlab/process_memory_cache/helper.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module Gitlab + class ProcessMemoryCache + module Helper + def fetch_memory_cache(key, &payload) + cache = cache_backend.read(key) + + if cache && !stale_cache?(key, cache) + cache[:data] + else + store_cache(key, &payload) + end + end + + def invalidate_memory_cache(key) + touch_cache_timestamp(key) + end + + private + + def touch_cache_timestamp(key, time = Time.current.to_f) + shared_backend.write(key, time) + end + + def stale_cache?(key, cache_info) + shared_timestamp = shared_backend.read(key) + return true unless shared_timestamp + + shared_timestamp.to_f > cache_info[:cached_at].to_f + end + + def store_cache(key) + data = yield + time = Time.current.to_f + + cache_backend.write(key, data: data, cached_at: time) + touch_cache_timestamp(key, time) + data + end + + def shared_backend + Rails.cache + end + + def cache_backend + ::Gitlab::ProcessMemoryCache.cache_backend + end + end + end +end diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb index d46601fa2e8..5aafcb5bb8a 100644 --- a/lib/gitlab/utils.rb +++ b/lib/gitlab/utils.rb @@ -3,6 +3,7 @@ module Gitlab module Utils extend self + PathTraversalAttackError ||= Class.new(StandardError) # Ensure that the relative path will not traverse outside the base directory # We url decode the path to avoid passing invalid paths forward in url encoded format. @@ -17,7 +18,7 @@ module Gitlab path.end_with?("#{File::SEPARATOR}..") || (!allowed_absolute && Pathname.new(path).absolute?) - raise StandardError.new("Invalid path") + raise PathTraversalAttackError.new('Invalid path') end path @@ -86,6 +86,7 @@ module QA autoload :Snippet, 'qa/resource/snippet' autoload :Tag, 'qa/resource/tag' autoload :ProjectMember, 'qa/resource/project_member' + autoload :ProjectSnippet, 'qa/resource/project_snippet' autoload :UserGPG, 'qa/resource/user_gpg' autoload :Visibility, 'qa/resource/visibility' @@ -332,6 +333,10 @@ module QA module WebIDE autoload :Edit, 'qa/page/project/web_ide/edit' end + + module Snippet + autoload :New, 'qa/page/project/snippet/new' + end end module Profile diff --git a/qa/qa/page/project/menu.rb b/qa/qa/page/project/menu.rb index 5967213a52b..e2984bf72bd 100644 --- a/qa/qa/page/project/menu.rb +++ b/qa/qa/page/project/menu.rb @@ -16,6 +16,7 @@ module QA element :activity_link element :merge_requests_link element :wiki_link + element :snippets_link end def click_merge_requests @@ -35,6 +36,12 @@ module QA click_element(:activity_link) end end + + def click_snippets + within_sidebar do + click_element(:snippets_link) + end + end end end end diff --git a/qa/qa/page/project/snippet/new.rb b/qa/qa/page/project/snippet/new.rb new file mode 100644 index 00000000000..1463dfc2c7f --- /dev/null +++ b/qa/qa/page/project/snippet/new.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module QA + module Page + module Project + module Snippet + class New < Page::Dashboard::Snippet::New + include Component::LazyLoader + view 'app/views/shared/empty_states/_snippets.html.haml' do + element :create_first_snippet_link + element :svg_content + end + + def click_create_first_snippet + finished_loading? + # The svg takes a fraction of a second to load after which the + # "New snippet" button shifts up a bit. This can cause + # webdriver to miss the hit so we wait for the svg to load before + # clicking the button. + within_element(:svg_content) do + has_element?(:js_lazy_loaded) + end + click_element(:create_first_snippet_link) + end + end + end + end + end +end diff --git a/qa/qa/resource/project_snippet.rb b/qa/qa/resource/project_snippet.rb new file mode 100644 index 00000000000..ce4be6445f1 --- /dev/null +++ b/qa/qa/resource/project_snippet.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module QA + module Resource + class ProjectSnippet < Snippet + attribute :project do + Project.fabricate_via_api! do |resource| + resource.name = 'project-with-snippets' + end + end + + def fabricate! + project.visit! + + Page::Project::Menu.perform { |sidebar| sidebar.click_snippets } + + Page::Project::Snippet::New.perform do |new_snippet| + new_snippet.click_create_first_snippet + new_snippet.fill_title(@title) + new_snippet.fill_description(@description) + new_snippet.set_visibility(@visibility) + new_snippet.fill_file_name(@file_name) + new_snippet.fill_file_content(@file_content) + new_snippet.click_create_snippet_button + end + end + end + end +end diff --git a/qa/qa/specs/features/browser_ui/3_create/snippet/clone_push_pull_project_snippet_spec.rb b/qa/qa/specs/features/browser_ui/3_create/snippet/clone_push_pull_project_snippet_spec.rb new file mode 100644 index 00000000000..a3011550db8 --- /dev/null +++ b/qa/qa/specs/features/browser_ui/3_create/snippet/clone_push_pull_project_snippet_spec.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +module QA + context 'Create' do + describe 'Version control for project snippets' do + let(:new_file) { 'new_snippet_file' } + let(:changed_content) { 'changes' } + let(:commit_message) { 'Changes to snippets' } + let(:added_content) { 'updated ' } + let(:branch_name) { 'master' } + + let(:snippet) do + Resource::ProjectSnippet.fabricate! do |snippet| + snippet.file_name = new_file + end + end + + let(:ssh_key) do + Resource::SSHKey.fabricate_via_api! do |resource| + resource.title = "my key title #{Time.now.to_f}" + end + end + + let(:repository_uri_http) do + snippet + Page::Dashboard::Snippet::Show.perform(&:get_repository_uri_http) + end + + let(:repository_uri_ssh) do + ssh_key + snippet + Page::Dashboard::Snippet::Show.perform(&:get_repository_uri_ssh) + end + + before do + Flow::Login.sign_in + end + + it 'clones, pushes, and pulls a project snippet over HTTP, edits via UI' do + Resource::Repository::Push.fabricate! do |push| + push.repository_http_uri = repository_uri_http + push.file_name = new_file + push.file_content = changed_content + push.commit_message = commit_message + push.new_branch = false + end + + page.refresh + verify_changes_in_ui + + Page::Dashboard::Snippet::Show.perform(&:click_edit_button) + + Page::Dashboard::Snippet::Edit.perform do |snippet| + snippet.add_to_file_content(added_content) + snippet.save_changes + end + + Git::Repository.perform do |repository| + repository.init_repository + repository.pull(repository_uri_http, branch_name) + + expect(repository.commits.size).to eq 3 + expect(repository.commits.first).to include 'Update snippet' + expect(repository.file_content(new_file)).to include "#{added_content}#{changed_content}" + end + end + + it 'clones, pushes, and pulls a project snippet over SSH, deletes via UI' do + Resource::Repository::Push.fabricate! do |push| + push.repository_ssh_uri = repository_uri_ssh + push.ssh_key = ssh_key + push.file_name = new_file + push.file_content = changed_content + push.commit_message = commit_message + push.new_branch = false + end + + page.refresh + verify_changes_in_ui + Page::Dashboard::Snippet::Show.perform(&:click_delete_button) + + # attempt to pull a deleted snippet, get a missing repository error + Git::Repository.perform do |repository| + repository.uri = repository_uri_ssh + repository.use_ssh_key(ssh_key) + repository.init_repository + + expect { repository.pull(repository_uri_ssh, branch_name) } + .to raise_error(QA::Git::Repository::RepositoryCommandError, /[fatal: Could not read from remote repository.]+/) + end + end + + def verify_changes_in_ui + Page::Dashboard::Snippet::Show.perform do |snippet| + expect(snippet).to have_file_name(new_file) + expect(snippet).to have_file_content(changed_content) + end + end + end + end +end diff --git a/spec/finders/admin/runners_finder_spec.rb b/spec/finders/admin/runners_finder_spec.rb deleted file mode 100644 index 94ccb398801..00000000000 --- a/spec/finders/admin/runners_finder_spec.rb +++ /dev/null @@ -1,81 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Admin::RunnersFinder do - describe '#execute' do - context 'with empty params' do - it 'returns all runners' do - runner1 = create :ci_runner, active: true - runner2 = create :ci_runner, active: false - - expect(described_class.new(params: {}).execute).to match_array [runner1, runner2] - end - end - - context 'filter by search term' do - it 'calls Ci::Runner.search' do - expect(Ci::Runner).to receive(:search).with('term').and_call_original - - described_class.new(params: { search: 'term' }).execute - end - end - - context 'filter by status' do - it 'calls the corresponding scope on Ci::Runner' do - expect(Ci::Runner).to receive(:paused).and_call_original - - described_class.new(params: { status_status: 'paused' }).execute - end - end - - context 'filter by runner type' do - it 'calls the corresponding scope on Ci::Runner' do - expect(Ci::Runner).to receive(:project_type).and_call_original - - described_class.new(params: { type_type: 'project_type' }).execute - end - end - - context 'filter by tag_name' do - it 'calls the corresponding scope on Ci::Runner' do - expect(Ci::Runner).to receive(:tagged_with).with(%w[tag1 tag2]).and_call_original - - described_class.new(params: { tag_name: %w[tag1 tag2] }).execute - end - end - - context 'sort' do - context 'without sort param' do - it 'sorts by created_at' do - runner1 = create :ci_runner, created_at: '2018-07-12 07:00' - runner2 = create :ci_runner, created_at: '2018-07-12 08:00' - runner3 = create :ci_runner, created_at: '2018-07-12 09:00' - - expect(described_class.new(params: {}).execute).to eq [runner3, runner2, runner1] - end - end - - context 'with sort param' do - it 'sorts by specified attribute' do - runner1 = create :ci_runner, contacted_at: 1.minute.ago - runner2 = create :ci_runner, contacted_at: 3.minutes.ago - runner3 = create :ci_runner, contacted_at: 2.minutes.ago - - expect(described_class.new(params: { sort: 'contacted_asc' }).execute).to eq [runner2, runner3, runner1] - end - end - end - - context 'paginate' do - it 'returns the runners for the specified page' do - stub_const('Admin::RunnersFinder::NUMBER_OF_RUNNERS_PER_PAGE', 1) - runner1 = create :ci_runner, created_at: '2018-07-12 07:00' - runner2 = create :ci_runner, created_at: '2018-07-12 08:00' - - expect(described_class.new(params: { page: 1 }).execute).to eq [runner2] - expect(described_class.new(params: { page: 2 }).execute).to eq [runner1] - end - end - end -end diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb new file mode 100644 index 00000000000..0c319d8cc0f --- /dev/null +++ b/spec/finders/ci/runners_finder_spec.rb @@ -0,0 +1,304 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Ci::RunnersFinder do + context 'admin' do + let_it_be(:admin) { create(:user, :admin) } + + describe '#execute' do + context 'with empty params' do + it 'returns all runners' do + runner1 = create :ci_runner, active: true + runner2 = create :ci_runner, active: false + + expect(described_class.new(current_user: admin, params: {}).execute).to match_array [runner1, runner2] + end + end + + context 'filter by search term' do + it 'calls Ci::Runner.search' do + expect(Ci::Runner).to receive(:search).with('term').and_call_original + + described_class.new(current_user: admin, params: { search: 'term' }).execute + end + end + + context 'filter by status' do + it 'calls the corresponding scope on Ci::Runner' do + expect(Ci::Runner).to receive(:paused).and_call_original + + described_class.new(current_user: admin, params: { status_status: 'paused' }).execute + end + end + + context 'filter by runner type' do + it 'calls the corresponding scope on Ci::Runner' do + expect(Ci::Runner).to receive(:project_type).and_call_original + + described_class.new(current_user: admin, params: { type_type: 'project_type' }).execute + end + end + + context 'filter by tag_name' do + it 'calls the corresponding scope on Ci::Runner' do + expect(Ci::Runner).to receive(:tagged_with).with(%w[tag1 tag2]).and_call_original + + described_class.new(current_user: admin, params: { tag_name: %w[tag1 tag2] }).execute + end + end + + context 'sort' do + context 'without sort param' do + it 'sorts by created_at' do + runner1 = create :ci_runner, created_at: '2018-07-12 07:00' + runner2 = create :ci_runner, created_at: '2018-07-12 08:00' + runner3 = create :ci_runner, created_at: '2018-07-12 09:00' + + expect(described_class.new(current_user: admin, params: {}).execute).to eq [runner3, runner2, runner1] + end + end + + context 'with sort param' do + it 'sorts by specified attribute' do + runner1 = create :ci_runner, contacted_at: 1.minute.ago + runner2 = create :ci_runner, contacted_at: 3.minutes.ago + runner3 = create :ci_runner, contacted_at: 2.minutes.ago + + expect(described_class.new(current_user: admin, params: { sort: 'contacted_asc' }).execute).to eq [runner2, runner3, runner1] + end + end + end + + context 'paginate' do + it 'returns the runners for the specified page' do + stub_const('Ci::RunnersFinder::NUMBER_OF_RUNNERS_PER_PAGE', 1) + runner1 = create :ci_runner, created_at: '2018-07-12 07:00' + runner2 = create :ci_runner, created_at: '2018-07-12 08:00' + + expect(described_class.new(current_user: admin, params: { page: 1 }).execute).to eq [runner2] + expect(described_class.new(current_user: admin, params: { page: 2 }).execute).to eq [runner1] + end + end + + context 'non admin user' do + it 'returns no runners' do + user = create :user + create :ci_runner, active: true + create :ci_runner, active: false + + expect(described_class.new(current_user: user, params: {}).execute).to be_empty + end + end + + context 'user is nil' do + it 'returns no runners' do + user = nil + create :ci_runner, active: true + create :ci_runner, active: false + + expect(described_class.new(current_user: user, params: {}).execute).to be_empty + end + end + end + end + + context 'group' do + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:sub_group_1) { create(:group, parent: group) } + let_it_be(:sub_group_2) { create(:group, parent: group) } + let_it_be(:sub_group_3) { create(:group, parent: sub_group_1) } + let_it_be(:sub_group_4) { create(:group, parent: sub_group_3) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:project_2) { create(:project, group: group) } + let_it_be(:project_3) { create(:project, group: sub_group_1) } + let_it_be(:project_4) { create(:project, group: sub_group_2) } + let_it_be(:project_5) { create(:project, group: sub_group_3) } + let_it_be(:project_6) { create(:project, group: sub_group_4) } + let_it_be(:runner_group) { create(:ci_runner, :group, contacted_at: 12.minutes.ago) } + let_it_be(:runner_sub_group_1) { create(:ci_runner, :group, active: false, contacted_at: 11.minutes.ago) } + let_it_be(:runner_sub_group_2) { create(:ci_runner, :group, contacted_at: 10.minutes.ago) } + let_it_be(:runner_sub_group_3) { create(:ci_runner, :group, contacted_at: 9.minutes.ago) } + let_it_be(:runner_sub_group_4) { create(:ci_runner, :group, contacted_at: 8.minutes.ago) } + let_it_be(:runner_project_1) { create(:ci_runner, :project, contacted_at: 7.minutes.ago, projects: [project])} + let_it_be(:runner_project_2) { create(:ci_runner, :project, contacted_at: 6.minutes.ago, projects: [project_2])} + let_it_be(:runner_project_3) { create(:ci_runner, :project, contacted_at: 5.minutes.ago, description: 'runner_project_search', projects: [project, project_2])} + let_it_be(:runner_project_4) { create(:ci_runner, :project, contacted_at: 4.minutes.ago, projects: [project_3])} + let_it_be(:runner_project_5) { create(:ci_runner, :project, contacted_at: 3.minutes.ago, tag_list: %w[runner_tag], projects: [project_4])} + let_it_be(:runner_project_6) { create(:ci_runner, :project, contacted_at: 2.minutes.ago, projects: [project_5])} + let_it_be(:runner_project_7) { create(:ci_runner, :project, contacted_at: 1.minute.ago, projects: [project_6])} + + let(:params) { {} } + + before do + group.runners << runner_group + sub_group_1.runners << runner_sub_group_1 + sub_group_2.runners << runner_sub_group_2 + sub_group_3.runners << runner_sub_group_3 + sub_group_4.runners << runner_sub_group_4 + end + + describe '#execute' do + subject { described_class.new(current_user: user, group: group, params: params).execute } + + context 'no params' do + before do + group.add_owner(user) + end + + it 'returns all runners' do + expect(subject).to eq([runner_project_7, runner_project_6, runner_project_5, + runner_project_4, runner_project_3, runner_project_2, + runner_project_1, runner_sub_group_4, runner_sub_group_3, + runner_sub_group_2, runner_sub_group_1, runner_group]) + end + end + + context 'with sort param' do + let(:params) { { sort: 'contacted_asc' } } + + before do + group.add_owner(user) + end + + it 'sorts by specified attribute' do + expect(subject).to eq([runner_group, runner_sub_group_1, runner_sub_group_2, + runner_sub_group_3, runner_sub_group_4, runner_project_1, + runner_project_2, runner_project_3, runner_project_4, + runner_project_5, runner_project_6, runner_project_7]) + end + end + + context 'paginate' do + using RSpec::Parameterized::TableSyntax + + let(:runners) do + [[runner_project_7, runner_project_6, runner_project_5], + [runner_project_4, runner_project_3, runner_project_2], + [runner_project_1, runner_sub_group_4, runner_sub_group_3], + [runner_sub_group_2, runner_sub_group_1, runner_group]] + end + + where(:page, :index) do + 1 | 0 + 2 | 1 + 3 | 2 + 4 | 3 + end + + before do + stub_const('Ci::RunnersFinder::NUMBER_OF_RUNNERS_PER_PAGE', 3) + + group.add_owner(user) + end + + with_them do + let(:params) { { page: page } } + + it 'returns the runners for the specified page' do + expect(subject).to eq(runners[index]) + end + end + end + + context 'filter by search term' do + let(:params) { { search: 'runner_project_search' } } + + before do + group.add_owner(user) + end + + it 'returns correct runner' do + expect(subject).to eq([runner_project_3]) + end + end + + context 'filter by status' do + let(:params) { { status_status: 'paused' } } + + before do + group.add_owner(user) + end + + it 'returns correct runner' do + expect(subject).to eq([runner_sub_group_1]) + end + end + + context 'filter by tag_name' do + let(:params) { { tag_name: %w[runner_tag] } } + + before do + group.add_owner(user) + end + + it 'returns correct runner' do + expect(subject).to eq([runner_project_5]) + end + end + + context 'filter by runner type' do + let(:params) { { type_type: 'project_type' } } + + before do + group.add_owner(user) + end + + it 'returns correct runners' do + expect(subject).to eq([runner_project_7, runner_project_6, + runner_project_5, runner_project_4, + runner_project_3, runner_project_2, runner_project_1]) + end + end + + context 'user has no access to runners' do + where(:user_permission) do + [:maintainer, :developer, :reporter, :guest] + end + + with_them do + before do + create(:group_member, user_permission, group: group, user: user) + end + + it 'returns no runners' do + expect(subject).to be_empty + end + end + end + + context 'user with no access' do + it 'returns no runners' do + expect(subject).to be_empty + end + end + + context 'user is nil' do + let_it_be(:user) { nil } + + it 'returns no runners' do + expect(subject).to be_empty + end + end + end + + describe '#sort_key' do + subject { described_class.new(current_user: user, group: group, params: params).sort_key } + + context 'no params' do + it 'returns created_date' do + expect(subject).to eq('created_date') + end + end + + context 'with params' do + let(:params) { { sort: 'contacted_asc' } } + + it 'returns contacted_asc' do + expect(subject).to eq('contacted_asc') + end + end + end + end +end diff --git a/spec/finders/uploader_finder_spec.rb b/spec/finders/uploader_finder_spec.rb new file mode 100644 index 00000000000..bb6d83289ef --- /dev/null +++ b/spec/finders/uploader_finder_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe UploaderFinder do + describe '#execute' do + let(:project) { build(:project) } + let(:upload) { create(:upload, :issuable_upload, :with_file) } + let(:secret) { upload.secret } + let(:file_name) { upload.path } + + subject { described_class.new(project, secret, file_name).execute } + + before do + upload.save + end + + context 'when successful' do + before do + allow_next_instance_of(FileUploader) do |uploader| + allow(uploader).to receive(:retrieve_from_store!).with(upload.path).and_return(uploader) + end + end + + it 'gets the file-like uploader' do + expect(subject).to be_an_instance_of(FileUploader) + expect(subject.model).to eq(project) + expect(subject.secret).to eq(secret) + end + end + + context 'when path traversal in file name' do + before do + upload.path = '/uploads/11111111111111111111111111111111/../../../../../../../../../../../../../../etc/passwd)' + upload.save + end + + it 'returns nil' do + expect(subject).to be(nil) + end + end + + context 'when unexpected failure' do + before do + allow_next_instance_of(FileUploader) do |uploader| + allow(uploader).to receive(:retrieve_from_store!).and_raise(StandardError) + end + end + + it 'returns nil when unexpected error is raised' do + expect { subject }.to raise_error(StandardError) + end + end + end +end diff --git a/spec/frontend/droplab/plugins/input_setter_spec.js b/spec/frontend/droplab/plugins/input_setter_spec.js new file mode 100644 index 00000000000..eebde018fa1 --- /dev/null +++ b/spec/frontend/droplab/plugins/input_setter_spec.js @@ -0,0 +1,259 @@ +import InputSetter from '~/droplab/plugins/input_setter'; + +describe('InputSetter', () => { + let testContext; + + beforeEach(() => { + testContext = {}; + }); + + describe('init', () => { + beforeEach(() => { + testContext.config = { InputSetter: {} }; + testContext.hook = { config: testContext.config }; + testContext.inputSetter = { + addEvents: jest.fn(), + }; + + InputSetter.init.call(testContext.inputSetter, testContext.hook); + }); + + it('should set .hook', () => { + expect(testContext.inputSetter.hook).toBe(testContext.hook); + }); + + it('should set .config', () => { + expect(testContext.inputSetter.config).toBe(testContext.config.InputSetter); + }); + + it('should set .eventWrapper', () => { + expect(testContext.inputSetter.eventWrapper).toEqual({}); + }); + + it('should call .addEvents', () => { + expect(testContext.inputSetter.addEvents).toHaveBeenCalled(); + }); + + describe('if config.InputSetter is not set', () => { + beforeEach(() => { + testContext.config = { InputSetter: undefined }; + testContext.hook = { config: testContext.config }; + + InputSetter.init.call(testContext.inputSetter, testContext.hook); + }); + + it('should set .config to an empty object', () => { + expect(testContext.inputSetter.config).toEqual({}); + }); + + it('should set hook.config to an empty object', () => { + expect(testContext.hook.config.InputSetter).toEqual({}); + }); + }); + }); + + describe('addEvents', () => { + beforeEach(() => { + testContext.hook = { + list: { + list: { + addEventListener: jest.fn(), + }, + }, + }; + testContext.inputSetter = { eventWrapper: {}, hook: testContext.hook, setInputs: () => {} }; + + InputSetter.addEvents.call(testContext.inputSetter); + }); + + it('should set .eventWrapper.setInputs', () => { + expect(testContext.inputSetter.eventWrapper.setInputs).toEqual(expect.any(Function)); + }); + + it('should call .addEventListener', () => { + expect(testContext.hook.list.list.addEventListener).toHaveBeenCalledWith( + 'click.dl', + testContext.inputSetter.eventWrapper.setInputs, + ); + }); + }); + + describe('removeEvents', () => { + beforeEach(() => { + testContext.hook = { + list: { + list: { + removeEventListener: jest.fn(), + }, + }, + }; + testContext.eventWrapper = { + setInputs: jest.fn(), + }; + testContext.inputSetter = { eventWrapper: testContext.eventWrapper, hook: testContext.hook }; + + InputSetter.removeEvents.call(testContext.inputSetter); + }); + + it('should call .removeEventListener', () => { + expect(testContext.hook.list.list.removeEventListener).toHaveBeenCalledWith( + 'click.dl', + testContext.eventWrapper.setInputs, + ); + }); + }); + + describe('setInputs', () => { + beforeEach(() => { + testContext.event = { detail: { selected: {} } }; + testContext.config = [0, 1]; + testContext.inputSetter = { config: testContext.config, setInput: () => {} }; + + jest.spyOn(testContext.inputSetter, 'setInput').mockImplementation(() => {}); + + InputSetter.setInputs.call(testContext.inputSetter, testContext.event); + }); + + it('should call .setInput for each config element', () => { + const allArgs = testContext.inputSetter.setInput.mock.calls; + + expect(allArgs.length).toEqual(2); + + allArgs.forEach((args, i) => { + expect(args[0]).toBe(testContext.config[i]); + expect(args[1]).toBe(testContext.event.detail.selected); + }); + }); + + describe('if config isnt an array', () => { + beforeEach(() => { + testContext.inputSetter = { config: {}, setInput: () => {} }; + + InputSetter.setInputs.call(testContext.inputSetter, testContext.event); + }); + + it('should set .config to an array with .config as the first element', () => { + expect(testContext.inputSetter.config).toEqual([{}]); + }); + }); + }); + + describe('setInput', () => { + beforeEach(() => { + testContext.selectedItem = { getAttribute: () => {} }; + testContext.input = { value: 'oldValue', tagName: 'INPUT', hasAttribute: () => {} }; + testContext.config = { valueAttribute: {}, input: testContext.input }; + testContext.inputSetter = { hook: { trigger: {} } }; + testContext.newValue = 'newValue'; + + jest.spyOn(testContext.selectedItem, 'getAttribute').mockReturnValue(testContext.newValue); + jest.spyOn(testContext.input, 'hasAttribute').mockReturnValue(false); + + InputSetter.setInput.call( + testContext.inputSetter, + testContext.config, + testContext.selectedItem, + ); + }); + + it('should call .getAttribute', () => { + expect(testContext.selectedItem.getAttribute).toHaveBeenCalledWith( + testContext.config.valueAttribute, + ); + }); + + it('should call .hasAttribute', () => { + expect(testContext.input.hasAttribute).toHaveBeenCalledWith(undefined); + }); + + it('should set the value of the input', () => { + expect(testContext.input.value).toBe(testContext.newValue); + }); + + describe('if no config.input is provided', () => { + beforeEach(() => { + testContext.config = { valueAttribute: {} }; + testContext.trigger = { value: 'oldValue', tagName: 'INPUT', hasAttribute: () => {} }; + testContext.inputSetter = { hook: { trigger: testContext.trigger } }; + + InputSetter.setInput.call( + testContext.inputSetter, + testContext.config, + testContext.selectedItem, + ); + }); + + it('should set the value of the hook.trigger', () => { + expect(testContext.trigger.value).toBe(testContext.newValue); + }); + }); + + describe('if the input tag is not INPUT', () => { + beforeEach(() => { + testContext.input = { textContent: 'oldValue', tagName: 'SPAN', hasAttribute: () => {} }; + testContext.config = { valueAttribute: {}, input: testContext.input }; + + InputSetter.setInput.call( + testContext.inputSetter, + testContext.config, + testContext.selectedItem, + ); + }); + + it('should set the textContent of the input', () => { + expect(testContext.input.textContent).toBe(testContext.newValue); + }); + }); + + describe('if there is an inputAttribute', () => { + beforeEach(() => { + testContext.selectedItem = { getAttribute: () => {} }; + testContext.input = { id: 'oldValue', hasAttribute: () => {}, setAttribute: () => {} }; + testContext.inputSetter = { hook: { trigger: {} } }; + testContext.newValue = 'newValue'; + testContext.inputAttribute = 'id'; + testContext.config = { + valueAttribute: {}, + input: testContext.input, + inputAttribute: testContext.inputAttribute, + }; + + jest.spyOn(testContext.selectedItem, 'getAttribute').mockReturnValue(testContext.newValue); + jest.spyOn(testContext.input, 'hasAttribute').mockReturnValue(true); + jest.spyOn(testContext.input, 'setAttribute').mockImplementation(() => {}); + + InputSetter.setInput.call( + testContext.inputSetter, + testContext.config, + testContext.selectedItem, + ); + }); + + it('should call setAttribute', () => { + expect(testContext.input.setAttribute).toHaveBeenCalledWith( + testContext.inputAttribute, + testContext.newValue, + ); + }); + + it('should not set the value or textContent of the input', () => { + expect(testContext.input.value).not.toBe('newValue'); + expect(testContext.input.textContent).not.toBe('newValue'); + }); + }); + }); + + describe('destroy', () => { + beforeEach(() => { + testContext.inputSetter = { + removeEvents: jest.fn(), + }; + + InputSetter.destroy.call(testContext.inputSetter); + }); + + it('should call .removeEvents', () => { + expect(testContext.inputSetter.removeEvents).toHaveBeenCalled(); + }); + }); +}); diff --git a/spec/frontend/ide/components/repo_editor_spec.js b/spec/frontend/ide/components/repo_editor_spec.js index 913c4f3747d..cdb107da737 100644 --- a/spec/frontend/ide/components/repo_editor_spec.js +++ b/spec/frontend/ide/components/repo_editor_spec.js @@ -283,25 +283,13 @@ describe('RepoEditor', () => { expect(vm.model.events.size).toBe(2); }); - it.each` - insertFinalNewline | input | eol | output - ${true} | ${'testing 123\n'} | ${'\n'} | ${'testing 123\n'} - ${true} | ${'testing 123'} | ${'\n'} | ${'testing 123\n'} - ${false} | ${'testing 123'} | ${'\n'} | ${'testing 123'} - ${true} | ${'testing 123'} | ${'\r\n'} | ${'testing 123\r\n'} - ${false} | ${'testing 123'} | ${'\r\n'} | ${'testing 123'} - `( - 'updates state with "$output" if `this.insertFinalNewline` is $insertFinalNewline', - ({ insertFinalNewline, input, eol, output }) => { - jest.spyOn(vm.model.getModel(), 'getEOL').mockReturnValue(eol); - - vm.addFinalNewline = insertFinalNewline; - - vm.model.setValue(input); - - expect(vm.file.content).toBe(output); - }, - ); + it('updates state with the value of the model', () => { + vm.model.setValue('testing 1234'); + + vm.setupEditor(); + + expect(vm.file.content).toBe('testing 1234'); + }); it('sets head model as staged file', () => { jest.spyOn(vm.editor, 'createModel'); diff --git a/spec/frontend/ide/lib/common/model_spec.js b/spec/frontend/ide/lib/common/model_spec.js index 2ef2f0da6da..df46b7774b0 100644 --- a/spec/frontend/ide/lib/common/model_spec.js +++ b/spec/frontend/ide/lib/common/model_spec.js @@ -133,5 +133,77 @@ describe('Multi-file editor library model', () => { expect(disposeSpy).toHaveBeenCalled(); }); + + it('applies custom options and triggers onChange callback', () => { + const changeSpy = jest.fn(); + jest.spyOn(model, 'applyCustomOptions'); + + model.onChange(changeSpy); + + model.dispose(); + + expect(model.applyCustomOptions).toHaveBeenCalled(); + expect(changeSpy).toHaveBeenCalled(); + }); + }); + + describe('updateOptions', () => { + it('sets the options on the options object', () => { + model.updateOptions({ insertSpaces: true, someOption: 'some value' }); + + expect(model.options).toEqual({ + endOfLine: 0, + insertFinalNewline: true, + insertSpaces: true, + someOption: 'some value', + trimTrailingWhitespace: false, + }); + }); + + it.each` + option | value + ${'insertSpaces'} | ${true} + ${'insertSpaces'} | ${false} + ${'indentSize'} | ${4} + ${'tabSize'} | ${3} + `("correctly sets option: $option=$value to Monaco's TextModel", ({ option, value }) => { + model.updateOptions({ [option]: value }); + + expect(model.getModel().getOptions()).toMatchObject({ [option]: value }); + }); + + it('applies custom options immediately', () => { + jest.spyOn(model, 'applyCustomOptions'); + + model.updateOptions({ trimTrailingWhitespace: true, someOption: 'some value' }); + + expect(model.applyCustomOptions).toHaveBeenCalled(); + }); + }); + + describe('applyCustomOptions', () => { + it.each` + option | value | contentBefore | contentAfter + ${'endOfLine'} | ${0} | ${'hello\nworld\n'} | ${'hello\nworld\n'} + ${'endOfLine'} | ${0} | ${'hello\r\nworld\r\n'} | ${'hello\nworld\n'} + ${'endOfLine'} | ${1} | ${'hello\nworld\n'} | ${'hello\r\nworld\r\n'} + ${'endOfLine'} | ${1} | ${'hello\r\nworld\r\n'} | ${'hello\r\nworld\r\n'} + ${'insertFinalNewline'} | ${true} | ${'hello\nworld'} | ${'hello\nworld\n'} + ${'insertFinalNewline'} | ${true} | ${'hello\nworld\n'} | ${'hello\nworld\n'} + ${'insertFinalNewline'} | ${false} | ${'hello\nworld'} | ${'hello\nworld'} + ${'trimTrailingWhitespace'} | ${true} | ${'hello \t\nworld \t\n'} | ${'hello\nworld\n'} + ${'trimTrailingWhitespace'} | ${true} | ${'hello \t\r\nworld \t\r\n'} | ${'hello\nworld\n'} + ${'trimTrailingWhitespace'} | ${false} | ${'hello \t\r\nworld \t\r\n'} | ${'hello \t\nworld \t\n'} + `( + 'correctly applies custom option $option=$value to content', + ({ option, value, contentBefore, contentAfter }) => { + model.options[option] = value; + + model.updateNewContent(contentBefore); + model.applyCustomOptions(); + + expect(model.getModel().getValue()).toEqual(contentAfter); + }, + ); }); }); diff --git a/spec/frontend/ide/lib/diff/diff_spec.js b/spec/frontend/ide/lib/diff/diff_spec.js index d9b088e2c12..901f9e7cfd1 100644 --- a/spec/frontend/ide/lib/diff/diff_spec.js +++ b/spec/frontend/ide/lib/diff/diff_spec.js @@ -73,5 +73,13 @@ describe('Multi-file editor library diff calculator', () => { expect(diff.endLineNumber).toBe(1); }); + + it('disregards changes for EOL type changes', () => { + const text1 = 'line1\nline2\nline3\n'; + const text2 = 'line1\r\nline2\r\nline3\r\n'; + + expect(computeDiff(text1, text2)).toEqual([]); + expect(computeDiff(text2, text1)).toEqual([]); + }); }); }); diff --git a/spec/frontend/ide/lib/editor_options_spec.js b/spec/frontend/ide/lib/editor_options_spec.js deleted file mode 100644 index b07a583b7c8..00000000000 --- a/spec/frontend/ide/lib/editor_options_spec.js +++ /dev/null @@ -1,11 +0,0 @@ -import editorOptions from '~/ide/lib/editor_options'; - -describe('Multi-file editor library editor options', () => { - it('returns an array', () => { - expect(editorOptions).toEqual(expect.any(Array)); - }); - - it('contains readOnly option', () => { - expect(editorOptions[0].readOnly).toBeDefined(); - }); -}); diff --git a/spec/frontend/ide/lib/editor_spec.js b/spec/frontend/ide/lib/editor_spec.js index 36d4c3c26ee..6320add8988 100644 --- a/spec/frontend/ide/lib/editor_spec.js +++ b/spec/frontend/ide/lib/editor_spec.js @@ -72,6 +72,7 @@ describe('Multi-file editor library', () => { expect(monacoEditor.createDiffEditor).toHaveBeenCalledWith(holder, { ...defaultEditorOptions, + ignoreTrimWhitespace: false, quickSuggestions: false, occurrencesHighlight: false, renderSideBySide: false, diff --git a/spec/frontend/ide/utils_spec.js b/spec/frontend/ide/utils_spec.js index 4ae440ff09e..0b46e2cc02f 100644 --- a/spec/frontend/ide/utils_spec.js +++ b/spec/frontend/ide/utils_spec.js @@ -2,7 +2,8 @@ import { isTextFile, registerLanguages, trimPathComponents, - addFinalNewline, + insertFinalNewline, + trimTrailingWhitespace, getPathParents, } from '~/ide/utils'; import { languages } from 'monaco-editor'; @@ -155,6 +156,20 @@ describe('WebIDE utils', () => { }); }); + describe('trimTrailingWhitespace', () => { + it.each` + input | output + ${'text \n more text \n'} | ${'text\n more text\n'} + ${'text \n more text \n\n \n'} | ${'text\n more text\n\n\n'} + ${'text \t\t \n more text \n\t\ttext\n \n\t\t'} | ${'text\n more text\n\t\ttext\n\n'} + ${'text \r\n more text \r\n'} | ${'text\r\n more text\r\n'} + ${'text \r\n more text \r\n\r\n \r\n'} | ${'text\r\n more text\r\n\r\n\r\n'} + ${'text \t\t \r\n more text \r\n\t\ttext\r\n \r\n\t\t'} | ${'text\r\n more text\r\n\t\ttext\r\n\r\n'} + `("trims trailing whitespace in each line of file's contents: $input", ({ input, output }) => { + expect(trimTrailingWhitespace(input)).toBe(output); + }); + }); + describe('addFinalNewline', () => { it.each` input | output @@ -163,7 +178,7 @@ describe('WebIDE utils', () => { ${'some text\n\n'} | ${'some text\n\n'} ${'some\n text'} | ${'some\n text\n'} `('adds a newline if it doesnt already exist for input: $input', ({ input, output }) => { - expect(addFinalNewline(input)).toEqual(output); + expect(insertFinalNewline(input)).toBe(output); }); it.each` @@ -174,7 +189,7 @@ describe('WebIDE utils', () => { ${'some text\r\n\r\n'} | ${'some text\r\n\r\n'} ${'some\r\n text'} | ${'some\r\n text\r\n'} `('works with CRLF newline style; input: $input', ({ input, output }) => { - expect(addFinalNewline(input, '\r\n')).toEqual(output); + expect(insertFinalNewline(input, '\r\n')).toBe(output); }); }); diff --git a/spec/javascripts/droplab/plugins/input_setter_spec.js b/spec/javascripts/droplab/plugins/input_setter_spec.js deleted file mode 100644 index 711e0486bff..00000000000 --- a/spec/javascripts/droplab/plugins/input_setter_spec.js +++ /dev/null @@ -1,214 +0,0 @@ -import InputSetter from '~/droplab/plugins/input_setter'; - -describe('InputSetter', function() { - describe('init', function() { - beforeEach(function() { - this.config = { InputSetter: {} }; - this.hook = { config: this.config }; - this.inputSetter = jasmine.createSpyObj('inputSetter', ['addEvents']); - - InputSetter.init.call(this.inputSetter, this.hook); - }); - - it('should set .hook', function() { - expect(this.inputSetter.hook).toBe(this.hook); - }); - - it('should set .config', function() { - expect(this.inputSetter.config).toBe(this.config.InputSetter); - }); - - it('should set .eventWrapper', function() { - expect(this.inputSetter.eventWrapper).toEqual({}); - }); - - it('should call .addEvents', function() { - expect(this.inputSetter.addEvents).toHaveBeenCalled(); - }); - - describe('if config.InputSetter is not set', function() { - beforeEach(function() { - this.config = { InputSetter: undefined }; - this.hook = { config: this.config }; - - InputSetter.init.call(this.inputSetter, this.hook); - }); - - it('should set .config to an empty object', function() { - expect(this.inputSetter.config).toEqual({}); - }); - - it('should set hook.config to an empty object', function() { - expect(this.hook.config.InputSetter).toEqual({}); - }); - }); - }); - - describe('addEvents', function() { - beforeEach(function() { - this.hook = { list: { list: jasmine.createSpyObj('list', ['addEventListener']) } }; - this.inputSetter = { eventWrapper: {}, hook: this.hook, setInputs: () => {} }; - - InputSetter.addEvents.call(this.inputSetter); - }); - - it('should set .eventWrapper.setInputs', function() { - expect(this.inputSetter.eventWrapper.setInputs).toEqual(jasmine.any(Function)); - }); - - it('should call .addEventListener', function() { - expect(this.hook.list.list.addEventListener).toHaveBeenCalledWith( - 'click.dl', - this.inputSetter.eventWrapper.setInputs, - ); - }); - }); - - describe('removeEvents', function() { - beforeEach(function() { - this.hook = { list: { list: jasmine.createSpyObj('list', ['removeEventListener']) } }; - this.eventWrapper = jasmine.createSpyObj('eventWrapper', ['setInputs']); - this.inputSetter = { eventWrapper: this.eventWrapper, hook: this.hook }; - - InputSetter.removeEvents.call(this.inputSetter); - }); - - it('should call .removeEventListener', function() { - expect(this.hook.list.list.removeEventListener).toHaveBeenCalledWith( - 'click.dl', - this.eventWrapper.setInputs, - ); - }); - }); - - describe('setInputs', function() { - beforeEach(function() { - this.event = { detail: { selected: {} } }; - this.config = [0, 1]; - this.inputSetter = { config: this.config, setInput: () => {} }; - - spyOn(this.inputSetter, 'setInput'); - - InputSetter.setInputs.call(this.inputSetter, this.event); - }); - - it('should call .setInput for each config element', function() { - const allArgs = this.inputSetter.setInput.calls.allArgs(); - - expect(allArgs.length).toEqual(2); - - allArgs.forEach((args, i) => { - expect(args[0]).toBe(this.config[i]); - expect(args[1]).toBe(this.event.detail.selected); - }); - }); - - describe('if config isnt an array', function() { - beforeEach(function() { - this.inputSetter = { config: {}, setInput: () => {} }; - - InputSetter.setInputs.call(this.inputSetter, this.event); - }); - - it('should set .config to an array with .config as the first element', function() { - expect(this.inputSetter.config).toEqual([{}]); - }); - }); - }); - - describe('setInput', function() { - beforeEach(function() { - this.selectedItem = { getAttribute: () => {} }; - this.input = { value: 'oldValue', tagName: 'INPUT', hasAttribute: () => {} }; - this.config = { valueAttribute: {}, input: this.input }; - this.inputSetter = { hook: { trigger: {} } }; - this.newValue = 'newValue'; - - spyOn(this.selectedItem, 'getAttribute').and.returnValue(this.newValue); - spyOn(this.input, 'hasAttribute').and.returnValue(false); - - InputSetter.setInput.call(this.inputSetter, this.config, this.selectedItem); - }); - - it('should call .getAttribute', function() { - expect(this.selectedItem.getAttribute).toHaveBeenCalledWith(this.config.valueAttribute); - }); - - it('should call .hasAttribute', function() { - expect(this.input.hasAttribute).toHaveBeenCalledWith(undefined); - }); - - it('should set the value of the input', function() { - expect(this.input.value).toBe(this.newValue); - }); - - describe('if no config.input is provided', function() { - beforeEach(function() { - this.config = { valueAttribute: {} }; - this.trigger = { value: 'oldValue', tagName: 'INPUT', hasAttribute: () => {} }; - this.inputSetter = { hook: { trigger: this.trigger } }; - - InputSetter.setInput.call(this.inputSetter, this.config, this.selectedItem); - }); - - it('should set the value of the hook.trigger', function() { - expect(this.trigger.value).toBe(this.newValue); - }); - }); - - describe('if the input tag is not INPUT', function() { - beforeEach(function() { - this.input = { textContent: 'oldValue', tagName: 'SPAN', hasAttribute: () => {} }; - this.config = { valueAttribute: {}, input: this.input }; - - InputSetter.setInput.call(this.inputSetter, this.config, this.selectedItem); - }); - - it('should set the textContent of the input', function() { - expect(this.input.textContent).toBe(this.newValue); - }); - }); - - describe('if there is an inputAttribute', function() { - beforeEach(function() { - this.selectedItem = { getAttribute: () => {} }; - this.input = { id: 'oldValue', hasAttribute: () => {}, setAttribute: () => {} }; - this.inputSetter = { hook: { trigger: {} } }; - this.newValue = 'newValue'; - this.inputAttribute = 'id'; - this.config = { - valueAttribute: {}, - input: this.input, - inputAttribute: this.inputAttribute, - }; - - spyOn(this.selectedItem, 'getAttribute').and.returnValue(this.newValue); - spyOn(this.input, 'hasAttribute').and.returnValue(true); - spyOn(this.input, 'setAttribute'); - - InputSetter.setInput.call(this.inputSetter, this.config, this.selectedItem); - }); - - it('should call setAttribute', function() { - expect(this.input.setAttribute).toHaveBeenCalledWith(this.inputAttribute, this.newValue); - }); - - it('should not set the value or textContent of the input', function() { - expect(this.input.value).not.toBe('newValue'); - expect(this.input.textContent).not.toBe('newValue'); - }); - }); - }); - - describe('destroy', function() { - beforeEach(function() { - this.inputSetter = jasmine.createSpyObj('inputSetter', ['removeEvents']); - - InputSetter.destroy.call(this.inputSetter); - }); - - it('should call .removeEvents', function() { - expect(this.inputSetter.removeEvents).toHaveBeenCalled(); - }); - }); -}); diff --git a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb index 0bf46217d60..7279399d1b8 100644 --- a/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb +++ b/spec/lib/gitlab/gfm/uploads_rewriter_spec.rb @@ -54,6 +54,14 @@ describe Gitlab::Gfm::UploadsRewriter do expect(new_paths).not_to include image_uploader.secret expect(new_paths).not_to include zip_uploader.secret end + + it 'skips nil files do' do + allow_next_instance_of(UploaderFinder) do |finder| + allow(finder).to receive(:execute).and_return(nil) + end + + expect(new_files).to be_empty + end end end @@ -68,16 +76,6 @@ describe Gitlab::Gfm::UploadsRewriter do expect(moved_text.scan(/\A\[.*?\]/).count).to eq(1) end - context 'path traversal in file name' do - let(:text) do - "" - end - - it 'throw an error' do - expect { rewriter.rewrite(new_project) }.to raise_error(an_instance_of(StandardError).and(having_attributes(message: "Invalid path"))) - end - end - context "file are stored locally" do include_examples "files are accessible" end diff --git a/spec/lib/gitlab/process_memory_cache/helper_spec.rb b/spec/lib/gitlab/process_memory_cache/helper_spec.rb new file mode 100644 index 00000000000..890642b1d5e --- /dev/null +++ b/spec/lib/gitlab/process_memory_cache/helper_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::ProcessMemoryCache::Helper, :use_clean_rails_memory_store_caching do + let(:minimal_test_class) do + Class.new do + include Gitlab::ProcessMemoryCache::Helper + + def cached_content + fetch_memory_cache(:cached_content_instance_key) { expensive_computation } + end + + def clear_cached_content + invalidate_memory_cache(:cached_content_instance_key) + end + end + end + + before do + stub_const("MinimalTestClass", minimal_test_class) + end + + subject { MinimalTestClass.new } + + describe '.fetch_memory_cache' do + it 'memoizes the result' do + is_expected.to receive(:expensive_computation).once.and_return(1) + + 2.times do + expect(subject.cached_content).to eq(1) + end + end + + it 'resets the cache when the shared key is missing', :aggregate_failures do + expect(Rails.cache).to receive(:read).with(:cached_content_instance_key).twice.and_return(nil) + is_expected.to receive(:expensive_computation).thrice.and_return(1, 2, 3) + + 3.times do |index| + expect(subject.cached_content).to eq(index + 1) + end + end + end + + describe '.invalidate_memory_cache' do + it 'invalidates the cache' do + is_expected.to receive(:expensive_computation).twice.and_return(1, 2) + + expect { subject.clear_cached_content }.to change { subject.cached_content } + end + end +end diff --git a/spec/models/ci/instance_variable_spec.rb b/spec/models/ci/instance_variable_spec.rb index ff8676e1424..4ad168ff0f2 100644 --- a/spec/models/ci/instance_variable_spec.rb +++ b/spec/models/ci/instance_variable_spec.rb @@ -39,7 +39,7 @@ describe Ci::InstanceVariable do it { expect(described_class.all_cached).to contain_exactly(protected_variable, unprotected_variable) } it 'memoizes the result' do - expect(described_class).to receive(:store_cache).with(:ci_instance_variable_data).once.and_call_original + expect(described_class).to receive(:unscoped).once.and_call_original 2.times do expect(described_class.all_cached).to contain_exactly(protected_variable, unprotected_variable) @@ -65,15 +65,6 @@ describe Ci::InstanceVariable do expect(described_class.all_cached).to contain_exactly(protected_variable, unprotected_variable, variable) end - - it 'resets the cache when the shared key is missing' do - expect(Rails.cache).to receive(:read).with(:ci_instance_variable_changed_at).twice.and_return(nil) - expect(described_class).to receive(:store_cache).with(:ci_instance_variable_data).thrice.and_call_original - - 3.times do - expect(described_class.all_cached).to contain_exactly(protected_variable, unprotected_variable) - end - end end describe '.unprotected_cached', :use_clean_rails_memory_store_caching do @@ -83,7 +74,7 @@ describe Ci::InstanceVariable do it { expect(described_class.unprotected_cached).to contain_exactly(unprotected_variable) } it 'memoizes the result' do - expect(described_class).to receive(:store_cache).with(:ci_instance_variable_data).once.and_call_original + expect(described_class).to receive(:unscoped).once.and_call_original 2.times do expect(described_class.unprotected_cached).to contain_exactly(unprotected_variable) |