diff options
37 files changed, 473 insertions, 192 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index c5d4cee36a1..4f9b378b40f 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.51.0 +0.52.0 diff --git a/app/workers/update_merge_requests_worker.rb b/app/workers/update_merge_requests_worker.rb index 150788ca611..89ae17cef37 100644 --- a/app/workers/update_merge_requests_worker.rb +++ b/app/workers/update_merge_requests_worker.rb @@ -2,10 +2,6 @@ class UpdateMergeRequestsWorker include Sidekiq::Worker include DedicatedSidekiqQueue - def metrics_tags - @metrics_tags || {} - end - def perform(project_id, user_id, oldrev, newrev, ref) project = Project.find_by(id: project_id) return unless project @@ -13,11 +9,6 @@ class UpdateMergeRequestsWorker user = User.find_by(id: user_id) return unless user - @metrics_tags = { - project_id: project_id, - user_id: user_id - } - MergeRequests::RefreshService.new(project, user).execute(oldrev, newrev, ref) end end diff --git a/changelogs/unreleased/.yml b/changelogs/unreleased/.yml new file mode 100644 index 00000000000..acf0bb80c72 --- /dev/null +++ b/changelogs/unreleased/.yml @@ -0,0 +1,5 @@ +--- +title: Remove update merge request worker tagging. +merge_request: +author: +type: removed diff --git a/changelogs/unreleased/39895-cant-set-mattermost-username-channel-from-api.yml b/changelogs/unreleased/39895-cant-set-mattermost-username-channel-from-api.yml new file mode 100644 index 00000000000..358c007387e --- /dev/null +++ b/changelogs/unreleased/39895-cant-set-mattermost-username-channel-from-api.yml @@ -0,0 +1,5 @@ +--- +title: Fix acceptance of username for Mattermost service update +merge_request: 15275 +author: +type: fixed diff --git a/changelogs/unreleased/fix-issues-api-list-performance.yml b/changelogs/unreleased/fix-issues-api-list-performance.yml new file mode 100644 index 00000000000..9c180f4d55e --- /dev/null +++ b/changelogs/unreleased/fix-issues-api-list-performance.yml @@ -0,0 +1,5 @@ +--- +title: Speed up issues list APIs +merge_request: +author: +type: performance diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 6de460f2778..b2e4b6d0955 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -58,6 +58,8 @@ Parameters: "project_id": 3, "title": "test1", "state": "opened", + "created_at": "2017-04-29T08:46:00Z", + "updated_at": "2017-04-29T08:46:00Z", "upvotes": 0, "downvotes": 0, "author": { @@ -170,6 +172,8 @@ Parameters: "project_id": 3, "title": "test1", "state": "opened", + "created_at": "2017-04-29T08:46:00Z", + "updated_at": "2017-04-29T08:46:00Z", "upvotes": 0, "downvotes": 0, "author": { @@ -246,6 +250,8 @@ Parameters: "project_id": 3, "title": "test1", "state": "merged", + "created_at": "2017-04-29T08:46:00Z", + "updated_at": "2017-04-29T08:46:00Z", "upvotes": 0, "downvotes": 0, "author": { diff --git a/doc/api/services.md b/doc/api/services.md index e642ec964de..08a2bee1518 100644 --- a/doc/api/services.md +++ b/doc/api/services.md @@ -572,7 +572,7 @@ Parameters: | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | | `token` | string | yes | The Mattermost token | - +| `username` | string | no | The username to use to post the message | ### Delete Mattermost slash command service diff --git a/lib/api/api.rb b/lib/api/api.rb index c37e596eb9d..8094597d238 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -61,7 +61,10 @@ module API mount ::API::V3::Variables end - before { header['X-Frame-Options'] = 'SAMEORIGIN' } + before do + header['X-Frame-Options'] = 'SAMEORIGIN' + header['X-Content-Type-Options'] = 'nosniff' + end # The locale is set to the current user's locale when `current_user` is loaded after { Gitlab::I18n.use_default_locale } diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 0df41dcc903..74dfd9f96de 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -68,7 +68,7 @@ module API desc: 'Return issues for the given scope: `created-by-me`, `assigned-to-me` or `all`' end get do - issues = find_issues + issues = paginate(find_issues) options = { with: Entities::IssueBasic, @@ -76,7 +76,7 @@ module API issuable_metadata: issuable_meta_data(issues, 'Issue') } - present paginate(issues), options + present issues, options end end @@ -95,7 +95,7 @@ module API get ":id/issues" do group = find_group!(params[:id]) - issues = find_issues(group_id: group.id) + issues = paginate(find_issues(group_id: group.id)) options = { with: Entities::IssueBasic, @@ -103,7 +103,7 @@ module API issuable_metadata: issuable_meta_data(issues, 'Issue') } - present paginate(issues), options + present issues, options end end @@ -124,7 +124,7 @@ module API get ":id/issues" do project = find_project!(params[:id]) - issues = find_issues(project_id: project.id) + issues = paginate(find_issues(project_id: project.id)) options = { with: Entities::IssueBasic, @@ -133,7 +133,7 @@ module API issuable_metadata: issuable_meta_data(issues, 'Issue') } - present paginate(issues), options + present issues, options end desc 'Get a single project issue' do diff --git a/lib/api/services.rb b/lib/api/services.rb index 6454e475036..bbcc851d07a 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -522,6 +522,12 @@ module API name: :webhook, type: String, desc: 'The Mattermost webhook. e.g. http://mattermost_host/hooks/...' + }, + { + required: false, + name: :username, + type: String, + desc: 'The username to use to post the message' } ], 'teamcity' => [ diff --git a/lib/gitlab/git/wiki.rb b/lib/gitlab/git/wiki.rb index fe901d049d4..5d36f16abd4 100644 --- a/lib/gitlab/git/wiki.rb +++ b/lib/gitlab/git/wiki.rb @@ -48,11 +48,14 @@ module Gitlab end def update_page(page_path, title, format, content, commit_details) - assert_type!(format, Symbol) - assert_type!(commit_details, CommitDetails) - - gollum_wiki.update_page(gollum_page_by_path(page_path), title, format, content, commit_details.to_h) - nil + @repository.gitaly_migrate(:wiki_update_page) do |is_enabled| + if is_enabled + gitaly_update_page(page_path, title, format, content, commit_details) + gollum_wiki.clear_cache + else + gollum_update_page(page_path, title, format, content, commit_details) + end + end end def pages @@ -149,6 +152,14 @@ module Gitlab nil end + def gollum_update_page(page_path, title, format, content, commit_details) + assert_type!(format, Symbol) + assert_type!(commit_details, CommitDetails) + + gollum_wiki.update_page(gollum_page_by_path(page_path), title, format, content, commit_details.to_h) + nil + end + def gollum_find_page(title:, version: nil, dir: nil) if version version = Gitlab::Git::Commit.find(@repository, version).id @@ -172,6 +183,10 @@ module Gitlab gitaly_wiki_client.write_page(name, format, content, commit_details) end + def gitaly_update_page(page_path, title, format, content, commit_details) + gitaly_wiki_client.update_page(page_path, title, format, content, commit_details) + end + def gitaly_delete_page(page_path, commit_details) gitaly_wiki_client.delete_page(page_path, commit_details) end diff --git a/lib/gitlab/gitaly_client/wiki_service.rb b/lib/gitlab/gitaly_client/wiki_service.rb index 15f0f30d303..1a668338f57 100644 --- a/lib/gitlab/gitaly_client/wiki_service.rb +++ b/lib/gitlab/gitaly_client/wiki_service.rb @@ -37,6 +37,31 @@ module Gitlab end end + def update_page(page_path, title, format, content, commit_details) + request = Gitaly::WikiUpdatePageRequest.new( + repository: @gitaly_repo, + page_path: GitalyClient.encode(page_path), + title: GitalyClient.encode(title), + format: format.to_s, + commit_details: gitaly_commit_details(commit_details) + ) + + strio = StringIO.new(content) + + enum = Enumerator.new do |y| + until strio.eof? + chunk = strio.read(MAX_MSG_SIZE) + request.content = GitalyClient.encode(chunk) + + y.yield request + + request = Gitaly::WikiUpdatePageRequest.new + end + end + + GitalyClient.call(@repository.storage, :wiki_service, :wiki_update_page, enum) + end + def delete_page(page_path, commit_details) request = Gitaly::WikiDeletePageRequest.new( repository: @gitaly_repo, diff --git a/lib/gitlab/github_import/client.rb b/lib/gitlab/github_import/client.rb index 844530b1ea7..5da9befa08e 100644 --- a/lib/gitlab/github_import/client.rb +++ b/lib/gitlab/github_import/client.rb @@ -38,7 +38,14 @@ module Gitlab # otherwise hitting the rate limit will result in a thread # being blocked in a `sleep()` call for up to an hour. def initialize(token, per_page: 100, parallel: true) - @octokit = Octokit::Client.new(access_token: token, per_page: per_page) + @octokit = Octokit::Client.new( + access_token: token, + per_page: per_page, + api_endpoint: api_endpoint + ) + + @octokit.connection_options[:ssl] = { verify: verify_ssl } + @parallel = parallel end @@ -122,6 +129,8 @@ module Gitlab # whether we are running in parallel mode or not. For more information see # `#rate_or_wait_for_rate_limit`. def with_rate_limit + return yield unless rate_limiting_enabled? + request_count_counter.increment raise_or_wait_for_rate_limit unless requests_remaining? @@ -163,8 +172,31 @@ module Gitlab octokit.rate_limit.resets_in + 5 end - def respond_to_missing?(method, include_private = false) - octokit.respond_to?(method, include_private) + def rate_limiting_enabled? + @rate_limiting_enabled ||= api_endpoint.include?('.github.com') + end + + def api_endpoint + custom_api_endpoint || default_api_endpoint + end + + def custom_api_endpoint + github_omniauth_provider.dig('args', 'client_options', 'site') + end + + def default_api_endpoint + OmniAuth::Strategies::GitHub.default_options[:client_options][:site] + end + + def verify_ssl + github_omniauth_provider.fetch('verify_ssl', true) + end + + def github_omniauth_provider + @github_omniauth_provider ||= + Gitlab.config.omniauth.providers + .find { |provider| provider.name == 'github' } + .to_h end def rate_limit_counter diff --git a/lib/gitlab/issuable_metadata.rb b/lib/gitlab/issuable_metadata.rb index 977c05910d3..0c9de72329c 100644 --- a/lib/gitlab/issuable_metadata.rb +++ b/lib/gitlab/issuable_metadata.rb @@ -1,6 +1,14 @@ module Gitlab module IssuableMetadata def issuable_meta_data(issuable_collection, collection_type) + # ActiveRecord uses Object#extend for null relations. + if !(issuable_collection.singleton_class < ActiveRecord::NullRelation) && + issuable_collection.respond_to?(:limit_value) && + issuable_collection.limit_value.nil? + + raise 'Collection must have a limit applied for preloading meta-data' + end + # map has to be used here since using pluck or select will # throw an error when ordering issuables by priority which inserts # a new order into the collection. diff --git a/lib/gitlab/metrics/sidekiq_middleware.rb b/lib/gitlab/metrics/sidekiq_middleware.rb index 55c707d5386..df4bdf16847 100644 --- a/lib/gitlab/metrics/sidekiq_middleware.rb +++ b/lib/gitlab/metrics/sidekiq_middleware.rb @@ -11,8 +11,6 @@ module Gitlab # Old gitlad-shell messages don't provide enqueued_at/created_at attributes trans.set(:sidekiq_queue_duration, Time.now.to_f - (message['enqueued_at'] || message['created_at'] || 0)) trans.run { yield } - - worker.metrics_tags.each { |tag, value| trans.add_tag(tag, value) } if worker.respond_to?(:metrics_tags) rescue Exception => error # rubocop: disable Lint/RescueException trans.add_event(:sidekiq_exception) diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb index fee1a127fd7..13150ddab67 100644 --- a/lib/gitlab/url_blocker.rb +++ b/lib/gitlab/url_blocker.rb @@ -22,10 +22,12 @@ module Gitlab return true if blocked_user_or_hostname?(uri.user) return true if blocked_user_or_hostname?(uri.hostname) - server_ips = Resolv.getaddresses(uri.hostname) + server_ips = Addrinfo.getaddrinfo(uri.hostname, 80, nil, :STREAM).map(&:ip_address) return true if (blocked_ips & server_ips).any? rescue Addressable::URI::InvalidURIError return true + rescue SocketError + return false end false diff --git a/lib/tasks/import.rake b/lib/tasks/import.rake index 8e9aef49e89..aafbe52e5f8 100644 --- a/lib/tasks/import.rake +++ b/lib/tasks/import.rake @@ -14,7 +14,9 @@ class GithubImport end def run! - @repo = GithubRepos.new(@options, @current_user, @github_repo).choose_one! + @repo = GithubRepos + .new(@options[:token], @current_user, @github_repo) + .choose_one! raise 'No repo found!' unless @repo @@ -28,7 +30,7 @@ class GithubImport private def show_warning! - puts "This will import GitHub #{@repo['full_name'].bright} into GitLab #{@project_path.bright} as #{@current_user.name}" + puts "This will import GitHub #{@repo.full_name.bright} into GitLab #{@project_path.bright} as #{@current_user.name}" puts "Permission checks are ignored. Press any key to continue.".color(:red) STDIN.getch @@ -65,16 +67,16 @@ class GithubImport @current_user, name: name, path: name, - description: @repo['description'], + description: @repo.description, namespace_id: namespace.id, visibility_level: visibility_level, - skip_wiki: @repo['has_wiki'] + skip_wiki: @repo.has_wiki ).execute project.update!( import_type: 'github', - import_source: @repo['full_name'], - import_url: @repo['clone_url'].sub('://', "://#{@options[:token]}@") + import_source: @repo.full_name, + import_url: @repo.clone_url.sub('://', "://#{@options[:token]}@") ) project @@ -93,13 +95,15 @@ class GithubImport end def visibility_level - @repo['private'] ? Gitlab::VisibilityLevel::PRIVATE : Gitlab::CurrentSettings.current_application_settings.default_project_visibility + @repo.private ? Gitlab::VisibilityLevel::PRIVATE : Gitlab::CurrentSettings.current_application_settings.default_project_visibility end end class GithubRepos - def initialize(options, current_user, github_repo) - @options = options + def initialize(token, current_user, github_repo) + @client = Gitlab::GithubImport::Client.new(token) + @client.octokit.auto_paginate = true + @current_user = current_user @github_repo = github_repo end @@ -108,17 +112,17 @@ class GithubRepos return found_github_repo if @github_repo repos.each do |repo| - print "ID: #{repo['id'].to_s.bright}".color(:green) - print "\tName: #{repo['full_name']}\n".color(:green) + print "ID: #{repo.id.to_s.bright}".color(:green) + print "\tName: #{repo.full_name}\n".color(:green) end print 'ID? '.bright - repos.find { |repo| repo['id'] == repo_id } + repos.find { |repo| repo.id == repo_id } end def found_github_repo - repos.find { |repo| repo['full_name'] == @github_repo } + repos.find { |repo| repo.full_name == @github_repo } end def repo_id @@ -126,7 +130,7 @@ class GithubRepos end def repos - Github::Repositories.new(@options).fetch + @client.octokit.list_repositories end end diff --git a/qa/.gitignore b/qa/.gitignore index 3fec32c8427..19ec17d0005 100644 --- a/qa/.gitignore +++ b/qa/.gitignore @@ -1 +1,2 @@ tmp/ +.ruby-version diff --git a/qa/bin/qa b/qa/bin/qa index cecdeac14db..f1704dc54e9 100755 --- a/qa/bin/qa +++ b/qa/bin/qa @@ -4,4 +4,4 @@ require_relative '../qa' QA::Scenario .const_get(ARGV.shift) - .perform(*ARGV) + .launch!(*ARGV) @@ -18,6 +18,7 @@ module QA ## # Support files # + autoload :Bootable, 'qa/scenario/bootable' autoload :Actable, 'qa/scenario/actable' autoload :Entrypoint, 'qa/scenario/entrypoint' autoload :Template, 'qa/scenario/template' diff --git a/qa/qa/page/mattermost/login.rb b/qa/qa/page/mattermost/login.rb index 2001dc5b230..42ab9c6f675 100644 --- a/qa/qa/page/mattermost/login.rb +++ b/qa/qa/page/mattermost/login.rb @@ -3,7 +3,7 @@ module QA module Mattermost class Login < Page::Base def initialize - visit(Runtime::Scenario.mattermost + '/login') + visit(Runtime::Scenario.mattermost_address + '/login') end def sign_in_using_oauth diff --git a/qa/qa/page/mattermost/main.rb b/qa/qa/page/mattermost/main.rb index e636d7676f4..4b8fc28e53f 100644 --- a/qa/qa/page/mattermost/main.rb +++ b/qa/qa/page/mattermost/main.rb @@ -3,7 +3,7 @@ module QA module Mattermost class Main < Page::Base def initialize - visit(Runtime::Scenario.mattermost) + visit(Runtime::Scenario.mattermost_address) end end end diff --git a/qa/qa/runtime/scenario.rb b/qa/qa/runtime/scenario.rb index 0c5e9787e17..7ef59046640 100644 --- a/qa/qa/runtime/scenario.rb +++ b/qa/qa/runtime/scenario.rb @@ -1,8 +1,28 @@ module QA module Runtime + ## + # Singleton approach to global test scenario arguments. + # module Scenario extend self - attr_accessor :mattermost + + attr_reader :attributes + + def define(attribute, value) + (@attributes ||= {}).store(attribute.to_sym, value) + + define_singleton_method(attribute) do + @attributes[attribute.to_sym].tap do |value| + if value.to_s.empty? + raise ArgumentError, "Empty `#{attribute}` attribute!" + end + end + end + end + + def method_missing(name, *) + raise ArgumentError, "Scenario attribute `#{name}` not defined!" + end end end end diff --git a/qa/qa/scenario/bootable.rb b/qa/qa/scenario/bootable.rb new file mode 100644 index 00000000000..22496bcc2fc --- /dev/null +++ b/qa/qa/scenario/bootable.rb @@ -0,0 +1,47 @@ +require 'optparse' + +module QA + module Scenario + module Bootable + Option = Struct.new(:name, :arg, :desc) + + def self.included(base) + base.extend(ClassMethods) + end + + module ClassMethods + def launch!(argv) + arguments = OptionParser.new do |parser| + options.to_a.each do |opt| + parser.on(opt.arg, opt.desc) do |value| + Runtime::Scenario.define(opt.name, value) + end + end + end + + arguments.parse!(argv) + + if has_attributes? + self.perform(**Runtime::Scenario.attributes) + else + self.perform(*argv) + end + end + + private + + def attribute(name, arg, desc) + options.push(Option.new(name, arg, desc)) + end + + def options + @options ||= [] + end + + def has_attributes? + options.any? + end + end + end + end +end diff --git a/qa/qa/scenario/entrypoint.rb b/qa/qa/scenario/entrypoint.rb index 33cb2696f8f..8f708fe38ab 100644 --- a/qa/qa/scenario/entrypoint.rb +++ b/qa/qa/scenario/entrypoint.rb @@ -5,6 +5,8 @@ module QA # including staging and on-premises installation. # class Entrypoint < Template + include Bootable + def self.tags(*tags) @tags = tags end diff --git a/qa/qa/scenario/test/integration/mattermost.rb b/qa/qa/scenario/test/integration/mattermost.rb index 0220da929ce..7d0702afdb1 100644 --- a/qa/qa/scenario/test/integration/mattermost.rb +++ b/qa/qa/scenario/test/integration/mattermost.rb @@ -10,7 +10,8 @@ module QA tags :core, :mattermost def perform(address, mattermost, *files) - Runtime::Scenario.mattermost = mattermost + Runtime::Scenario.define(:mattermost_address, mattermost) + super(address, *files) end end diff --git a/qa/spec/runtime/scenario_spec.rb b/qa/spec/runtime/scenario_spec.rb new file mode 100644 index 00000000000..7009192bcc0 --- /dev/null +++ b/qa/spec/runtime/scenario_spec.rb @@ -0,0 +1,27 @@ +describe QA::Runtime::Scenario do + subject do + Module.new.extend(described_class) + end + + it 'makes it possible to define global scenario attributes' do + subject.define(:my_attribute, 'some-value') + subject.define(:another_attribute, 'another-value') + + expect(subject.my_attribute).to eq 'some-value' + expect(subject.another_attribute).to eq 'another-value' + expect(subject.attributes) + .to eq(my_attribute: 'some-value', another_attribute: 'another-value') + end + + it 'raises error when attribute is not known' do + expect { subject.invalid_accessor } + .to raise_error ArgumentError, /invalid_accessor/ + end + + it 'raises error when attribute is empty' do + subject.define(:empty_attribute, '') + + expect { subject.empty_attribute } + .to raise_error ArgumentError, /empty_attribute/ + end +end diff --git a/qa/spec/scenario/bootable_spec.rb b/qa/spec/scenario/bootable_spec.rb new file mode 100644 index 00000000000..273aac7677e --- /dev/null +++ b/qa/spec/scenario/bootable_spec.rb @@ -0,0 +1,24 @@ +describe QA::Scenario::Bootable do + subject do + Class.new(QA::Scenario::Template) + .include(described_class) + end + + it 'makes it possible to define the scenario attribute' do + subject.class_eval do + attribute :something, '--something SOMETHING', 'Some attribute' + attribute :another, '--another ANOTHER', 'Some other attribute' + end + + expect(subject).to receive(:perform) + .with(something: 'test', another: 'other') + + subject.launch!(%w[--another other --something test]) + end + + it 'does not require attributes to be defined' do + expect(subject).to receive(:perform).with('some', 'argv') + + subject.launch!(%w[some argv]) + end +end diff --git a/spec/javascripts/sidebar/sidebar_service_spec.js b/spec/javascripts/sidebar/sidebar_service_spec.js deleted file mode 100644 index 7324d34d84a..00000000000 --- a/spec/javascripts/sidebar/sidebar_service_spec.js +++ /dev/null @@ -1,66 +0,0 @@ -import Vue from 'vue'; -import SidebarService from '~/sidebar/services/sidebar_service'; -import Mock from './mock_data'; - -describe('Sidebar service', () => { - beforeEach(() => { - Vue.http.interceptors.push(Mock.sidebarMockInterceptor); - this.service = new SidebarService({ - endpoint: '/gitlab-org/gitlab-shell/issues/5.json', - toggleSubscriptionEndpoint: '/gitlab-org/gitlab-shell/issues/5/toggle_subscription', - moveIssueEndpoint: '/gitlab-org/gitlab-shell/issues/5/move', - projectsAutocompleteEndpoint: '/autocomplete/projects?project_id=15', - }); - }); - - afterEach(() => { - SidebarService.singleton = null; - Vue.http.interceptors = _.without(Vue.http.interceptors, Mock.sidebarMockInterceptor); - }); - - it('gets the data', (done) => { - this.service.get() - .then((resp) => { - expect(resp).toBeDefined(); - done(); - }) - .then(done) - .catch(done.fail); - }); - - it('updates the data', (done) => { - this.service.update('issue[assignee_ids]', [1]) - .then((resp) => { - expect(resp).toBeDefined(); - }) - .then(done) - .catch(done.fail); - }); - - it('gets projects for autocomplete', (done) => { - this.service.getProjectsAutocomplete() - .then((resp) => { - expect(resp).toBeDefined(); - }) - .then(done) - .catch(done.fail); - }); - - it('moves the issue to another project', (done) => { - this.service.moveIssue(123) - .then((resp) => { - expect(resp).toBeDefined(); - }) - .then(done) - .catch(done.fail); - }); - - it('toggles the subscription', (done) => { - this.service.toggleSubscription() - .then((resp) => { - expect(resp).toBeDefined(); - }) - .then(done) - .catch(done.fail); - }); -}); diff --git a/spec/javascripts/vue_mr_widget/services/mr_widget_service_spec.js b/spec/javascripts/vue_mr_widget/services/mr_widget_service_spec.js deleted file mode 100644 index e667b4b3677..00000000000 --- a/spec/javascripts/vue_mr_widget/services/mr_widget_service_spec.js +++ /dev/null @@ -1,47 +0,0 @@ -import Vue from 'vue'; -import VueResource from 'vue-resource'; -import MRWidgetService from '~/vue_merge_request_widget/services/mr_widget_service'; - -Vue.use(VueResource); - -describe('MRWidgetService', () => { - const mr = { - mergePath: './', - mergeCheckPath: './', - cancelAutoMergePath: './', - removeWIPPath: './', - sourceBranchPath: './', - ciEnvironmentsStatusPath: './', - statusPath: './', - mergeActionsContentPath: './', - isServiceStore: true, - }; - - it('should have store and resources created in constructor', () => { - const service = new MRWidgetService(mr); - - expect(service.mergeResource).toBeDefined(); - expect(service.mergeCheckResource).toBeDefined(); - expect(service.cancelAutoMergeResource).toBeDefined(); - expect(service.removeWIPResource).toBeDefined(); - expect(service.removeSourceBranchResource).toBeDefined(); - expect(service.deploymentsResource).toBeDefined(); - expect(service.pollResource).toBeDefined(); - expect(service.mergeActionsContentResource).toBeDefined(); - }); - - it('should have methods defined', () => { - window.history.pushState({}, null, '/'); - const service = new MRWidgetService(mr); - - expect(service.merge()).toBeDefined(); - expect(service.cancelAutomaticMerge()).toBeDefined(); - expect(service.removeWIP()).toBeDefined(); - expect(service.removeSourceBranch()).toBeDefined(); - expect(service.fetchDeployments()).toBeDefined(); - expect(service.poll()).toBeDefined(); - expect(service.checkStatus()).toBeDefined(); - expect(service.fetchMergeActionsContent()).toBeDefined(); - expect(MRWidgetService.stopEnvironment()).toBeDefined(); - }); -}); diff --git a/spec/lib/gitlab/github_import/client_spec.rb b/spec/lib/gitlab/github_import/client_spec.rb index 6bbdf6bf3b6..5b2642d9473 100644 --- a/spec/lib/gitlab/github_import/client_spec.rb +++ b/spec/lib/gitlab/github_import/client_spec.rb @@ -185,6 +185,17 @@ describe Gitlab::GithubImport::Client do client.with_rate_limit { } end + + it 'ignores rate limiting when disabled' do + expect(client) + .to receive(:rate_limiting_enabled?) + .and_return(false) + + expect(client) + .not_to receive(:requests_remaining?) + + expect(client.with_rate_limit { 10 }).to eq(10) + end end describe '#requests_remaining?' do @@ -260,27 +271,122 @@ describe Gitlab::GithubImport::Client do end end - describe '#method_missing' do - it 'delegates missing methods to the request method' do - client = described_class.new('foo') + describe '#api_endpoint' do + let(:client) { described_class.new('foo') } + + context 'without a custom endpoint configured in Omniauth' do + it 'returns the default API endpoint' do + expect(client) + .to receive(:custom_api_endpoint) + .and_return(nil) - expect(client).to receive(:milestones).with(state: 'all') + expect(client.api_endpoint).to eq('https://api.github.com') + end + end - client.milestones(state: 'all') + context 'with a custom endpoint configured in Omniauth' do + it 'returns the custom endpoint' do + endpoint = 'https://github.kittens.com' + + expect(client) + .to receive(:custom_api_endpoint) + .and_return(endpoint) + + expect(client.api_endpoint).to eq(endpoint) + end end end - describe '#respond_to_missing?' do - it 'returns true for methods supported by Octokit' do - client = described_class.new('foo') + describe '#custom_api_endpoint' do + let(:client) { described_class.new('foo') } - expect(client.respond_to?(:milestones)).to eq(true) + context 'without a custom endpoint' do + it 'returns nil' do + expect(client) + .to receive(:github_omniauth_provider) + .and_return({}) + + expect(client.custom_api_endpoint).to be_nil + end end - it 'returns false for methods not supported by Octokit' do + context 'with a custom endpoint' do + it 'returns the API endpoint' do + endpoint = 'https://github.kittens.com' + + expect(client) + .to receive(:github_omniauth_provider) + .and_return({ 'args' => { 'client_options' => { 'site' => endpoint } } }) + + expect(client.custom_api_endpoint).to eq(endpoint) + end + end + end + + describe '#default_api_endpoint' do + it 'returns the default API endpoint' do client = described_class.new('foo') - expect(client.respond_to?(:kittens)).to eq(false) + expect(client.default_api_endpoint).to eq('https://api.github.com') + end + end + + describe '#verify_ssl' do + let(:client) { described_class.new('foo') } + + context 'without a custom configuration' do + it 'returns true' do + expect(client) + .to receive(:github_omniauth_provider) + .and_return({}) + + expect(client.verify_ssl).to eq(true) + end + end + + context 'with a custom configuration' do + it 'returns the configured value' do + expect(client.verify_ssl).to eq(false) + end + end + end + + describe '#github_omniauth_provider' do + let(:client) { described_class.new('foo') } + + context 'without a configured provider' do + it 'returns an empty Hash' do + expect(Gitlab.config.omniauth) + .to receive(:providers) + .and_return([]) + + expect(client.github_omniauth_provider).to eq({}) + end + end + + context 'with a configured provider' do + it 'returns the provider details as a Hash' do + hash = client.github_omniauth_provider + + expect(hash['name']).to eq('github') + expect(hash['url']).to eq('https://github.com/') + end + end + end + + describe '#rate_limiting_enabled?' do + let(:client) { described_class.new('foo') } + + it 'returns true when using GitHub.com' do + expect(client.rate_limiting_enabled?).to eq(true) + end + + it 'returns false for GitHub enterprise installations' do + expect(client) + .to receive(:api_endpoint) + .and_return('https://github.kittens.com/') + + expect(client.rate_limiting_enabled?).to eq(false) end end end diff --git a/spec/lib/gitlab/issuable_metadata_spec.rb b/spec/lib/gitlab/issuable_metadata_spec.rb index 2455969a183..42635a68ee1 100644 --- a/spec/lib/gitlab/issuable_metadata_spec.rb +++ b/spec/lib/gitlab/issuable_metadata_spec.rb @@ -1,8 +1,8 @@ require 'spec_helper' describe Gitlab::IssuableMetadata do - let(:user) { create(:user) } - let!(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace) } + let(:user) { create(:user) } + let!(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace) } subject { Class.new { include Gitlab::IssuableMetadata }.new } @@ -10,6 +10,10 @@ describe Gitlab::IssuableMetadata do expect(subject.issuable_meta_data(Issue.none, 'Issue')).to eq({}) end + it 'raises an error when given a collection with no limit' do + expect { subject.issuable_meta_data(Issue.all, 'Issue') }.to raise_error(/must have a limit/) + end + context 'issues' do let!(:issue) { create(:issue, author: user, project: project) } let!(:closed_issue) { create(:issue, state: :closed, author: user, project: project) } @@ -19,7 +23,7 @@ describe Gitlab::IssuableMetadata do let!(:closing_issues) { create(:merge_requests_closing_issues, issue: issue, merge_request: merge_request) } it 'aggregates stats on issues' do - data = subject.issuable_meta_data(Issue.all, 'Issue') + data = subject.issuable_meta_data(Issue.all.limit(10), 'Issue') expect(data.count).to eq(2) expect(data[issue.id].upvotes).to eq(1) @@ -42,7 +46,7 @@ describe Gitlab::IssuableMetadata do let!(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") } it 'aggregates stats on merge requests' do - data = subject.issuable_meta_data(MergeRequest.all, 'MergeRequest') + data = subject.issuable_meta_data(MergeRequest.all.limit(10), 'MergeRequest') expect(data.count).to eq(2) expect(data[merge_request.id].upvotes).to eq(1) diff --git a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb index 6d69b5305d2..ae1d8b47fe9 100644 --- a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb @@ -4,32 +4,40 @@ describe Gitlab::Metrics::SidekiqMiddleware do let(:middleware) { described_class.new } let(:message) { { 'args' => ['test'], 'enqueued_at' => Time.new(2016, 6, 23, 6, 59).to_f } } - def run(worker, message) - expect(Gitlab::Metrics::BackgroundTransaction).to receive(:new) - .with(worker.class) - .and_call_original - - expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:set) - .with(:sidekiq_queue_duration, instance_of(Float)) + describe '#call' do + it 'tracks the transaction' do + worker = double(:worker, class: double(:class, name: 'TestWorker')) - expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:finish) + expect(Gitlab::Metrics::BackgroundTransaction).to receive(:new) + .with(worker.class) + .and_call_original - middleware.call(worker, message, :test) { nil } - end + expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:set) + .with(:sidekiq_queue_duration, instance_of(Float)) - describe '#call' do - let(:test_worker_class) { double(:class, name: 'TestWorker') } - let(:worker) { double(:worker, class: test_worker_class) } + expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:finish) - it 'tracks the transaction' do - run(worker, message) + middleware.call(worker, message, :test) { nil } end it 'tracks the transaction (for messages without `enqueued_at`)' do - run(worker, {}) + worker = double(:worker, class: double(:class, name: 'TestWorker')) + + expect(Gitlab::Metrics::BackgroundTransaction).to receive(:new) + .with(worker.class) + .and_call_original + + expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:set) + .with(:sidekiq_queue_duration, instance_of(Float)) + + expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:finish) + + middleware.call(worker, {}, :test) { nil } end it 'tracks any raised exceptions' do + worker = double(:worker, class: double(:class, name: 'TestWorker')) + expect_any_instance_of(Gitlab::Metrics::Transaction) .to receive(:run).and_raise(RuntimeError) diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb index f18823b61ef..d9b3c2350b1 100644 --- a/spec/lib/gitlab/url_blocker_spec.rb +++ b/spec/lib/gitlab/url_blocker_spec.rb @@ -20,6 +20,22 @@ describe Gitlab::UrlBlocker do expect(described_class.blocked_url?('https://gitlab.com:25/foo/foo.git')).to be true end + it 'returns true for alternative version of 127.0.0.1 (0177.1)' do + expect(described_class.blocked_url?('https://0177.1:65535/foo/foo.git')).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (0x7f.1)' do + expect(described_class.blocked_url?('https://0x7f.1:65535/foo/foo.git')).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (2130706433)' do + expect(described_class.blocked_url?('https://2130706433:65535/foo/foo.git')).to be true + end + + it 'returns true for alternative version of 127.0.0.1 (127.000.000.001)' do + expect(described_class.blocked_url?('https://127.000.000.001:65535/foo/foo.git')).to be true + end + it 'returns true for a non-alphanumeric hostname' do stub_resolv diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index abe367d4e11..50f6c8b7d64 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -50,6 +50,12 @@ describe API::Projects do expect(json_response).to be_an Array expect(json_response.map { |p| p['id'] }).to contain_exactly(*projects.map(&:id)) end + + it 'returns the proper security headers' do + get api('/projects', current_user), filter + + expect(response).to include_security_headers + end end shared_examples_for 'projects response without N + 1 queries' do diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index dfe48e45d49..ba697e2b305 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -175,4 +175,25 @@ describe API::Services do end end end + + describe 'Mattermost service' do + let(:service_name) { 'mattermost' } + let(:params) do + { webhook: 'https://hook.example.com', username: 'username' } + end + + before do + project.create_mattermost_service( + active: true, + properties: params + ) + end + + it 'accepts a username for update' do + put api("/projects/#{project.id}/services/mattermost", user), params.merge(username: 'new_username') + + expect(response).to have_gitlab_http_status(200) + expect(json_response['properties']['username']).to eq('new_username') + end + end end diff --git a/spec/support/matchers/security_header_matcher.rb b/spec/support/matchers/security_header_matcher.rb new file mode 100644 index 00000000000..f8518d13ebb --- /dev/null +++ b/spec/support/matchers/security_header_matcher.rb @@ -0,0 +1,5 @@ +RSpec::Matchers.define :include_security_headers do |expected| + match do |actual| + expect(actual.headers).to include('X-Content-Type-Options') + end +end |