diff options
36 files changed, 446 insertions, 90 deletions
@@ -301,7 +301,7 @@ gem 'sentry-raven', '~> 2.9' gem 'premailer-rails', '~> 1.10.3' # LabKit: Tracing and Correlation -gem 'gitlab-labkit', '0.10.1' +gem 'gitlab-labkit', '0.11.0' # I18n gem 'ruby_parser', '~> 3.8', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 89cee333738..75c560aa2e9 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -380,7 +380,7 @@ GEM github-markup (1.7.0) gitlab-chronic (0.10.5) numerizer (~> 0.2) - gitlab-labkit (0.10.1) + gitlab-labkit (0.11.0) actionpack (>= 5.0.0, < 6.1.0) activesupport (>= 5.0.0, < 6.1.0) grpc (~> 1.19) @@ -1232,7 +1232,7 @@ DEPENDENCIES gitaly (~> 1.86.0) github-markup (~> 1.7.0) gitlab-chronic (~> 0.10.5) - gitlab-labkit (= 0.10.1) + gitlab-labkit (= 0.11.0) gitlab-license (~> 1.0) gitlab-mail_room (~> 0.0.3) gitlab-markup (~> 1.7.0) diff --git a/app/assets/javascripts/deploy_keys/components/key.vue b/app/assets/javascripts/deploy_keys/components/key.vue index c856e380c41..585b091bc51 100644 --- a/app/assets/javascripts/deploy_keys/components/key.vue +++ b/app/assets/javascripts/deploy_keys/components/key.vue @@ -1,5 +1,5 @@ <script> -import _ from 'underscore'; +import { head, tail } from 'lodash'; import { s__, sprintf } from '~/locale'; import icon from '~/vue_shared/components/icon.vue'; import tooltip from '~/vue_shared/directives/tooltip'; @@ -48,8 +48,7 @@ export default { const projects = [...this.deployKey.deploy_keys_projects]; if (this.projectId !== null) { - const indexOfCurrentProject = _.findIndex( - projects, + const indexOfCurrentProject = projects.findIndex( project => project && project.project && @@ -66,10 +65,10 @@ export default { return projects; }, firstProject() { - return _.head(this.projects); + return head(this.projects); }, restProjects() { - return _.tail(this.projects); + return tail(this.projects); }, restProjectsTooltip() { return sprintf(s__('DeployKeys|Expand %{count} other projects'), { diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue index e0fb1226674..9544fbe9fc5 100644 --- a/app/assets/javascripts/diffs/components/diff_table_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue @@ -166,7 +166,6 @@ export default { :href="lineHref" @click="setHighlightedRow(lineCode)" > - {{ lineNumber }} </a> <diff-gutter-avatars v-if="shouldShowAvatarsOnGutter" diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index e83450f2dbc..24c6fec064a 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -485,6 +485,10 @@ table.code { } } } + + &:not(.js-unfold-bottom) a::before { + content: attr(data-linenumber); + } } &.line_content { diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index e2dd2d77a30..01c23ce9db7 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -26,6 +26,8 @@ class SessionsController < Devise::SessionsController before_action :load_recaptcha before_action :frontend_tracking_data, only: [:new] + around_action :set_current_context + after_action :log_failed_login, if: :action_new_and_failed_login? helper_method :captcha_enabled?, :captcha_on_login_required? @@ -305,6 +307,13 @@ class SessionsController < Devise::SessionsController # We want tracking data pushed to the frontend when the user is _in_ the control group frontend_experimentation_tracking_data(:signup_flow, 'start') unless experiment_enabled?(:signup_flow) end + + def set_current_context(&block) + Gitlab::ApplicationContext.with_context( + user: -> { current_user }, + caller_id: "#{self.class.name}##{action_name}", + &block) + end end SessionsController.prepend_if_ee('EE::SessionsController') diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 5c2e03e4b9c..a24428dee50 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -307,7 +307,7 @@ class Snippet < ApplicationRecord end class << self - # Searches for snippets with a matching title or file name. + # Searches for snippets with a matching title, description or file name. # # This method uses ILIKE on PostgreSQL and LIKE on MySQL. # @@ -315,7 +315,7 @@ class Snippet < ApplicationRecord # # Returns an ActiveRecord::Relation. def search(query) - fuzzy_search(query, [:title, :file_name]) + fuzzy_search(query, [:title, :description, :file_name]) end # Searches for snippets with matching content. diff --git a/changelogs/unreleased/14707-add-nginx-throughput.yml b/changelogs/unreleased/14707-add-nginx-throughput.yml deleted file mode 100644 index 66fb7757136..00000000000 --- a/changelogs/unreleased/14707-add-nginx-throughput.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Add nginx request aggregations to WAF anomaly service -merge_request: 25273 -author: -type: added diff --git a/changelogs/unreleased/196688-replace-underscore-with-lodash-for-app-assets-javascripts-deploy_k.yml b/changelogs/unreleased/196688-replace-underscore-with-lodash-for-app-assets-javascripts-deploy_k.yml new file mode 100644 index 00000000000..42099b8771f --- /dev/null +++ b/changelogs/unreleased/196688-replace-underscore-with-lodash-for-app-assets-javascripts-deploy_k.yml @@ -0,0 +1,5 @@ +--- +title: Replace underscore with lodash for ./app/assets/javascripts/deploy_keys +merge_request: 24965 +author: Jacopo Beschi @jacopo-beschi +type: changed diff --git a/changelogs/unreleased/199220-snippet-search.yml b/changelogs/unreleased/199220-snippet-search.yml new file mode 100644 index 00000000000..d9ba1c6a626 --- /dev/null +++ b/changelogs/unreleased/199220-snippet-search.yml @@ -0,0 +1,5 @@ +--- +title: Include snippet description as part of snippet title search (when Elasticsearch is not enabled) +merge_request: 25961 +author: +type: added diff --git a/changelogs/unreleased/202639.yml b/changelogs/unreleased/202639.yml deleted file mode 100644 index 6fa70055e96..00000000000 --- a/changelogs/unreleased/202639.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Replace line diff number css selector with actual HTML inside MRs -merge_request: -author: Oregand -type: other diff --git a/changelogs/unreleased/207808-geo-bug-filedownloaddispatchworker-may-sometimes-excessively-resyn.yml b/changelogs/unreleased/207808-geo-bug-filedownloaddispatchworker-may-sometimes-excessively-resyn.yml new file mode 100644 index 00000000000..37e73e7b3cc --- /dev/null +++ b/changelogs/unreleased/207808-geo-bug-filedownloaddispatchworker-may-sometimes-excessively-resyn.yml @@ -0,0 +1,5 @@ +--- +title: 'Use uncached SQL queries for Geo long-running workers' +merge_request: 26187 +author: +type: fixed diff --git a/changelogs/unreleased/georgekoltsov-bump-project-import-limit.yml b/changelogs/unreleased/georgekoltsov-bump-project-import-limit.yml new file mode 100644 index 00000000000..1b692060624 --- /dev/null +++ b/changelogs/unreleased/georgekoltsov-bump-project-import-limit.yml @@ -0,0 +1,5 @@ +--- +title: Update Project Import API rate limit +merge_request: 26903 +author: +type: other diff --git a/changelogs/unreleased/sh-use-process-cache-for-feature-flags.yml b/changelogs/unreleased/sh-use-process-cache-for-feature-flags.yml new file mode 100644 index 00000000000..a812d61dadf --- /dev/null +++ b/changelogs/unreleased/sh-use-process-cache-for-feature-flags.yml @@ -0,0 +1,5 @@ +--- +title: Use process-wide memory cache for feature flags +merge_request: 26935 +author: +type: performance diff --git a/doc/api/releases/index.md b/doc/api/releases/index.md index 88fdedfa1b8..ffd4efdca5d 100644 --- a/doc/api/releases/index.md +++ b/doc/api/releases/index.md @@ -70,7 +70,11 @@ Example response: "updated_at":"2019-07-12T19:45:44.256Z", "due_date":"2019-08-16T11:00:00.256Z", "start_date":"2019-07-30T12:00:00.256Z", - "web_url":"https://gitlab.example.com/root/awesome-app/-/milestones/1" + "web_url":"https://gitlab.example.com/root/awesome-app/-/milestones/1", + "issue_stats": { + "total": 98, + "closed": 76 + } }, { "id":52, @@ -83,7 +87,11 @@ Example response: "updated_at":"2019-07-16T14:00:12.256Z", "due_date":"2019-08-16T11:00:00.256Z", "start_date":"2019-07-30T12:00:00.256Z", - "web_url":"https://gitlab.example.com/root/awesome-app/-/milestones/2" + "web_url":"https://gitlab.example.com/root/awesome-app/-/milestones/2", + "issue_stats": { + "total": 24, + "closed": 21 + } } ], "commit_path":"/root/awesome-app/commit/588440f66559714280628a4f9799f0c4eb880a4a", @@ -252,7 +260,11 @@ Example response: "updated_at":"2019-07-12T19:45:44.256Z", "due_date":"2019-08-16T11:00:00.256Z", "start_date":"2019-07-30T12:00:00.256Z", - "web_url":"https://gitlab.example.com/root/awesome-app/-/milestones/1" + "web_url":"https://gitlab.example.com/root/awesome-app/-/milestones/1", + "issue_stats": { + "total": 98, + "closed": 76 + } }, { "id":52, @@ -265,7 +277,11 @@ Example response: "updated_at":"2019-07-16T14:00:12.256Z", "due_date":"2019-08-16T11:00:00.256Z", "start_date":"2019-07-30T12:00:00.256Z", - "web_url":"https://gitlab.example.com/root/awesome-app/-/milestones/2" + "web_url":"https://gitlab.example.com/root/awesome-app/-/milestones/2", + "issue_stats": { + "total": 24, + "closed": 21 + } } ], "commit_path":"/root/awesome-app/commit/588440f66559714280628a4f9799f0c4eb880a4a", @@ -374,7 +390,11 @@ Example response: "updated_at":"2019-07-12T19:45:44.256Z", "due_date":"2019-08-16T11:00:00.256Z", "start_date":"2019-07-30T12:00:00.256Z", - "web_url":"https://gitlab.example.com/root/awesome-app/-/milestones/1" + "web_url":"https://gitlab.example.com/root/awesome-app/-/milestones/1", + "issue_stats": { + "total": 99, + "closed": 76 + } }, { "id":52, @@ -387,7 +407,11 @@ Example response: "updated_at":"2019-07-16T14:00:12.256Z", "due_date":"2019-08-16T11:00:00.256Z", "start_date":"2019-07-30T12:00:00.256Z", - "web_url":"https://gitlab.example.com/root/awesome-app/-/milestones/2" + "web_url":"https://gitlab.example.com/root/awesome-app/-/milestones/2", + "issue_stats": { + "total": 24, + "closed": 21 + } } ], "commit_path":"/root/awesome-app/commit/588440f66559714280628a4f9799f0c4eb880a4a", @@ -495,7 +519,11 @@ Example response: "updated_at":"2019-09-01T13:00:00.256Z", "due_date":"2019-09-20T13:00:00.256Z", "start_date":"2019-09-05T12:00:00.256Z", - "web_url":"https://gitlab.example.com/root/awesome-app/-/milestones/3" + "web_url":"https://gitlab.example.com/root/awesome-app/-/milestones/3", + "issue_stats": { + "opened": 11, + "closed": 78 + } } ], "commit_path":"/root/awesome-app/commit/588440f66559714280628a4f9799f0c4eb880a4a", diff --git a/doc/development/README.md b/doc/development/README.md index d73b83e53fc..b207f208e3d 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -145,6 +145,7 @@ Complementary reads: - [Hash indexes](hash_indexes.md) - [Storing SHA1 hashes as binary](sha1_as_binary.md) - [Iterating tables in batches](iterating_tables_in_batches.md) +- [Insert into tables in batches](insert_into_tables_in_batches.md) - [Ordering table columns](ordering_table_columns.md) - [Verifying database capabilities](verifying_database_capabilities.md) - [Database Debugging and Troubleshooting](database_debugging.md) diff --git a/doc/development/insert_into_tables_in_batches.md b/doc/development/insert_into_tables_in_batches.md new file mode 100644 index 00000000000..763185013c9 --- /dev/null +++ b/doc/development/insert_into_tables_in_batches.md @@ -0,0 +1,156 @@ +--- +description: "Sometimes it is necessary to store large amounts of records at once, which can be inefficient +when iterating collections and performing individual `save`s. With the arrival of `insert_all` +in Rails 6, which operates at the row level (that is, using `Hash`es), GitLab has added a set +of APIs that make it safe and simple to insert ActiveRecord objects in bulk." +--- + +# Insert into tables in batches + +Sometimes it is necessary to store large amounts of records at once, which can be inefficient +when iterating collections and saving each record individually. With the arrival of +[`insert_all`](https://apidock.com/rails/ActiveRecord/Persistence/ClassMethods/insert_all) +in Rails 6, which operates at the row level (that is, using `Hash` objects), GitLab has added a set +of APIs that make it safe and simple to insert `ActiveRecord` objects in bulk. + +## Prepare `ApplicationRecord`s for bulk insertion + +In order for a model class to take advantage of the bulk insertion API, it has to include the +`BulkInsertSafe` concern first: + +```ruby +class MyModel < ApplicationRecord + # other includes here + # ... + include BulkInsertSafe # include this last + + # ... +end +``` + +The `BulkInsertSafe` concern has two functions: + +- It performs checks against your model class to ensure that it does not use ActiveRecord + APIs that are not safe to use with respect to bulk insertions (more on that below). +- It adds a new class method `bulk_insert!`, which you can use to insert many records at once. + +## Insert records via `bulk_insert!` + +If the target class passes the checks performed by `BulkInsertSafe`, you can proceed to use +the `bulk_insert!` class method as follows: + +```ruby +records = [MyModel.new, ...] + +MyModel.bulk_insert!(records) +``` + +### Record validation + +The `bulk_insert!` method guarantees that `records` will be inserted transactionally, and +will run validations on each record prior to insertion. If any record fails to validate, +an error is raised and the transaction is rolled back. You can turn off validations via +the `:validate` option: + +```ruby +MyModel.bulk_insert!(records, validate: false) +``` + +### Batch size configuration + +In those cases where the number of `records` is above a given threshold, insertions will +occur in multiple batches. The default batch size is defined in +[`BulkInsertSafe::DEFAULT_BATCH_SIZE`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/models/concerns/bulk_insert_safe.rb). +Assuming a default threshold of 500, inserting 950 records +would result in two batches being written sequentially (of size 500 and 450 respectively.) +You can override the default batch size via the `:batch_size` option: + +```ruby +MyModel.bulk_insert!(records, batch_size: 100) +``` + +Assuming the same number of 950 records, this would result in 10 batches being written instead. +Since this will also affect the number of `INSERT`s that occur, make sure you measure the +performance impact this might have on your code. There is a trade-off between the number of +`INSERT` statements the database has to process and the size and cost of each `INSERT`. + +### Requirements for safe bulk insertions + +Large parts of ActiveRecord's persistence API are built around the notion of callbacks. Many +of these callbacks fire in response to model life cycle events such as `save` or `create`. +These callbacks cannot be used with bulk insertions, since they are meant to be called for +every instance that is saved or created. Since these events do not fire when +records are inserted in bulk, we currently disallow their use. + +The specifics around which callbacks are disallowed are defined in +[`BulkInsertSafe`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/models/concerns/bulk_insert_safe.rb). +Consult the module source code for details. If your class uses any of the blacklisted +functionality, and you `include BulkInsertSafe`, the application will fail with an error. + +### `BulkInsertSafe` versus `InsertAll` + +Internally, `BulkInsertSafe` is based on `InsertAll`, and you may wonder when to choose +the former over the latter. To help you make the decision, +the key differences between these classes are listed in the table below. + +| | Input type | Validates input | Specify batch size | Can bypass callbacks | Transactional | +|--------------- | -------------------- | --------------- | ------------------ | --------------------------------- | ------------- | +| `bulk_insert!` | ActiveRecord objects | Yes (optional) | Yes (optional) | No (prevents unsafe callback use) | Yes | +| `insert_all!` | Attribute hashes | No | No | Yes | Yes | + +To summarize, `BulkInsertSafe` moves bulk inserts closer to how ActiveRecord objects +and inserts would normally behave. However, if all you need is to insert raw data in bulk, then +`insert_all` is more efficient. + +## Insert `has_many` associations in bulk + +A common use case is to save collections of associated relations through the owner side of the relation, +where the owned relation is associated to the owner through the `has_many` class method: + +```ruby +owner = OwnerModel.new(owned_relations: array_of_owned_relations) +# saves all `owned_relations` one-by-one +owner.save! +``` + +This will issue a single `INSERT`, and transaction, for every record in `owned_relations`, which is inefficient if +`array_of_owned_relations` is large. To remedy this, the `BulkInsertableAssociations` concern can be +used to declare that the owner defines associations that are safe for bulk insertion: + +```ruby +class OwnerModel < ApplicationRecord + # other includes here + # ... + include BulkInsertableAssociations # include this last + + has_many :my_models +end +``` + +Here `my_models` must be declared `BulkInsertSafe` (as described previously) for bulk insertions +to happen. You can now insert any yet unsaved records as follows: + +```ruby +BulkInsertableAssociations.with_bulk_insert do + owner = OwnerModel.new(my_models: array_of_my_model_instances) + # saves `my_models` using a single bulk insert (possibly via multiple batches) + owner.save! +end +``` + +Note that you can still save relations that are not `BulkInsertSafe` in this block; they will +simply be treated as if you had invoked `save` from outside the block. + +## Known limitations + +There are a few restrictions to how these APIs can be used: + +- Bulk inserts only work for new records; `UPDATE`s or "upserts" are not supported yet. +- `ON CONFLICT` behavior cannot currently be configured; an error will be raised on primary key conflicts. +- `BulkInsertableAssociations` furthermore has the following restrictions: + - only compatible with `has_many` relations. + - does not support `has_many through: ...` relations. + +Moreover, input data should either be limited to around 1000 records at most, +or already batched prior to calling bulk insert. The `INSERT` statement will run in a single +transaction, so for large amounts of records it may negatively affect database stability. diff --git a/doc/gitlab-basics/create-your-ssh-keys.md b/doc/gitlab-basics/create-your-ssh-keys.md index 98f2679c9d6..9b3431a5a42 100644 --- a/doc/gitlab-basics/create-your-ssh-keys.md +++ b/doc/gitlab-basics/create-your-ssh-keys.md @@ -1,14 +1,13 @@ --- type: howto --- - -# Create and add your SSH public key +# Create and add your SSH key pair It is best practice to use [Git over SSH instead of Git over HTTP](https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols). In order to use SSH, you will need to: -1. [Create an SSH key pair](#creating-your-ssh-key-pair) on your local computer. -1. [Add the key to GitLab](#adding-your-ssh-public-key-to-gitlab). +1. Create an SSH key pair +1. Add your SSH public key to GitLab ## Creating your SSH key pair diff --git a/doc/user/project/settings/import_export.md b/doc/user/project/settings/import_export.md index c69a4740ab3..9ff9f76dadb 100644 --- a/doc/user/project/settings/import_export.md +++ b/doc/user/project/settings/import_export.md @@ -29,10 +29,11 @@ Note the following: - Exports are stored in a temporary [shared directory](../../../development/shared_files.md) and are deleted every 24 hours by a specific worker. - Group members are exported as project members, as long as the user has - maintainer or admin access to the group where the exported project lives. Import admins should map users by email address. + maintainer or admin access to the group where the exported project lives. +- Project members with owner access will be imported as maintainers. +- Using an admin account to import will map users by email address (self-managed only). Otherwise, a supplementary comment is left to mention that the original author and the MRs, notes, or issues will be owned by the importer. -- Project members with owner access will be imported as maintainers. - If an imported project contains merge requests originating from forks, then new branches associated with such merge requests will be created within a project during the import/export. Thus, the number of branches @@ -142,4 +143,4 @@ To help avoid abuse, users are rate limited to: | ---------------- | --------------------------- | | Export | 1 project per 5 minutes | | Download export | 10 projects per 10 minutes | -| Import | 30 projects per 10 minutes | +| Import | 30 projects per 5 minutes | diff --git a/lib/api/entities/milestone_with_stats.rb b/lib/api/entities/milestone_with_stats.rb new file mode 100644 index 00000000000..33fa322573b --- /dev/null +++ b/lib/api/entities/milestone_with_stats.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module API + module Entities + class MilestoneWithStats < Entities::Milestone + expose :issue_stats do + expose :total_issues_count, as: :total + expose :closed_issues_count, as: :closed + end + end + end +end diff --git a/lib/api/entities/release.rb b/lib/api/entities/release.rb index dc4b91e594e..292b2e0102b 100644 --- a/lib/api/entities/release.rb +++ b/lib/api/entities/release.rb @@ -18,7 +18,7 @@ module API expose :author, using: Entities::UserBasic, if: -> (release, _) { release.author.present? } expose :commit, using: Entities::Commit, if: ->(_, _) { can_download_code? } expose :upcoming_release?, as: :upcoming_release - expose :milestones, using: Entities::Milestone, if: -> (release, _) { release.milestones.present? && can_read_milestone? } + expose :milestones, using: Entities::MilestoneWithStats, if: -> (release, _) { release.milestones.present? && can_read_milestone? } expose :commit_path, expose_nil: false expose :tag_path, expose_nil: false expose :evidence_sha, expose_nil: false, if: ->(_, _) { can_download_code? } diff --git a/lib/feature.rb b/lib/feature.rb index aadc2c64957..37cbe33f7c9 100644 --- a/lib/feature.rb +++ b/lib/feature.rb @@ -134,7 +134,11 @@ class Feature end def l1_cache_backend - Gitlab::ThreadMemoryCache.cache_backend + if Gitlab::Utils.to_boolean(ENV['USE_THREAD_MEMORY_CACHE']) + Gitlab::ThreadMemoryCache.cache_backend + else + Gitlab::ProcessMemoryCache.cache_backend + end end def l2_cache_backend diff --git a/lib/gitlab/application_rate_limiter.rb b/lib/gitlab/application_rate_limiter.rb index 8468c1c6601..211c59fe841 100644 --- a/lib/gitlab/application_rate_limiter.rb +++ b/lib/gitlab/application_rate_limiter.rb @@ -23,7 +23,7 @@ module Gitlab project_download_export: { threshold: 10, interval: 10.minutes }, project_repositories_archive: { threshold: 5, interval: 1.minute }, project_generate_new_export: { threshold: 1, interval: 5.minutes }, - project_import: { threshold: 30, interval: 10.minutes }, + project_import: { threshold: 30, interval: 5.minutes }, play_pipeline_schedule: { threshold: 1, interval: 1.minute }, show_raw_controller: { threshold: -> { Gitlab::CurrentSettings.current_application_settings.raw_blob_request_limit }, interval: 1.minute } }.freeze diff --git a/lib/gitlab/middleware/multipart.rb b/lib/gitlab/middleware/multipart.rb index 0ee9563c227..cb4213d23a4 100644 --- a/lib/gitlab/middleware/multipart.rb +++ b/lib/gitlab/middleware/multipart.rb @@ -84,12 +84,6 @@ module Gitlab end def open_file(params, key) - allowed_paths = [ - ::FileUploader.root, - Gitlab.config.uploads.storage_path, - File.join(Rails.root, 'public/uploads/tmp') - ] - ::UploadedFile.from_params(params, key, allowed_paths) end @@ -106,6 +100,16 @@ module Gitlab # inside other env keys, here we ensure everything is updated correctly ActionDispatch::Request.new(@request.env).update_param(key, value) end + + private + + def allowed_paths + [ + ::FileUploader.root, + Gitlab.config.uploads.storage_path, + File.join(Rails.root, 'public/uploads/tmp') + ] + end end def initialize(app) @@ -125,3 +129,5 @@ module Gitlab end end end + +::Gitlab::Middleware::Multipart::Handler.prepend_if_ee('EE::Gitlab::Middleware::Multipart::Handler') diff --git a/lib/gitlab/process_memory_cache.rb b/lib/gitlab/process_memory_cache.rb new file mode 100644 index 00000000000..5e8578711b2 --- /dev/null +++ b/lib/gitlab/process_memory_cache.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Gitlab + class ProcessMemoryCache + # ActiveSupport::Cache::MemoryStore is thread-safe: + # https://github.com/rails/rails/blob/2f1fefe456932a6d7d2b155d27b5315c33f3daa1/activesupport/lib/active_support/cache/memory_store.rb#L19 + @cache = ActiveSupport::Cache::MemoryStore.new + + def self.cache_backend + @cache + end + end +end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 2f597fd5cb3..a677e17ab0c 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -495,4 +495,45 @@ describe SessionsController do expect(session[:failed_login_attempts]).to eq(1) end end + + describe '#set_current_context' do + before do + set_devise_mapping(context: @request) + end + + context 'when signed in' do + let_it_be(:user) { create(:user) } + + before do + sign_in(user) + end + + it 'sets the username and caller_id in the context' do + expect(controller).to receive(:destroy).and_wrap_original do |m, *args| + expect(Labkit::Context.current.to_h) + .to include('meta.user' => user.username, + 'meta.caller_id' => 'SessionsController#destroy') + + m.call(*args) + end + + delete :destroy + end + end + + context 'when not signed in' do + it 'sets the caller_id in the context' do + expect(controller).to receive(:new).and_wrap_original do |m, *args| + expect(Labkit::Context.current.to_h) + .to include('meta.caller_id' => 'SessionsController#new') + expect(Labkit::Context.current.to_h) + .not_to include('meta.user') + + m.call(*args) + end + + get :new + end + end + end end diff --git a/spec/fixtures/api/schemas/public_api/v4/milestone_with_stats.json b/spec/fixtures/api/schemas/public_api/v4/milestone_with_stats.json new file mode 100644 index 00000000000..e2475545ee9 --- /dev/null +++ b/spec/fixtures/api/schemas/public_api/v4/milestone_with_stats.json @@ -0,0 +1,30 @@ +{ + "type": "object", + "properties" : { + "id": { "type": "integer" }, + "iid": { "type": "integer" }, + "project_id": { "type": ["integer", "null"] }, + "group_id": { "type": ["integer", "null"] }, + "title": { "type": "string" }, + "description": { "type": ["string", "null"] }, + "state": { "type": "string" }, + "created_at": { "type": "date" }, + "updated_at": { "type": "date" }, + "start_date": { "type": "date" }, + "due_date": { "type": "date" }, + "web_url": { "type": "string" }, + "issue_stats": { + "required": ["total", "closed"], + "properties": { + "total": { "type": "integer" }, + "closed": { "type": "integer" } + }, + "additionalProperties": false + } + }, + "required": [ + "id", "iid", "title", "description", "state", + "state", "created_at", "updated_at", "start_date", "due_date", "issue_stats" + ], + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/public_api/v4/release.json b/spec/fixtures/api/schemas/public_api/v4/release.json index 46703c69dc0..a239be09919 100644 --- a/spec/fixtures/api/schemas/public_api/v4/release.json +++ b/spec/fixtures/api/schemas/public_api/v4/release.json @@ -17,7 +17,7 @@ }, "milestones": { "type": "array", - "items": { "$ref": "milestone.json" } + "items": { "$ref": "milestone_with_stats.json" } }, "commit_path": { "type": "string" }, "tag_path": { "type": "string" }, diff --git a/spec/fixtures/api/schemas/public_api/v4/release/release_for_guest.json b/spec/fixtures/api/schemas/public_api/v4/release/release_for_guest.json index bce74892059..1a1e92ac778 100644 --- a/spec/fixtures/api/schemas/public_api/v4/release/release_for_guest.json +++ b/spec/fixtures/api/schemas/public_api/v4/release/release_for_guest.json @@ -10,7 +10,7 @@ "upcoming_release": { "type": "boolean" }, "milestones": { "type": "array", - "items": { "$ref": "../milestone.json" } + "items": { "$ref": "../milestone_with_stats.json" } }, "commit_path": { "type": "string" }, "tag_path": { "type": "string" }, diff --git a/spec/frontend/diffs/components/diff_table_cell_spec.js b/spec/frontend/diffs/components/diff_table_cell_spec.js index ad70b5695cc..1af0746f3bd 100644 --- a/spec/frontend/diffs/components/diff_table_cell_spec.js +++ b/spec/frontend/diffs/components/diff_table_cell_spec.js @@ -155,10 +155,6 @@ describe('DiffTableCell', () => { }); }); - it('renders the correct line number', () => { - expect(findLineNumber().text()).toEqual(TEST_LINE_NUMBER.toString()); - }); - it('on click, dispatches setHighlightedRow', () => { expect(store.dispatch).not.toHaveBeenCalled(); diff --git a/spec/lib/feature_spec.rb b/spec/lib/feature_spec.rb index 2525dd17b89..cbe617359ef 100644 --- a/spec/lib/feature_spec.rb +++ b/spec/lib/feature_spec.rb @@ -146,7 +146,15 @@ describe Feature do expect(described_class.enabled?(:enabled_feature_flag)).to be_truthy end - it { expect(described_class.l1_cache_backend).to eq(Gitlab::ThreadMemoryCache.cache_backend) } + context 'with USE_THREAD_MEMORY_CACHE defined' do + before do + stub_env('USE_THREAD_MEMORY_CACHE', '1') + end + + it { expect(described_class.l1_cache_backend).to eq(Gitlab::ThreadMemoryCache.cache_backend) } + end + + it { expect(described_class.l1_cache_backend).to eq(Gitlab::ProcessMemoryCache.cache_backend) } it { expect(described_class.l2_cache_backend).to eq(Rails.cache) } it 'caches the status in L1 and L2 caches', diff --git a/spec/lib/gitlab/middleware/multipart_spec.rb b/spec/lib/gitlab/middleware/multipart_spec.rb index 33797817578..1d3ad3afb56 100644 --- a/spec/lib/gitlab/middleware/multipart_spec.rb +++ b/spec/lib/gitlab/middleware/multipart_spec.rb @@ -5,9 +5,7 @@ require 'spec_helper' require 'tempfile' describe Gitlab::Middleware::Multipart do - let(:app) { double(:app) } - let(:middleware) { described_class.new(app) } - let(:original_filename) { 'filename' } + include_context 'multipart middleware context' shared_examples_for 'multipart upload files' do it 'opens top-level files' do @@ -82,22 +80,12 @@ describe Gitlab::Middleware::Multipart do end it 'allows files in uploads/tmp directory' do - Dir.mktmpdir do |dir| - uploads_dir = File.join(dir, 'public/uploads/tmp') - FileUtils.mkdir_p(uploads_dir) - - allow(Rails).to receive(:root).and_return(dir) - allow(Dir).to receive(:tmpdir).and_return(File.join(Dir.tmpdir, 'tmpsubdir')) - - Tempfile.open('top-level', uploads_dir) do |tempfile| - env = post_env({ 'file' => tempfile.path }, { 'file.name' => original_filename, 'file.path' => tempfile.path }, Gitlab::Workhorse.secret, 'gitlab-workhorse') - - expect(app).to receive(:call) do |env| - expect(get_params(env)['file']).to be_a(::UploadedFile) - end - - middleware.call(env) + with_tmp_dir('public/uploads/tmp') do |dir, env| + expect(app).to receive(:call) do |env| + expect(get_params(env)['file']).to be_a(::UploadedFile) end + + middleware.call(env) end end @@ -127,22 +115,4 @@ describe Gitlab::Middleware::Multipart do middleware.call(env) end end - - # Rails 5 doesn't combine the GET/POST parameters in - # ActionDispatch::HTTP::Parameters if action_dispatch.request.parameters is set: - # https://github.com/rails/rails/blob/aea6423f013ca48f7704c70deadf2cd6ac7d70a1/actionpack/lib/action_dispatch/http/parameters.rb#L41 - def get_params(env) - req = ActionDispatch::Request.new(env) - req.GET.merge(req.POST) - end - - def post_env(rewritten_fields, params, secret, issuer) - token = JWT.encode({ 'iss' => issuer, 'rewritten_fields' => rewritten_fields }, secret, 'HS256') - Rack::MockRequest.env_for( - '/', - method: 'post', - params: params, - described_class::RACK_ENV_KEY => token - ) - end end diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 95807f5f0c1..c9656b4d6ca 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -149,7 +149,7 @@ describe Snippet do end describe '.search' do - let(:snippet) { create(:snippet, title: 'test snippet') } + let(:snippet) { create(:snippet, title: 'test snippet', description: 'description') } it 'returns snippets with a matching title' do expect(described_class.search(snippet.title)).to eq([snippet]) @@ -174,6 +174,10 @@ describe Snippet do it 'returns snippets with a matching file name regardless of the casing' do expect(described_class.search(snippet.file_name.upcase)).to eq([snippet]) end + + it 'returns snippets with a matching description' do + expect(described_class.search(snippet.description)).to eq([snippet]) + end end describe '.search_code' do diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index f9e7253a88b..5de8d5aa3ff 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -359,12 +359,29 @@ describe API::Releases do let(:milestone) { create(:milestone, project: project) } + it 'matches schema' do + get api("/projects/#{project.id}/releases/v0.1", non_project_member) + + expect(response).to match_response_schema('public_api/v4/release') + end + it 'exposes milestones' do get api("/projects/#{project.id}/releases/v0.1", non_project_member) expect(json_response['milestones'].first['title']).to eq(milestone.title) end + it 'returns issue stats for milestone' do + create_list(:issue, 2, milestone: milestone, project: project) + create_list(:issue, 3, :closed, milestone: milestone, project: project) + + get api("/projects/#{project.id}/releases/v0.1", non_project_member) + + issue_stats = json_response['milestones'].first["issue_stats"] + expect(issue_stats["total"]).to eq(5) + expect(issue_stats["closed"]).to eq(3) + end + context 'when project restricts visibility of issues and merge requests' do let!(:project) { create(:project, :repository, :public, :issues_private, :merge_requests_private) } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b862fc4bbd9..d379fd494fa 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -193,8 +193,10 @@ RSpec.configure do |config| # expect(Gitlab::Git::KeepAround).to receive(:execute).and_call_original allow(Gitlab::Git::KeepAround).to receive(:execute) - # Clear thread cache and Sidekiq queues - Gitlab::ThreadMemoryCache.cache_backend.clear + [Gitlab::ThreadMemoryCache, Gitlab::ProcessMemoryCache].each do |cache| + cache.cache_backend.clear + end + Sidekiq::Worker.clear_all # Temporary patch to force admin mode to be active by default in tests when diff --git a/spec/support/shared_contexts/lib/gitlab/middleware/multipart_shared_contexts.rb b/spec/support/shared_contexts/lib/gitlab/middleware/multipart_shared_contexts.rb new file mode 100644 index 00000000000..f1554ea8e9f --- /dev/null +++ b/spec/support/shared_contexts/lib/gitlab/middleware/multipart_shared_contexts.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +RSpec.shared_context 'multipart middleware context' do + let(:app) { double(:app) } + let(:middleware) { described_class.new(app) } + let(:original_filename) { 'filename' } + + # Rails 5 doesn't combine the GET/POST parameters in + # ActionDispatch::HTTP::Parameters if action_dispatch.request.parameters is set: + # https://github.com/rails/rails/blob/aea6423f013ca48f7704c70deadf2cd6ac7d70a1/actionpack/lib/action_dispatch/http/parameters.rb#L41 + def get_params(env) + req = ActionDispatch::Request.new(env) + req.GET.merge(req.POST) + end + + def post_env(rewritten_fields, params, secret, issuer) + token = JWT.encode({ 'iss' => issuer, 'rewritten_fields' => rewritten_fields }, secret, 'HS256') + Rack::MockRequest.env_for( + '/', + method: 'post', + params: params, + described_class::RACK_ENV_KEY => token + ) + end + + def with_tmp_dir(uploads_sub_dir, storage_path = '') + Dir.mktmpdir do |dir| + upload_dir = File.join(dir, storage_path, uploads_sub_dir) + FileUtils.mkdir_p(upload_dir) + + allow(Rails).to receive(:root).and_return(dir) + allow(Dir).to receive(:tmpdir).and_return(File.join(Dir.tmpdir, 'tmpsubdir')) + allow(GitlabUploader).to receive(:root).and_return(File.join(dir, storage_path)) + + Tempfile.open('top-level', upload_dir) do |tempfile| + env = post_env({ 'file' => tempfile.path }, { 'file.name' => original_filename, 'file.path' => tempfile.path }, Gitlab::Workhorse.secret, 'gitlab-workhorse') + + yield dir, env + end + end + end +end |