Add latest changes from gitlab-org/gitlab@master

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

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

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

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)
 ```

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
 ```
 

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"}

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/
 ```

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'

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
 ```
 

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

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) }

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

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

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

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

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

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

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

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

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

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