diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS new file mode 100644 index 0000000000000000000000000000000000000000..5b6e5a719fa56e97de2eeb88dcb4cd8e6980122e --- /dev/null +++ b/.gitlab/CODEOWNERS @@ -0,0 +1,15 @@ +# Backend Maintainers are the default for all ruby files +*.rb @ayufan @DouweM @dzaporozhets @grzesiek @nick.thomas @rspeicher @rymai @smcgivern +*.rake @ayufan @DouweM @dzaporozhets @grzesiek @nick.thomas @rspeicher @rymai @smcgivern + +# Technical writing team are the default reviewers for everything in `doc/` +/doc/ @axil @marcia + +# Frontend maintainers should see everything in `app/assets/` +app/assets/ @annabeldunstone @ClemMakesApps @fatihacet @filipa @iamphill @mikegreiling @timzallmann + +# Someone from the database team should review changes in `db/` +db/ @abrandl @NikolayS + +# Feature specific owners +/ee/lib/gitlab/code_owners/ @reprazent diff --git a/app/models/blob.rb b/app/models/blob.rb index acc64ffca67271d357d6161f348d8b9a1144f0a0..34f226c7ef697a2fee63048a655072acec2dea37 100644 --- a/app/models/blob.rb +++ b/app/models/blob.rb @@ -2,6 +2,8 @@ # Blob is a Rails-specific wrapper around Gitlab::Git::Blob objects class Blob < SimpleDelegator + prepend EE::Blob + CACHE_TIME = 60 # Cache raw blobs referred to by a (mutable) ref for 1 minute CACHE_TIME_IMMUTABLE = 3600 # Cache blobs referred to by an immutable reference for 1 hour diff --git a/app/models/concerns/case_sensitivity.rb b/app/models/concerns/case_sensitivity.rb index 6e80365ee5b80894443b84e9e2b627d327cce290..c93b6589ee78405653b12b1b30b1ffc7f9599a01 100644 --- a/app/models/concerns/case_sensitivity.rb +++ b/app/models/concerns/case_sensitivity.rb @@ -9,23 +9,46 @@ module CaseSensitivity # # Unlike other ActiveRecord methods this method only operates on a Hash. def iwhere(params) - criteria = self - cast_lower = Gitlab::Database.postgresql? + criteria = self params.each do |key, value| - column = ActiveRecord::Base.connection.quote_table_name(key) + criteria = case value + when Array + criteria.where(value_in(key, value)) + else + criteria.where(value_equal(key, value)) + end + end + + criteria + end - condition = - if cast_lower - "LOWER(#{column}) = LOWER(:value)" - else - "#{column} = :value" - end + private + + def value_equal(column, value) + lower_value = lower_value(value) + + lower_column(arel_table[column]).eq(lower_value).to_sql + end - criteria = criteria.where(condition, value: value) + def value_in(column, values) + lower_values = values.map do |value| + lower_value(value) end - criteria + lower_column(arel_table[column]).in(lower_values).to_sql + end + + def lower_value(value) + return value if Gitlab::Database.mysql? + + Arel::Nodes::NamedFunction.new('LOWER', [Arel::Nodes.build_quoted(value)]) + end + + def lower_column(column) + return column if Gitlab::Database.mysql? + + column.lower end end end diff --git a/app/models/user.rb b/app/models/user.rb index 6b749606355ef7a51c79b8685cfce242ef08c138..950311bd6f1155a739da3ec734e327c65a3824ba 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -266,6 +266,7 @@ def inactive_message scope :order_recent_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'DESC')) } scope :order_oldest_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'ASC')) } scope :confirmed, -> { where.not(confirmed_at: nil) } + scope :by_username, -> (usernames) { iwhere(username: usernames) } # Limits the users to those that have TODOs, optionally in the given state. # @@ -457,11 +458,11 @@ def by_login(login) end def find_by_username(username) - iwhere(username: username).take + by_username(username).take end def find_by_username!(username) - iwhere(username: username).take! + by_username(username).take! end def find_by_personal_access_token(token_string) diff --git a/app/views/projects/blob/_blob.html.haml b/app/views/projects/blob/_blob.html.haml index a4b1b496b69cde2644b5bfc7d23dc3c1db0fccfc..bb4bbeb6b384fd7597be573c4b098bf0c18c1670 100644 --- a/app/views/projects/blob/_blob.html.haml +++ b/app/views/projects/blob/_blob.html.haml @@ -4,7 +4,7 @@ .well-segment %ul.blob-commit-info = render 'projects/commits/commit', commit: @last_commit, project: @project, ref: @ref - + = render_if_exists 'projects/blob/owners', blob: blob = render "projects/blob/auxiliary_viewer", blob: blob #blob-content-holder.blob-content-holder diff --git a/doc/user/project/code_owners.md b/doc/user/project/code_owners.md new file mode 100644 index 0000000000000000000000000000000000000000..9d4e6cb8a781888429bd5a6bdf87cb4ba977e602 --- /dev/null +++ b/doc/user/project/code_owners.md @@ -0,0 +1,85 @@ +# Code owners + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/6916) +in [Gitlab Starter](https://about.gitlab.com/pricing/) 11.3. + +You can use a `CODEOWNERS` file to specify users that are responsible +for certain files in a repository. + +You can choose and add the `CODEOWNERS` file in three places: + +- to the root directory of the repository +- inside the `.gitlab/` directory +- inside the `docs/` directory + +The `CODEOWNERS` file is scoped to a branch, which means that with the +introduction of new files, the person adding the new content can +specify themselves as a code owner, all before the new changes +get merged to the default branch. + +When a file matches multiple entries in the `CODEOWNERS` file, +the users from all entries are displayed on the blob page of +the given file. + +## The syntax of a code owners file + +Files can be specified using the same kind of patterns you would use +in the `.gitignore` file followed by the `@username` or email of one +or more users that should be owners of the file. + +The order in which the paths are defined is significant: the last +pattern that matches a given path will be used to find the code +owners. + +Starting a line with a `#` indicates a comment. This needs to be +escaped using `\#` to address files for which the name starts with a +`#`. + +Example `CODEOWNERS` file: + +``` +# This is an example code owners file, lines starting with a `#` will +# be ignored. + +# app/ @commented-rule + +# We can specifiy a default match using wildcards: +* @default-codeowner + +# Rules defined later in the file take precedence over the rules +# defined before. +# This will match all files for which the file name ends in `.rb` +*.rb @ruby-owner + +# Files with a `#` can still be accesssed by escaping the pound sign +\#file_with_pound.rb @owner-file-with-pound + +# Multiple codeowners can be specified, separated by whitespace +CODEOWNERS @multiple @owners @tab-separated + +# Both usernames or email addresses can be used to match +# users. Everything else will be ignored. For example this will +# specify `@legal` and a user with email `janedoe@gitlab.com` as the +# owner for the LICENSE file +LICENSE @legal this_does_not_match janedoe@gitlab.com + +# Ending a path in a `/` will specify the code owners for every file +# nested in that directory, on any level +/docs/ @all-docs + +# Ending a path in `/*` will specify code owners for every file in +# that directory, but not nested deeper. This will match +# `docs/index.md` but not `docs/projects/index.md` +/docs/* @root-docs + +# This will make a `lib` directory nested anywhere in the repository +# match +lib/ @lib-owner + +# This will only match a `config` directory in the root of the +# repository +/config/ @config-owner + +# If the path contains spaces, these need to be escaped like this: +path\ with\ spaces/ @space-owner +``` diff --git a/doc/user/project/index.md b/doc/user/project/index.md index 1aeec6ec9d3e5c3c4844481ffd697c62aefd3dfe..e1677218447db42a46cdfe5e2b9cd2d8f6a6035c 100644 --- a/doc/user/project/index.md +++ b/doc/user/project/index.md @@ -87,6 +87,7 @@ website with GitLab Pages your code blocks, overriding GitLab's default choice of language - [Badges](badges.md): Badges for the project overview - [Maven packages](maven_packages.md): Your private Maven repository in GitLab +- [Code owners](code_owners.md): Specify code owners for certain files **[STARTER]** ### Project's integrations diff --git a/ee/app/helpers/ee/users_helper.rb b/ee/app/helpers/ee/users_helper.rb new file mode 100644 index 0000000000000000000000000000000000000000..85ca4091e3a51bf596d089ece1f566ee5989bafc --- /dev/null +++ b/ee/app/helpers/ee/users_helper.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module EE + module UsersHelper + def users_sentence(users, link_class: nil) + users.map { |user| link_to(user.name, user, class: link_class) }.to_sentence.html_safe + end + end +end diff --git a/ee/app/models/ee/blob.rb b/ee/app/models/ee/blob.rb new file mode 100644 index 0000000000000000000000000000000000000000..6f3183a17ae5209518fb69b9e468d11c1a9438f4 --- /dev/null +++ b/ee/app/models/ee/blob.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module EE + module Blob + extend ActiveSupport::Concern + + def owners + @owners ||= ::Gitlab::CodeOwners.for_blob(self) + end + end +end diff --git a/ee/app/models/ee/repository.rb b/ee/app/models/ee/repository.rb index 6d712eca676bf53b561b1c0189ecd4a14e6a6c3d..ada64c4bcd2a62c08dc76646a9e792651471418d 100644 --- a/ee/app/models/ee/repository.rb +++ b/ee/app/models/ee/repository.rb @@ -87,5 +87,10 @@ def log_geo_updated_event def geo_updated_event_source is_wiki ? Geo::RepositoryUpdatedEvent::WIKI : Geo::RepositoryUpdatedEvent::REPOSITORY end + + def code_owners_blob(ref: 'HEAD') + possible_code_owner_blobs = ::Gitlab::CodeOwners::FILE_PATHS.map { |path| [ref, path] } + blobs_at(possible_code_owner_blobs).compact.first + end end end diff --git a/ee/app/models/license.rb b/ee/app/models/license.rb index 9c316a3fa79ecfa3332bd4c0c810b3eb1371319f..5d9a51248ba02a20ab6fe475d4501aa7ae3b3116 100644 --- a/ee/app/models/license.rb +++ b/ee/app/models/license.rb @@ -9,6 +9,7 @@ class License < ActiveRecord::Base EES_FEATURES = %i[ audit_events burndown_charts + code_owners contribution_analytics elastic_search export_issues diff --git a/ee/app/views/projects/blob/_owners.html.haml b/ee/app/views/projects/blob/_owners.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..b86543a9d2e27768c9d3577f8ac81849412a374b --- /dev/null +++ b/ee/app/views/projects/blob/_owners.html.haml @@ -0,0 +1,9 @@ +- return if blob.owners.empty? + +.well-segment.blob-auxiliary-viewer.file-owner-content + = sprite_icon('users', size: 18, css_class: 'icon') + %strong + = _("Code owners") + = link_to icon('question-circle'), help_page_path('user/project/code_owners'), title: 'About this feature', target: '_blank' + : + = users_sentence(blob.owners, link_class: 'file-owner-link') diff --git a/ee/changelogs/unreleased/bvl-codeowners-file.yml b/ee/changelogs/unreleased/bvl-codeowners-file.yml new file mode 100644 index 0000000000000000000000000000000000000000..26229fb7b3330d00377f8e928d64c3746ccfcee2 --- /dev/null +++ b/ee/changelogs/unreleased/bvl-codeowners-file.yml @@ -0,0 +1,5 @@ +--- +title: Allow specifying code owners in a CODEOWNERS file +merge_request: 6916 +author: +type: added diff --git a/ee/lib/gitlab/code_owners.rb b/ee/lib/gitlab/code_owners.rb new file mode 100644 index 0000000000000000000000000000000000000000..d7ae7dd856bae5a16efe9bd81018ebb75de54b49 --- /dev/null +++ b/ee/lib/gitlab/code_owners.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Gitlab + module CodeOwners + FILE_NAME = 'CODEOWNERS' + FILE_PATHS = [FILE_NAME, "docs/#{FILE_NAME}", ".gitlab/#{FILE_NAME}"].freeze + + def self.for_blob(blob) + if blob.project.feature_available?(:code_owners) + Loader.new(blob.project, blob.commit_id, blob.path).users + else + User.none + end + end + end +end diff --git a/ee/lib/gitlab/code_owners/file.rb b/ee/lib/gitlab/code_owners/file.rb new file mode 100644 index 0000000000000000000000000000000000000000..f680c39727aea39ca8ebd93136503ecfff53b47d --- /dev/null +++ b/ee/lib/gitlab/code_owners/file.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +module Gitlab + module CodeOwners + class File + def initialize(blob) + @blob = blob + end + + def parsed_data + @parsed_data ||= get_parsed_data + end + + def empty? + parsed_data.empty? + end + + def owners_for_path(path) + matching_pattern = parsed_data.keys.reverse.detect do |pattern| + path_matches?(pattern, path) + end + + parsed_data[matching_pattern] + end + + private + + def data + if @blob && !@blob.binary? + @blob.data + else + "" + end + end + + def get_parsed_data + parsed = {} + + data.lines.each do |line| + line = line.strip + next unless line.present? + next if line.starts_with?('#') + + pattern, _separator, owners = line.partition(/(? Code owners', :js do + let(:project) { create(:project, :private, :repository) } + let(:user) { project.owner } + let(:code_owner) { create(:user, username: 'documentation-owner') } + + before do + sign_in(user) + project.add_developer(code_owner) + end + + def visit_blob(path, anchor: nil, ref: 'master') + visit project_blob_path(project, File.join(ref, path), anchor: anchor) + + wait_for_requests + end + + context 'when there is a codeowners file' do + context 'when the feature is available' do + before do + stub_licensed_features(code_owners: true) + end + + it 'shows the code owners related to a file' do + visit_blob('docs/CODEOWNERS', ref: 'with-codeowners') + + within('.file-owner-content') do + expect(page).to have_content('Code owners') + expect(page).to have_link(code_owner.name) + end + end + + it 'does not show the code owners banner when there are no code owners' do + visit_blob('README.md') + + expect(page).not_to have_content('Code owners:') + end + end + + context 'when the feature is not available' do + before do + stub_licensed_features(code_owners: false) + end + + it 'does not show the code owners related to a file' do + visit_blob('docs/CODEOWNERS', ref: 'with-codeowners') + + expect(page).not_to have_content('Code owners') + end + end + end +end diff --git a/ee/spec/fixtures/codeowners_example b/ee/spec/fixtures/codeowners_example new file mode 100644 index 0000000000000000000000000000000000000000..d106d18b1dea737d63fa622a72bf807c9f66d7cd --- /dev/null +++ b/ee/spec/fixtures/codeowners_example @@ -0,0 +1,44 @@ +# This is an example code owners file, lines starting with a `#` will +# be ignored. + +# app/ @commented-rule + +# We can specifiy a default match using wildcards: +* @default-codeowner + +# Rules defined later in the file take precedence over the rules +# defined before. +# This will match all files for which the file name ends in `.rb` +*.rb @ruby-owner + +# Files with a `#` can still be accesssed by escaping the pound sign +\#file_with_pound.rb @owner-file-with-pound + +# Multiple codeowners can be specified, separated by whitespace +CODEOWNERS @multiple @owners @tab-separated + +# Both usernames or email addresses can be used to match +# users. Everything else will be ignored. For example this will +# specify `@legal` and a user with email `janedoe@gitlab.com` as the +# owner for the LICENSE file +LICENSE @legal this does not match janedoe@gitlab.com + +# Ending a path in a `/` will specify the code owners for every file +# nested in that directory, on any level +/docs/ @all-docs + +# Ending a path in `/*` will specify code owners for every file in +# that directory, but not nested deeper. This will match +# `docs/index.md` but not `docs/projects/index.md` +/docs/* @root-docs + +# This will make a `lib` directory nested anywhere in the repository +# match +lib/ @lib-owner + +# This will only match a `config` directory in the root of the +# repository +/config/ @config-owner + +# If the path contains spaces, these need to be escaped like this: +path\ with\ spaces/ @space-owner diff --git a/ee/spec/lib/gitlab/code_owners/file_spec.rb b/ee/spec/lib/gitlab/code_owners/file_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..355636a1021a6992ba8bcbac17fc3b26410b2962 --- /dev/null +++ b/ee/spec/lib/gitlab/code_owners/file_spec.rb @@ -0,0 +1,164 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::CodeOwners::File do + include FakeBlobHelpers + + let(:project) { build(:project) } + let(:file_content) do + File.read(Rails.root.join('ee', 'spec', 'fixtures', 'codeowners_example')) + end + + let(:blob) { fake_blob(path: 'CODEOWNERS', data: file_content) } + + subject(:file) { described_class.new(blob) } + + describe '#parsed_data' do + it 'parses all the required lines' do + expected_patterns = [ + '*', '**#file_with_pound.rb', '*.rb', '**CODEOWNERS', '**LICENSE', 'docs/', + 'docs/*', 'config/', '**lib/', '**path with spaces/' + ] + expect(file.parsed_data.keys) + .to contain_exactly(*expected_patterns) + end + + it 'allows usernames and emails' do + expect(file.parsed_data['**LICENSE']).to include('legal', 'janedoe@gitlab.com') + end + + context 'when there are entries that do not look like user references' do + let(:file_content) do + "a-path/ this is all random @username and email@gitlab.org" + end + + it 'ignores the entries' do + expect(file.parsed_data['**a-path/']).to include('username', 'email@gitlab.org') + end + end + end + + describe '#empty?' do + subject { file.empty? } + + it { is_expected.to be(false) } + + context 'when there is no content' do + let(:file_content) { "" } + + it { is_expected.to be(true) } + end + + context 'when the file is binary' do + let(:blob) { fake_blob(binary: true) } + + it { is_expected.to be(true) } + end + + context 'when the file did not exist' do + let(:blob) { nil } + + it { is_expected.to be(true) } + end + end + + describe '#owners_for_path' do + context 'for a path without matches' do + let(:file_content) do + <<~CONTENT + # Simulating a CODOWNERS without entries + CONTENT + end + + it 'returns an nil for an unmatched path' do + owners = file.owners_for_path('no_matches') + + expect(owners).to be_nil + end + end + + it 'matches random files to a pattern' do + owners = file.owners_for_path('app/assets/something.vue') + + expect(owners).to include('default-codeowner') + end + + it 'uses the last pattern if multiple patterns match' do + owners = file.owners_for_path('hello.rb') + + expect(owners).to eq('@ruby-owner') + end + + it 'returns the usernames for a file matching a pattern with a glob' do + owners = file.owners_for_path('app/models/repository.rb') + + expect(owners).to eq('@ruby-owner') + end + + it 'allows specifying multiple users' do + owners = file.owners_for_path('CODEOWNERS') + + expect(owners).to include('multiple', 'owners', 'tab-separated') + end + + it 'returns emails and usernames for a matched pattern' do + owners = file.owners_for_path('LICENSE') + + expect(owners).to include('legal', 'janedoe@gitlab.com') + end + + it 'allows escaping the pound sign used for comments' do + owners = file.owners_for_path('examples/#file_with_pound.rb') + + expect(owners).to include('owner-file-with-pound') + end + + it 'returns the usernames for a file nested in a directory' do + owners = file.owners_for_path('docs/projects/index.md') + + expect(owners).to include('all-docs') + end + + it 'returns the usernames for a pattern matched with a glob in a folder' do + owners = file.owners_for_path('docs/index.md') + + expect(owners).to include('root-docs') + end + + it 'allows matching files nested anywhere in the repository', :aggregate_failures do + lib_owners = file.owners_for_path('lib/gitlab/git/repository.rb') + other_lib_owners = file.owners_for_path('ee/lib/gitlab/git/repository.rb') + + expect(lib_owners).to include('lib-owner') + expect(other_lib_owners).to include('lib-owner') + end + + it 'allows allows limiting the matching files to the root of the repository', :aggregate_failures do + config_owners = file.owners_for_path('config/database.yml') + other_config_owners = file.owners_for_path('other/config/database.yml') + + expect(config_owners).to include('config-owner') + expect(other_config_owners).to eq('@default-codeowner') + end + + it 'correctly matches paths with spaces' do + owners = file.owners_for_path('path with spaces/README.md') + + expect(owners).to eq('@space-owner') + end + + context 'paths with whitespaces and username lookalikes' do + let(:file_content) do + 'a/weird\ path\ with/\ @username\ /\ and-email@lookalikes.com\ / @user-1 email@gitlab.org @user-2' + end + + it 'parses correctly' do + owners = file.owners_for_path('a/weird path with/ @username / and-email@lookalikes.com /test.rb') + + expect(owners).to include('user-1', 'user-2', 'email@gitlab.org') + expect(owners).not_to include('username', 'and-email@lookalikes.com') + end + end + end +end diff --git a/ee/spec/lib/gitlab/code_owners/loader_spec.rb b/ee/spec/lib/gitlab/code_owners/loader_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..f2901f6836bb0cfa5e3b2d38e71bb3b7e26d0a7a --- /dev/null +++ b/ee/spec/lib/gitlab/code_owners/loader_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::CodeOwners::Loader do + include FakeBlobHelpers + set(:group) { create(:group) } + set(:project) { create(:project, namespace: group) } + subject(:loader) { described_class.new(project, 'with-codeowners', path) } + + let!(:owner_1) { create(:user, username: 'owner-1') } + let!(:email_owner) { create(:user, username: 'owner-2') } + let!(:owner_3) { create(:user, username: 'owner-3') } + let!(:documentation_owner) { create(:user, username: 'documentation-owner') } + let(:codeowner_content) do + <<~CODEOWNERS + docs/* @documentation-owner + docs/CODEOWNERS @owner-1 owner2@gitlab.org @owner-3 @documentation-owner + CODEOWNERS + end + let(:codeowner_blob) { fake_blob(path: 'CODEOWNERS', data: codeowner_content) } + let(:path) { 'docs/CODEOWNERS' } + + before do + create(:email, user: email_owner, email: 'owner2@gitlab.org') + project.add_developer(owner_1) + project.add_developer(email_owner) + project.add_developer(documentation_owner) + + allow(project.repository).to receive(:code_owners_blob).and_return(codeowner_blob) + end + + describe '#users' do + context 'with a CODEOWNERS file' do + context 'for a path with code owners' do + it 'returns all existing users that are members of the project' do + expect(loader.users).to contain_exactly(owner_1, email_owner, documentation_owner) + end + + it 'does not return users that are not members of the project' do + expect(loader.users).not_to include(owner_3) + end + + it 'includes group members of the project' do + group.add_developer(owner_3) + + expect(loader.users).to include(owner_3) + end + end + + context 'for another path' do + let(:path) { 'no-codeowner' } + + it 'returns no users' do + expect(loader.users).to be_empty + end + end + end + + context 'when there is no codeowners file' do + let(:codeowner_blob) { nil } + + it 'returns no users without failing' do + expect(loader.users).to be_empty + end + end + + context 'with the request store', :request_store do + it 'only calls out to the repository once' do + expect(project.repository).to receive(:code_owners_blob).once + + 2.times { loader.users } + end + + it 'only processes the file once' do + code_owners_file = loader.__send__(:code_owners_file) + + expect(code_owners_file).to receive(:get_parsed_data).once.and_call_original + + 2.times { loader.users } + end + end + end +end diff --git a/ee/spec/lib/gitlab/code_owners_spec.rb b/ee/spec/lib/gitlab/code_owners_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..49d86effe0b082c9d338607b6cad7eb1ab3708bf --- /dev/null +++ b/ee/spec/lib/gitlab/code_owners_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::CodeOwners do + include FakeBlobHelpers + + let!(:code_owner) { create(:user, username: 'owner-1') } + let(:project) { create(:project, :repository) } + let(:blob) do + project.repository.blob_at(TestEnv::BRANCH_SHA['with-codeowners'], 'docs/CODEOWNERS') + end + let(:codeowner_content) { "docs/CODEOWNERS @owner-1" } + let(:codeowner_blob) { fake_blob(path: 'CODEOWNERS', data: codeowner_content) } + + before do + project.add_developer(code_owner) + allow(project.repository).to receive(:code_owners_blob).and_return(codeowner_blob) + end + + describe '.for_blob' do + context 'when the feature is available' do + before do + stub_licensed_features(code_owners: true) + end + + it 'returns users for a blob' do + expect(described_class.for_blob(blob)).to include(code_owner) + end + end + + context 'when the feature is not available' do + before do + stub_licensed_features(code_owners: false) + end + + it 'returns no users' do + expect(described_class.for_blob(blob)).to be_empty + end + end + end +end diff --git a/ee/spec/models/blob_spec.rb b/ee/spec/models/blob_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..2772e8a534c013ae33c46fa4e3795414092ffdbd --- /dev/null +++ b/ee/spec/models/blob_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Blob do + let(:project) { create(:project, :repository) } + let(:blob) do + project.repository.blob_at(TestEnv::BRANCH_SHA['with-codeowners'], 'docs/CODEOWNERS') + end + let(:code_owner) { create(:user, username: 'documentation-owner') } + + before do + project.add_developer(code_owner) + end + + describe '#owners' do + context 'when the feature is available' do + before do + stub_licensed_features(code_owners: true) + end + + it 'returns the owners from the file' do + expect(blob.owners).to include(code_owner) + end + end + + context 'when the feature is not available' do + before do + stub_licensed_features(code_owners: false) + end + + it 'returns no code owners' do + expect(blob.owners).to be_empty + end + end + end +end diff --git a/ee/spec/models/repository_spec.rb b/ee/spec/models/repository_spec.rb index b397a5824be94bbd9cf486223058401b6eaac758..df382e318adbb0a55707505529e254d9298dca6d 100644 --- a/ee/spec/models/repository_spec.rb +++ b/ee/spec/models/repository_spec.rb @@ -154,4 +154,24 @@ def keys_should_not_be_set end end end + + describe '#code_owners_blob' do + it 'returns nil if there is no codeowners file' do + expect(repository.code_owners_blob(ref: 'master')).to be_nil + end + + it 'returns the content of the codeowners file when it is found' do + expect(repository.code_owners_blob(ref: 'with-codeowners').data).to include('example CODEOWNERS file') + end + + it 'requests the CODOWNER blobs in batch in the correct order' do + expect(repository).to receive(:blobs_at) + .with([%w(HEAD CODEOWNERS), + %w(HEAD docs/CODEOWNERS), + %w(HEAD .gitlab/CODEOWNERS)]) + .and_call_original + + repository.code_owners_blob + end + end end diff --git a/lib/gitlab/user_extractor.rb b/lib/gitlab/user_extractor.rb new file mode 100644 index 0000000000000000000000000000000000000000..3ede0a5b5e6b12a34fc5f19842dd429533063ca5 --- /dev/null +++ b/lib/gitlab/user_extractor.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +# This class extracts all users found in a piece of text by the username or the +# email adress + +module Gitlab + class UserExtractor + # Not using `Devise.email_regexp` to filter out any chars that an email + # does not end with and not pinning the email to a start of end of a string. + EMAIL_REGEXP = /(?([^@\s]+@[^@\s]+(? 'bar')).to eq(criteria) - end - end - - describe 'with multiple column/value pairs' do - it 'returns the criteria for a column and a value' do - initial = double(:criteria) - final = double(:criteria) - - expect(connection).to receive(:quote_table_name) - .with(:foo) - .and_return('"foo"') - - expect(connection).to receive(:quote_table_name) - .with(:bar) - .and_return('"bar"') - - expect(model).to receive(:where) - .with(%q{LOWER("foo") = LOWER(:value)}, value: 'bar') - .and_return(initial) - - expect(initial).to receive(:where) - .with(%q{LOWER("bar") = LOWER(:value)}, value: 'baz') - .and_return(final) - - got = model.iwhere(foo: 'bar', bar: 'baz') - - expect(got).to eq(final) - end - - it 'returns the criteria for a column with a table, and a value' do - initial = double(:criteria) - final = double(:criteria) - - expect(connection).to receive(:quote_table_name) - .with(:'foo.bar') - .and_return('"foo"."bar"') - - expect(connection).to receive(:quote_table_name) - .with(:'foo.baz') - .and_return('"foo"."baz"') - - expect(model).to receive(:where) - .with(%q{LOWER("foo"."bar") = LOWER(:value)}, value: 'bar') - .and_return(initial) - - expect(initial).to receive(:where) - .with(%q{LOWER("foo"."baz") = LOWER(:value)}, value: 'baz') - .and_return(final) - - got = model.iwhere('foo.bar'.to_sym => 'bar', - 'foo.baz'.to_sym => 'baz') - - expect(got).to eq(final) - end + let(:model) do + Class.new(ActiveRecord::Base) do + include CaseSensitivity + self.table_name = 'namespaces' end end - describe 'using MySQL' do - before do - allow(Gitlab::Database).to receive(:postgresql?).and_return(false) - allow(Gitlab::Database).to receive(:mysql?).and_return(true) - end - - describe 'with a single column/value pair' do - it 'returns the criteria for a column and a value' do - criteria = double(:criteria) - - expect(connection).to receive(:quote_table_name) - .with(:foo) - .and_return('`foo`') - - expect(model).to receive(:where) - .with(%q{`foo` = :value}, value: 'bar') - .and_return(criteria) + let!(:model_1) { model.create(path: 'mOdEl-1', name: 'mOdEl 1') } + let!(:model_2) { model.create(path: 'mOdEl-2', name: 'mOdEl 2') } - expect(model.iwhere(foo: 'bar')).to eq(criteria) - end + it 'finds a single instance by a single attribute regardless of case' do + expect(model.iwhere(path: 'MODEL-1')).to contain_exactly(model_1) + end - it 'returns the criteria for a column with a table, and a value' do - criteria = double(:criteria) + it 'finds multiple instances by a single attribute regardless of case' do + expect(model.iwhere(path: %w(MODEL-1 model-2))).to contain_exactly(model_1, model_2) + end - expect(connection).to receive(:quote_table_name) - .with(:'foo.bar') - .and_return('`foo`.`bar`') + it 'finds instances by multiple attributes' do + expect(model.iwhere(path: %w(MODEL-1 model-2), name: 'model 1')) + .to contain_exactly(model_1) + end - expect(model).to receive(:where) - .with(%q{`foo`.`bar` = :value}, value: 'bar') - .and_return(criteria) + # Using `mysql` & `postgresql` metadata-tags here because both adapters build + # the query slightly differently + context 'for MySQL', :mysql do + it 'builds a simple query' do + query = model.iwhere(path: %w(MODEL-1 model-2), name: 'model 1').to_sql + expected_query = <<~QRY.strip + SELECT `namespaces`.* FROM `namespaces` WHERE (`namespaces`.`path` IN ('MODEL-1', 'model-2')) AND (`namespaces`.`name` = 'model 1') + QRY - expect(model.iwhere('foo.bar'.to_sym => 'bar')) - .to eq(criteria) - end + expect(query).to eq(expected_query) end + end - describe 'with multiple column/value pairs' do - it 'returns the criteria for a column and a value' do - initial = double(:criteria) - final = double(:criteria) - - expect(connection).to receive(:quote_table_name) - .with(:foo) - .and_return('`foo`') - - expect(connection).to receive(:quote_table_name) - .with(:bar) - .and_return('`bar`') - - expect(model).to receive(:where) - .with(%q{`foo` = :value}, value: 'bar') - .and_return(initial) - - expect(initial).to receive(:where) - .with(%q{`bar` = :value}, value: 'baz') - .and_return(final) - - got = model.iwhere(foo: 'bar', bar: 'baz') - - expect(got).to eq(final) - end - - it 'returns the criteria for a column with a table, and a value' do - initial = double(:criteria) - final = double(:criteria) - - expect(connection).to receive(:quote_table_name) - .with(:'foo.bar') - .and_return('`foo`.`bar`') - - expect(connection).to receive(:quote_table_name) - .with(:'foo.baz') - .and_return('`foo`.`baz`') - - expect(model).to receive(:where) - .with(%q{`foo`.`bar` = :value}, value: 'bar') - .and_return(initial) - - expect(initial).to receive(:where) - .with(%q{`foo`.`baz` = :value}, value: 'baz') - .and_return(final) - - got = model.iwhere('foo.bar'.to_sym => 'bar', - 'foo.baz'.to_sym => 'baz') + context 'for PostgreSQL', :postgresql do + it 'builds a query using LOWER' do + query = model.iwhere(path: %w(MODEL-1 model-2), name: 'model 1').to_sql + expected_query = <<~QRY.strip + SELECT \"namespaces\".* FROM \"namespaces\" WHERE (LOWER(\"namespaces\".\"path\") IN (LOWER('MODEL-1'), LOWER('model-2'))) AND (LOWER(\"namespaces\".\"name\") = LOWER('model 1')) + QRY - expect(got).to eq(final) - end + expect(query).to eq(expected_query) end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index fd367d624ecbae89dc35614835e39a1201331456..4f981cda2e6919113b6d0900bbc3ada362e2da6a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -433,6 +433,23 @@ end end end + + describe '.by_username' do + it 'finds users regardless of the case passed' do + user = create(:user, username: 'CaMeLcAsEd') + user2 = create(:user, username: 'UPPERCASE') + + expect(described_class.by_username(%w(CAMELCASED uppercase))) + .to contain_exactly(user, user2) + end + + it 'finds a single user regardless of the case passed' do + user = create(:user, username: 'CaMeLcAsEd') + + expect(described_class.by_username('CAMELCASED')) + .to contain_exactly(user) + end + end end describe "Respond to" do diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index 21103771d1f742748ca53224e3b66772c44224a6..3f8e3ae5190c57267480fdc8a2e5b46cd212d56c 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -52,7 +52,8 @@ module TestEnv 'add_images_and_changes' => '010d106', 'update-gitlab-shell-v-6-0-1' => '2f61d70', 'update-gitlab-shell-v-6-0-3' => 'de78448', - '2-mb-file' => 'bf12d25' + '2-mb-file' => 'bf12d25', + 'with-codeowners' => '219560e' }.freeze # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily