diff options
-rw-r--r-- | changelogs/unreleased/refactoring-entities-file-12.yml | 5 | ||||
-rw-r--r-- | config/unicorn.rb.example | 5 | ||||
-rw-r--r-- | config/unicorn.rb.example.development | 5 | ||||
-rw-r--r-- | doc/development/iterating_tables_in_batches.md | 2 | ||||
-rw-r--r-- | doc/development/licensing.md | 8 | ||||
-rw-r--r-- | doc/development/logging.md | 2 | ||||
-rw-r--r-- | doc/development/packages.md | 2 | ||||
-rw-r--r-- | doc/development/query_recorder.md | 6 | ||||
-rw-r--r-- | doc/development/rake_tasks.md | 22 | ||||
-rw-r--r-- | lib/api/entities.rb | 65 | ||||
-rw-r--r-- | lib/api/entities/basic_project_details.rb | 2 | ||||
-rw-r--r-- | lib/api/entities/deploy_key_with_user.rb | 9 | ||||
-rw-r--r-- | lib/api/entities/deploy_keys_project.rb | 10 | ||||
-rw-r--r-- | lib/api/entities/diff_position.rb | 10 | ||||
-rw-r--r-- | lib/api/entities/discussion.rb | 11 | ||||
-rw-r--r-- | lib/api/entities/gpg_key.rb | 9 | ||||
-rw-r--r-- | lib/api/entities/note.rb | 30 | ||||
-rw-r--r-- | lib/api/entities/ssh_key.rb | 9 | ||||
-rw-r--r-- | lib/api/entities/ssh_key_with_user.rb | 9 | ||||
-rw-r--r-- | lib/gitlab/log_timestamp_formatter.rb | 11 | ||||
-rw-r--r-- | spec/lib/gitlab/log_timestamp_formatter_spec.rb | 15 |
21 files changed, 164 insertions, 83 deletions
diff --git a/changelogs/unreleased/refactoring-entities-file-12.yml b/changelogs/unreleased/refactoring-entities-file-12.yml new file mode 100644 index 00000000000..60a9f550a20 --- /dev/null +++ b/changelogs/unreleased/refactoring-entities-file-12.yml @@ -0,0 +1,5 @@ +--- +title: Separate key and other entities into own class files +merge_request: 24495 +author: Rajendra Kadam +type: added diff --git a/config/unicorn.rb.example b/config/unicorn.rb.example index 9f13fac5cca..77e440eddde 100644 --- a/config/unicorn.rb.example +++ b/config/unicorn.rb.example @@ -82,6 +82,7 @@ preload_app true check_client_connection false require_relative "/home/git/gitlab/lib/gitlab/cluster/lifecycle_events" +require_relative "/home/git/gitlab/lib/gitlab/log_timestamp_formatter.rb" before_exec do |server| # Signal application hooks that we're about to restart @@ -137,3 +138,7 @@ after_fork do |server, worker| # addr = "127.0.0.1:#{9293 + worker.nr}" # server.listen(addr, :tries => -1, :delay => 5, :tcp_nopush => true) end + +# Configure the default logger to use a custom formatter that formats the +# timestamps to be in UTC and in ISO8601.3 format +Configurator::DEFAULTS[:logger].formatter = Gitlab::LogTimestampFormatter.new diff --git a/config/unicorn.rb.example.development b/config/unicorn.rb.example.development index 92bb1c7344a..2c6e809f753 100644 --- a/config/unicorn.rb.example.development +++ b/config/unicorn.rb.example.development @@ -15,6 +15,7 @@ preload_app true check_client_connection false require_relative "/home/git/gitlab/lib/gitlab/cluster/lifecycle_events" +require_relative "/home/git/gitlab/lib/gitlab/log_timestamp_formatter.rb" before_exec do |server| # Signal application hooks that we're about to restart @@ -70,3 +71,7 @@ after_fork do |server, worker| # addr = "127.0.0.1:#{9293 + worker.nr}" # server.listen(addr, :tries => -1, :delay => 5, :tcp_nopush => true) end + +# Configure the default logger to use a custom formatter that formats the +# timestamps to be in UTC and in ISO8601.3 format +Configurator::DEFAULTS[:logger].formatter = Gitlab::LogTimestampFormatter.new diff --git a/doc/development/iterating_tables_in_batches.md b/doc/development/iterating_tables_in_batches.md index 590c8cbba2d..56cbb3a0e9e 100644 --- a/doc/development/iterating_tables_in_batches.md +++ b/doc/development/iterating_tables_in_batches.md @@ -27,7 +27,7 @@ end This will end up producing queries such as: -``` +```plaintext User Load (0.7ms) SELECT "users"."id" FROM "users" WHERE ("users"."id" >= 41654) ORDER BY "users"."id" ASC LIMIT 1 OFFSET 1000 (0.7ms) SELECT COUNT(*) FROM "users" WHERE ("users"."id" >= 41654) AND ("users"."id" < 42687) ``` diff --git a/doc/development/licensing.md b/doc/development/licensing.md index 052c90c2b37..2dc77b2eec8 100644 --- a/doc/development/licensing.md +++ b/doc/development/licensing.md @@ -16,25 +16,25 @@ There are a few basic commands License Finder provides that you'll need in order To verify that the checks are passing, and/or to see what dependencies are causing the checks to fail: -``` +```shell bundle exec license_finder ``` To whitelist a new license: -``` +```shell license_finder whitelist add MIT ``` To blacklist a new license: -``` +```shell license_finder blacklist add GPLv2 ``` To tell License Finder about a dependency's license if it isn't auto-detected: -``` +```shell license_finder licenses add my_unknown_dependency MIT ``` diff --git a/doc/development/logging.md b/doc/development/logging.md index 202c7a5ce9f..e4de7169815 100644 --- a/doc/development/logging.md +++ b/doc/development/logging.md @@ -9,7 +9,7 @@ Currently `Rails.logger` calls all get saved into `production.log`, which contai a mix of Rails' logs and other calls developers have inserted in the code base. For example: -``` +```plaintext Started GET "/gitlabhq/yaml_db/tree/master" for 168.111.56.1 at 2015-02-12 19:34:53 +0200 Processing by Projects::TreeController#show as HTML Parameters: {"project_id"=>"gitlabhq/yaml_db", "id"=>"master"} diff --git a/doc/development/packages.md b/doc/development/packages.md index d08891c6e7f..ac3eba5c42a 100644 --- a/doc/development/packages.md +++ b/doc/development/packages.md @@ -38,7 +38,7 @@ endpoints like: Since the packages belong to a project, it's expected to have project-level endpoint (remote) for uploading and downloading them. For example: -``` +```plaintext GET https://gitlab.com/api/v4/projects/<your_project_id>/packages/npm/ PUT https://gitlab.com/api/v4/projects/<your_project_id>/packages/npm/ ``` diff --git a/doc/development/query_recorder.md b/doc/development/query_recorder.md index 81970b45bbd..75a8a33e02a 100644 --- a/doc/development/query_recorder.md +++ b/doc/development/query_recorder.md @@ -28,7 +28,7 @@ By default, QueryRecorder will ignore cached queries in the count. However, it m all queries to avoid introducing an N+1 query that may be masked by the statement cache. To do this, pass the `skip_cached` variable to `QueryRecorder` and use the `exceed_all_query_limit` matcher: -``` +```ruby it "avoids N+1 database queries" do control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { visit_some_page }.count create_list(:issue, 5) @@ -48,13 +48,13 @@ This could lead to false successes where subsequent "requests" could have querie It may be useful to identify the source of the queries by looking at the call backtrace. To enable this, run the specs with the `QUERY_RECORDER_DEBUG` environment variable set. For example: -``` +```shell QUERY_RECORDER_DEBUG=1 bundle exec rspec spec/requests/api/projects_spec.rb ``` This will log calls to QueryRecorder into the `test.log`. For example: -``` +```plaintext QueryRecorder SQL: SELECT COUNT(*) FROM "issues" WHERE "issues"."deleted_at" IS NULL AND "issues"."project_id" = $1 AND ("issues"."state" IN ('opened')) AND "issues"."confidential" = $2 --> /home/user/gitlab/gdk/gitlab/spec/support/query_recorder.rb:19:in `callback' --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications/fanout.rb:127:in `finish' diff --git a/doc/development/rake_tasks.md b/doc/development/rake_tasks.md index d9b2ace1b5b..83dd5027a02 100644 --- a/doc/development/rake_tasks.md +++ b/doc/development/rake_tasks.md @@ -4,7 +4,7 @@ Note that if your db user does not have advanced privileges you must create the db manually before running this command. -``` +```shell bundle exec rake setup ``` @@ -70,7 +70,7 @@ Group are additionally seeded with epics if GitLab instance has epics feature av If you're very sure that you want to **wipe the current database** and refill seeds, you could: -``` shell +```shell echo 'yes' | bundle exec rake setup ``` @@ -83,7 +83,7 @@ your terminal, and it would generate more than 20G logs if you just redirect it to a file. If we don't care about the output, we could just redirect it to `/dev/null`: -``` shell +```shell echo 'yes' | bundle exec rake setup > /dev/null ``` @@ -138,13 +138,13 @@ you don't need to boot it every time you run a test, rake task or migration. If you want to use it, you'll need to export the `ENABLE_SPRING` environment variable to `1`: -``` +```shell export ENABLE_SPRING=1 ``` Alternatively you can use the following on each spec run, -``` +```shell bundle exec spring rspec some_spec.rb ``` @@ -154,7 +154,7 @@ You shouldn't ever need to compile frontend assets manually in development, but if you ever need to test how the assets get compiled in a production environment you can do so with the following command: -``` +```shell RAILS_ENV=production NODE_ENV=production bundle exec rake gitlab:assets:compile ``` @@ -167,7 +167,7 @@ they can be easily inspected. To update the Emoji aliases file (used for Emoji autocomplete) you must run the following: -``` +```shell bundle exec rake gemojione:aliases ``` @@ -176,7 +176,7 @@ bundle exec rake gemojione:aliases To update the Emoji digests file (used for Emoji autocomplete) you must run the following: -``` +```shell bundle exec rake gemojione:digests ``` @@ -187,7 +187,7 @@ available Emoji. Generating a sprite file containing all the Emoji can be done by running: -``` +```shell bundle exec rake gemojione:sprite ``` @@ -201,7 +201,7 @@ task, then check the dimensions of the new spritesheet and update the Starting a project from a template needs this project to be exported. On a up to date master branch run: -``` +```shell gdk start bundle exec rake gitlab:update_project_templates git checkout -b update-project-templates @@ -233,7 +233,7 @@ a file for quick reference. To see a list of all obsolete `ignored_columns` run: -``` +```shell bundle exec rake db:obsolete_ignored_columns ``` diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 98ae4af2a19..122b2c40623 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -129,63 +129,6 @@ module API end end - class SSHKey < Grape::Entity - expose :id, :title, :key, :created_at - end - - class SSHKeyWithUser < SSHKey - expose :user, using: Entities::UserPublic - end - - class DeployKeyWithUser < SSHKeyWithUser - expose :deploy_keys_projects - end - - class DeployKeysProject < Grape::Entity - expose :deploy_key, merge: true, using: Entities::SSHKey - expose :can_push - end - - class GPGKey < Grape::Entity - expose :id, :key, :created_at - end - - class DiffPosition < Grape::Entity - expose :base_sha, :start_sha, :head_sha, :old_path, :new_path, - :position_type - end - - class Note < Grape::Entity - # Only Issue and MergeRequest have iid - NOTEABLE_TYPES_WITH_IID = %w(Issue MergeRequest).freeze - - expose :id - expose :type - expose :note, as: :body - expose :attachment_identifier, as: :attachment - expose :author, using: Entities::UserBasic - expose :created_at, :updated_at - expose :system?, as: :system - expose :noteable_id, :noteable_type - - expose :position, if: ->(note, options) { note.is_a?(DiffNote) } do |note| - note.position.to_h - end - - expose :resolvable?, as: :resolvable - expose :resolved?, as: :resolved, if: ->(note, options) { note.resolvable? } - expose :resolved_by, using: Entities::UserBasic, if: ->(note, options) { note.resolvable? } - - # Avoid N+1 queries as much as possible - expose(:noteable_iid) { |note| note.noteable.iid if NOTEABLE_TYPES_WITH_IID.include?(note.noteable_type) } - end - - class Discussion < Grape::Entity - expose :id - expose :individual_note?, as: :individual_note - expose :notes, using: Entities::Note - end - class Avatar < Grape::Entity expose :avatar_url do |avatarable, options| avatarable.avatar_url(only_path: false, size: options[:size]) @@ -733,9 +676,9 @@ module API expose :id, :status, :stage, :name, :ref, :tag, :coverage, :allow_failure expose :created_at, :started_at, :finished_at expose :duration - expose :user, with: User - expose :commit, with: Commit - expose :pipeline, with: PipelineBasic + expose :user, with: Entities::User + expose :commit, with: Entities::Commit + expose :pipeline, with: Entities::PipelineBasic expose :web_url do |job, _options| Gitlab::Routing.url_helpers.project_job_url(job.project, job) @@ -751,7 +694,7 @@ module API end class JobBasicWithProject < JobBasic - expose :project, with: ProjectIdentity + expose :project, with: Entities::ProjectIdentity end class Trigger < Grape::Entity diff --git a/lib/api/entities/basic_project_details.rb b/lib/api/entities/basic_project_details.rb index 9ea3aeb9903..13bc19456b3 100644 --- a/lib/api/entities/basic_project_details.rb +++ b/lib/api/entities/basic_project_details.rb @@ -2,7 +2,7 @@ module API module Entities - class BasicProjectDetails < ProjectIdentity + class BasicProjectDetails < Entities::ProjectIdentity include ::API::ProjectsRelationBuilder expose :default_branch, if: -> (project, options) { Ability.allowed?(options[:current_user], :download_code, project) } diff --git a/lib/api/entities/deploy_key_with_user.rb b/lib/api/entities/deploy_key_with_user.rb new file mode 100644 index 00000000000..31024dc3910 --- /dev/null +++ b/lib/api/entities/deploy_key_with_user.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module API + module Entities + class DeployKeyWithUser < Entities::SSHKeyWithUser + expose :deploy_keys_projects + end + end +end diff --git a/lib/api/entities/deploy_keys_project.rb b/lib/api/entities/deploy_keys_project.rb new file mode 100644 index 00000000000..64725459167 --- /dev/null +++ b/lib/api/entities/deploy_keys_project.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module API + module Entities + class DeployKeysProject < Grape::Entity + expose :deploy_key, merge: true, using: Entities::SSHKey + expose :can_push + end + end +end diff --git a/lib/api/entities/diff_position.rb b/lib/api/entities/diff_position.rb new file mode 100644 index 00000000000..10150d04ac8 --- /dev/null +++ b/lib/api/entities/diff_position.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module API + module Entities + class DiffPosition < Grape::Entity + expose :base_sha, :start_sha, :head_sha, :old_path, :new_path, + :position_type + end + end +end diff --git a/lib/api/entities/discussion.rb b/lib/api/entities/discussion.rb new file mode 100644 index 00000000000..dd1dd40da23 --- /dev/null +++ b/lib/api/entities/discussion.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module API + module Entities + class Discussion < Grape::Entity + expose :id + expose :individual_note?, as: :individual_note + expose :notes, using: Entities::Note + end + end +end diff --git a/lib/api/entities/gpg_key.rb b/lib/api/entities/gpg_key.rb new file mode 100644 index 00000000000..a97e704a5dd --- /dev/null +++ b/lib/api/entities/gpg_key.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module API + module Entities + class GPGKey < Grape::Entity + expose :id, :key, :created_at + end + end +end diff --git a/lib/api/entities/note.rb b/lib/api/entities/note.rb new file mode 100644 index 00000000000..dcfb9a6d670 --- /dev/null +++ b/lib/api/entities/note.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module API + module Entities + class Note < Grape::Entity + # Only Issue and MergeRequest have iid + NOTEABLE_TYPES_WITH_IID = %w(Issue MergeRequest).freeze + + expose :id + expose :type + expose :note, as: :body + expose :attachment_identifier, as: :attachment + expose :author, using: Entities::UserBasic + expose :created_at, :updated_at + expose :system?, as: :system + expose :noteable_id, :noteable_type + + expose :position, if: ->(note, options) { note.is_a?(DiffNote) } do |note| + note.position.to_h + end + + expose :resolvable?, as: :resolvable + expose :resolved?, as: :resolved, if: ->(note, options) { note.resolvable? } + expose :resolved_by, using: Entities::UserBasic, if: ->(note, options) { note.resolvable? } + + # Avoid N+1 queries as much as possible + expose(:noteable_iid) { |note| note.noteable.iid if NOTEABLE_TYPES_WITH_IID.include?(note.noteable_type) } + end + end +end diff --git a/lib/api/entities/ssh_key.rb b/lib/api/entities/ssh_key.rb new file mode 100644 index 00000000000..0e2f6ebae8c --- /dev/null +++ b/lib/api/entities/ssh_key.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module API + module Entities + class SSHKey < Grape::Entity + expose :id, :title, :key, :created_at + end + end +end diff --git a/lib/api/entities/ssh_key_with_user.rb b/lib/api/entities/ssh_key_with_user.rb new file mode 100644 index 00000000000..95559bbf2ac --- /dev/null +++ b/lib/api/entities/ssh_key_with_user.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module API + module Entities + class SSHKeyWithUser < Entities::SSHKey + expose :user, using: Entities::UserPublic + end + end +end diff --git a/lib/gitlab/log_timestamp_formatter.rb b/lib/gitlab/log_timestamp_formatter.rb new file mode 100644 index 00000000000..433dedeb7a0 --- /dev/null +++ b/lib/gitlab/log_timestamp_formatter.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Gitlab + class LogTimestampFormatter < Logger::Formatter + FORMAT = "%s, [%s #%d] %5s -- %s: %s\n" + + def call(severity, timestamp, program_name, message) + FORMAT % [severity[0..0], timestamp.utc.iso8601(3), $$, severity, program_name, msg2str(message)] + end + end +end diff --git a/spec/lib/gitlab/log_timestamp_formatter_spec.rb b/spec/lib/gitlab/log_timestamp_formatter_spec.rb new file mode 100644 index 00000000000..1a76d02889b --- /dev/null +++ b/spec/lib/gitlab/log_timestamp_formatter_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::LogTimestampFormatter do + subject { described_class.new } + + let(:formatted_timestamp) { Time.now.utc.iso8601(3) } + + it 'logs the timestamp in UTC and ISO8601.3 format' do + Timecop.freeze(Time.now) do + expect(subject.call('', Time.now, '', '')).to include formatted_timestamp + end + end +end |