diff --git a/app/workers/concerns/gitlab/github_import/object_importer.rb b/app/workers/concerns/gitlab/github_import/object_importer.rb index e1f404b250d5001d2a2fea2eeea3626adce02adb..f2ce98872efe0b6ec5ff71a710d00eb097ec79e3 100644 --- a/app/workers/concerns/gitlab/github_import/object_importer.rb +++ b/app/workers/concerns/gitlab/github_import/object_importer.rb @@ -23,29 +23,18 @@ module ObjectImporter # client - An instance of `Gitlab::GithubImport::Client` # hash - A Hash containing the details of the object to import. def import(project, client, hash) - object = representation_class.from_json_hash(hash) + context = Context.new(project: project, client: client) + + self.representation = representation_class.new(context) + representation.deserialize(hash) - # To better express in the logs what object is being imported. - self.github_identifiers = object.github_identifiers info(project.id, message: 'starting importer') - importer_class.new(object, project, client).execute + importer_class.new(representation, project, client).execute Gitlab::GithubImport::ObjectCounter.increment(project, object_type, :imported) info(project.id, message: 'importer finished') - rescue NoMethodError => e - # This exception will be more useful in development when a new - # Representation is created but the developer forgot to add a - # `:github_identifiers` field. - Gitlab::Import::ImportFailureService.track( - project_id: project.id, - error_source: importer_class.name, - exception: e, - fail_import: true - ) - - raise(e) rescue StandardError => e Gitlab::Import::ImportFailureService.track( project_id: project.id, @@ -59,7 +48,7 @@ def object_type end # Returns the representation class to use for the object. This class must - # define the class method `from_json_hash`. + # define the class method `deserialize`. def representation_class raise NotImplementedError end @@ -71,7 +60,7 @@ def importer_class private - attr_accessor :github_identifiers + attr_accessor :representation def info(project_id, extra = {}) Logger.info(log_attributes(project_id, extra)) @@ -81,7 +70,7 @@ def log_attributes(project_id, extra = {}) extra.merge( project_id: project_id, importer: importer_class.name, - github_identifiers: github_identifiers + github_identifiers: representation.github_identifiers ) end end diff --git a/lib/gitlab/github_import/context.rb b/lib/gitlab/github_import/context.rb new file mode 100644 index 0000000000000000000000000000000000000000..b6958bf0d2d2b195de0500c53b0a34090b44a32a --- /dev/null +++ b/lib/gitlab/github_import/context.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Gitlab + module GithubImport + class Context + def initialize(project:, client:) + @attributes = ActiveSupport::HashWithIndifferentAccess.new + @attributes[:project] = project + @attributes[:client] = client + end + + def [](key) + @attributes[key] + end + + def []=(key, value) + @attributes[key] = value + end + end + end +end diff --git a/lib/gitlab/github_import/parallel_scheduling.rb b/lib/gitlab/github_import/parallel_scheduling.rb index a8e006ea082974f51d4625edf805445e8f34d864..014cdf048cca62c20a4e79c23047c13de9d3959a 100644 --- a/lib/gitlab/github_import/parallel_scheduling.rb +++ b/lib/gitlab/github_import/parallel_scheduling.rb @@ -63,9 +63,9 @@ def execute # Imports all the objects in sequence in the current thread. def sequential_import each_object_to_import do |object| - repr = representation_class.from_api_response(object) + representation.parse(object) - importer_class.new(repr, project, client).execute + importer_class.new(representation, project, client).execute end end @@ -75,10 +75,10 @@ def parallel_import waiter = JobWaiter.new each_object_to_import do |object| - repr = representation_class.from_api_response(object) + representation.parse(object) sidekiq_worker_class - .perform_async(project.id, repr.to_hash, waiter.key) + .perform_async(project.id, representation.to_hash, waiter.key) waiter.jobs_remaining += 1 end @@ -155,6 +155,12 @@ def representation_class raise NotImplementedError end + def representation + @representation ||= representation_class.new( + Context.new(project: project, client: client) + ) + end + # The class to use for importing objects when importing them sequentially. def importer_class raise NotImplementedError diff --git a/lib/gitlab/github_import/representation.rb b/lib/gitlab/github_import/representation.rb deleted file mode 100644 index 639477ef2a2b60f479e097e2cf3ea3b512b92201..0000000000000000000000000000000000000000 --- a/lib/gitlab/github_import/representation.rb +++ /dev/null @@ -1,25 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module GithubImport - module Representation - TIMESTAMP_KEYS = %i[created_at updated_at merged_at].freeze - - # Converts a Hash with String based keys to one that can be used by the - # various Representation classes. - # - # Example: - # - # Representation.symbolize_hash('number' => 10) # => { number: 10 } - def self.symbolize_hash(raw_hash = nil) - hash = raw_hash.deep_symbolize_keys - - TIMESTAMP_KEYS.each do |key| - hash[key] = Time.parse(hash[key]) if hash[key].is_a?(String) - end - - hash - end - end - end -end diff --git a/lib/gitlab/github_import/representation/base.rb b/lib/gitlab/github_import/representation/base.rb new file mode 100644 index 0000000000000000000000000000000000000000..4f479dabbd310f55d0f7949be18f7665f9273e0f --- /dev/null +++ b/lib/gitlab/github_import/representation/base.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +# Representation Objects have two main functions: +# +# - serialize/deserialize Github responses to/from import jobs; +# - transform Github data to expect Gitlab format; +module Gitlab + module GithubImport + module Representation + class Base + include ToHash + include ExposeAttribute + + TIMESTAMP_KEYS = %i[created_at updated_at merged_at submitted_at].freeze + + # attributes - A Hash containing the user details. The keys of this + # Hash (and any nested hashes) must be symbols. + def initialize(context) + @context = context + end + + # Parse the original github response + # + # response - Sawyer::Resource + def parse(response) + assign_attributes(response.to_hash) + end + + # Load a hash of attributes, created from Representation#to_hash + # to the current Representation object + def deserialize(hash) + assign_attributes(hash) + end + + # Set of relevant github identifier used to log the current object + def github_identifiers + raise NotImplementedError + end + + private + + attr_reader :context, :attributes + + def assign_attributes(attributes) + @attributes = symbolize_hash(attributes) + end + + def parse_with(klass, object) + klass.new(context).tap { |representer| representer.parse(object) } + end + + def deserialize_with(klass, hash) + klass.new(context).tap { |representer| representer.deserialize(hash) } + end + + # Converts a Hash with String based keys to one that can be used by the + # various Representation classes. + # + # Example: + # + # Representation.symbolize_hash('number' => 10) # => { number: 10 } + def symbolize_hash(raw_hash = nil) + hash = raw_hash.deep_symbolize_keys + + TIMESTAMP_KEYS.each do |key| + hash[key] = Time.parse(hash[key]).in_time_zone if convert_timestamp?(hash[key]) + end + + hash + end + + def convert_timestamp?(value) + value.present? && value.is_a?(String) + end + end + end + end +end diff --git a/lib/gitlab/github_import/representation/diff_note.rb b/lib/gitlab/github_import/representation/diff_note.rb index a3dcd2e380c6ceadd02b599f3cee9d1444c3e1b4..ed9f3c455378f0e5de21abab38e417edcffdfeaf 100644 --- a/lib/gitlab/github_import/representation/diff_note.rb +++ b/lib/gitlab/github_import/representation/diff_note.rb @@ -3,22 +3,14 @@ module Gitlab module GithubImport module Representation - class DiffNote - include ToHash - include ExposeAttribute - - attr_reader :attributes - + class DiffNote < Base expose_attribute :noteable_type, :noteable_id, :commit_id, :file_path, :diff_hunk, :author, :note, :created_at, :updated_at, :original_commit_id, :note_id NOTEABLE_ID_REGEX = %r{/pull/(?\d+)}i.freeze - # Builds a diff note from a GitHub API response. - # - # note - An instance of `Sawyer::Resource` containing the note details. - def self.from_api_response(note) + def parse(note) matches = note.html_url.match(NOTEABLE_ID_REGEX) unless matches @@ -28,7 +20,7 @@ def self.from_api_response(note) ) end - user = Representation::User.from_api_response(note.user) if note.user + user = parse_with(Representation::User, note.user) if note.user hash = { noteable_type: 'MergeRequest', noteable_id: matches[:iid].to_i, @@ -45,21 +37,13 @@ def self.from_api_response(note) start_line: note.start_line } - new(hash) + super(hash) end - # Builds a new note using a Hash that was built from a JSON payload. - def self.from_json_hash(raw_hash) - hash = Representation.symbolize_hash(raw_hash) - hash[:author] &&= Representation::User.from_json_hash(hash[:author]) - - new(hash) - end + def deserialize(hash) + super(hash) - # attributes - A Hash containing the raw note details. The keys of this - # Hash must be Symbols. - def initialize(attributes) - @attributes = attributes + attributes[:author] &&= deserialize_with(Representation::User, attributes[:author]) end def line_code diff --git a/lib/gitlab/github_import/representation/issue.rb b/lib/gitlab/github_import/representation/issue.rb index db4a8188c0333d38c74804832974c264bacc5d2e..e5b351007a6ceb165cf2920df627827d07123676 100644 --- a/lib/gitlab/github_import/representation/issue.rb +++ b/lib/gitlab/github_import/representation/issue.rb @@ -3,25 +3,13 @@ module Gitlab module GithubImport module Representation - class Issue - include ToHash - include ExposeAttribute - - attr_reader :attributes - + class Issue < Base expose_attribute :iid, :title, :description, :milestone_number, :created_at, :updated_at, :state, :assignees, :label_names, :author - # Builds an issue from a GitHub API response. - # - # issue - An instance of `Sawyer::Resource` containing the issue - # details. - def self.from_api_response(issue) - user = - if issue.user - Representation::User.from_api_response(issue.user) - end + def parse(issue) + user = parse_with(Representation::User, issue.user) if issue.user hash = { iid: issue.number, @@ -30,7 +18,7 @@ def self.from_api_response(issue) milestone_number: issue.milestone&.number, state: issue.state == 'open' ? :opened : :closed, assignees: issue.assignees.map do |u| - Representation::User.from_api_response(u) + parse_with(Representation::User, u) end, label_names: issue.labels.map(&:name), author: user, @@ -39,24 +27,15 @@ def self.from_api_response(issue) pull_request: issue.pull_request ? true : false } - new(hash) + super(hash) end - # Builds a new issue using a Hash that was built from a JSON payload. - def self.from_json_hash(raw_hash) - hash = Representation.symbolize_hash(raw_hash) - - hash[:state] = hash[:state].to_sym - hash[:assignees].map! { |u| Representation::User.from_json_hash(u) } - hash[:author] &&= Representation::User.from_json_hash(hash[:author]) - - new(hash) - end + def deserialize(hash) + super(hash) - # attributes - A hash containing the raw issue details. The keys of this - # Hash (and any nested hashes) must be symbols. - def initialize(attributes) - @attributes = attributes + attributes[:state] = attributes[:state].to_sym + attributes[:assignees].map! { |u| deserialize_with(Representation::User, u) } + attributes[:author] &&= deserialize_with(Representation::User, attributes[:author]) end def truncated_title diff --git a/lib/gitlab/github_import/representation/lfs_object.rb b/lib/gitlab/github_import/representation/lfs_object.rb index 18737bfcde3852b806cdc325d4189ee5bda0a8db..42b61903ef4ea9e7eaea2fcb7043733af4455ed0 100644 --- a/lib/gitlab/github_import/representation/lfs_object.rb +++ b/lib/gitlab/github_import/representation/lfs_object.rb @@ -3,34 +3,17 @@ module Gitlab module GithubImport module Representation - class LfsObject - include ToHash - include ExposeAttribute - - attr_reader :attributes - + class LfsObject < Base expose_attribute :oid, :link, :size - # Builds a lfs_object - def self.from_api_response(lfs_object) - new( + def parse(lfs_object) + super( oid: lfs_object.oid, link: lfs_object.link, size: lfs_object.size ) end - # Builds a new lfs_object using a Hash that was built from a JSON payload. - def self.from_json_hash(raw_hash) - new(Representation.symbolize_hash(raw_hash)) - end - - # attributes - A Hash containing the raw lfs_object details. The keys of this - # Hash must be Symbols. - def initialize(attributes) - @attributes = attributes - end - def github_identifiers { oid: oid diff --git a/lib/gitlab/github_import/representation/note.rb b/lib/gitlab/github_import/representation/note.rb index bcdb1a5459b46396e80057d648428d0691bfacb0..cadb929c07717cb9e476e107b907b0469f38da65 100644 --- a/lib/gitlab/github_import/representation/note.rb +++ b/lib/gitlab/github_import/representation/note.rb @@ -3,21 +3,13 @@ module Gitlab module GithubImport module Representation - class Note - include ToHash - include ExposeAttribute - - attr_reader :attributes - + class Note < Base expose_attribute :noteable_id, :noteable_type, :author, :note, :created_at, :updated_at, :note_id NOTEABLE_TYPE_REGEX = %r{/(?(pull|issues))/(?\d+)}i.freeze - # Builds a note from a GitHub API response. - # - # note - An instance of `Sawyer::Resource` containing the note details. - def self.from_api_response(note) + def parse(note) matches = note.html_url.match(NOTEABLE_TYPE_REGEX) if !matches || !matches[:type] @@ -34,7 +26,7 @@ def self.from_api_response(note) 'Issue' end - user = Representation::User.from_api_response(note.user) if note.user + user = parse_with(Representation::User, note.user) if note.user hash = { noteable_type: noteable_type, noteable_id: matches[:iid].to_i, @@ -45,22 +37,13 @@ def self.from_api_response(note) note_id: note.id } - new(hash) + super(hash) end - # Builds a new note using a Hash that was built from a JSON payload. - def self.from_json_hash(raw_hash) - hash = Representation.symbolize_hash(raw_hash) - - hash[:author] &&= Representation::User.from_json_hash(hash[:author]) - - new(hash) - end + def deserialize(hash) + super(hash) - # attributes - A Hash containing the raw note details. The keys of this - # Hash must be Symbols. - def initialize(attributes) - @attributes = attributes + attributes[:author] &&= deserialize_with(Representation::User, attributes[:author]) end alias_method :issuable_type, :noteable_type diff --git a/lib/gitlab/github_import/representation/pull_request.rb b/lib/gitlab/github_import/representation/pull_request.rb index 82bcdee8b2bffebea1eea14c1939116c256816b0..bd69e9d28a24ba71fa93ac0d4af69bd456ee843b 100644 --- a/lib/gitlab/github_import/representation/pull_request.rb +++ b/lib/gitlab/github_import/representation/pull_request.rb @@ -3,25 +3,17 @@ module Gitlab module GithubImport module Representation - class PullRequest - include ToHash - include ExposeAttribute - - attr_reader :attributes - + class PullRequest < Base expose_attribute :iid, :title, :description, :source_branch, :source_branch_sha, :target_branch, :target_branch_sha, :milestone_number, :author, :assignee, :created_at, - :updated_at, :merged_at, :source_repository_id, + :updated_at, :source_repository_id, :target_repository_id, :source_repository_owner, :merged_by - # Builds a PR from a GitHub API response. - # - # issue - An instance of `Sawyer::Resource` containing the PR details. - def self.from_api_response(pr) - assignee = Representation::User.from_api_response(pr.assignee) if pr.assignee - user = Representation::User.from_api_response(pr.user) if pr.user - merged_by = Representation::User.from_api_response(pr.merged_by) if pr.merged_by + def parse(pr) + assignee = parse_with(Representation::User, pr.assignee) if pr.assignee + user = parse_with(Representation::User, pr.user) if pr.user + merged_by = parse_with(Representation::User, pr.merged_by) if pr.merged_by hash = { iid: pr.number, @@ -34,7 +26,7 @@ def self.from_api_response(pr) source_repository_id: pr.head&.repo&.id, target_repository_id: pr.base&.repo&.id, source_repository_owner: pr.head&.user&.login, - state: pr.state == 'open' ? :opened : :closed, + state: pr.state, milestone_number: pr.milestone&.number, author: user, assignee: assignee, @@ -44,28 +36,18 @@ def self.from_api_response(pr) merged_by: merged_by } - new(hash) + super(hash) end - # Builds a new PR using a Hash that was built from a JSON payload. - def self.from_json_hash(raw_hash) - hash = Representation.symbolize_hash(raw_hash) - - hash[:state] = hash[:state].to_sym - hash[:author] &&= Representation::User.from_json_hash(hash[:author]) + def deserialize(hash) + super(hash) - # Assignees are optional so we only convert it from a Hash if one was - # set. - hash[:assignee] &&= Representation::User.from_json_hash(hash[:assignee]) - hash[:merged_by] &&= Representation::User.from_json_hash(hash[:merged_by]) + attributes[:state] = attributes[:state].to_s + attributes[:author] &&= deserialize_with(Representation::User, attributes[:author]) - new(hash) - end - - # attributes - A Hash containing the raw PR details. The keys of this - # Hash (and any nested hashes) must be symbols. - def initialize(attributes) - @attributes = attributes + # Assignees are optional so we only convert it from a attributes if one was set. + attributes[:assignee] &&= deserialize_with(Representation::User, attributes[:assignee]) + attributes[:merged_by] &&= deserialize_with(Representation::User, attributes[:merged_by]) end def truncated_title @@ -90,11 +72,17 @@ def formatted_source_branch end end + def merged_at + attributes[:merged_at].presence + end + def state if merged_at :merged + elsif attributes[:state] == 'open' + :opened else - attributes[:state] + :closed end end diff --git a/lib/gitlab/github_import/representation/pull_request_review.rb b/lib/gitlab/github_import/representation/pull_request_review.rb index 70c1e51ffddc5ee85ea9e23860ef4de79c3daf2e..e52c57912fbc4f4f34dc0b5aa442fb043a14c048 100644 --- a/lib/gitlab/github_import/representation/pull_request_review.rb +++ b/lib/gitlab/github_import/representation/pull_request_review.rb @@ -3,18 +3,13 @@ module Gitlab module GithubImport module Representation - class PullRequestReview - include ToHash - include ExposeAttribute + class PullRequestReview < Base + expose_attribute :author, :note, :review_type, :merge_request_id, :review_id - attr_reader :attributes + def parse(review) + user = parse_with(Representation::User, review.user) if review.user - expose_attribute :author, :note, :review_type, :submitted_at, :merge_request_id, :review_id - - def self.from_api_response(review) - user = Representation::User.from_api_response(review.user) if review.user - - new( + super( merge_request_id: review.merge_request_id, author: user, note: review.body, @@ -24,26 +19,20 @@ def self.from_api_response(review) ) end - # Builds a new note using a Hash that was built from a JSON payload. - def self.from_json_hash(raw_hash) - hash = Representation.symbolize_hash(raw_hash) + def deserialize(hash) + super(hash) - hash[:author] &&= Representation::User.from_json_hash(hash[:author]) - hash[:submitted_at] = Time.parse(hash[:submitted_at]).in_time_zone if hash[:submitted_at].present? - - new(hash) - end - - # attributes - A Hash containing the raw note details. The keys of this - # Hash must be Symbols. - def initialize(attributes) - @attributes = attributes + attributes[:author] &&= deserialize_with(Representation::User, attributes[:author]) end def approval? review_type == 'APPROVED' end + def submitted_at + attributes[:submitted_at].presence + end + def github_identifiers { review_id: review_id, diff --git a/lib/gitlab/github_import/representation/user.rb b/lib/gitlab/github_import/representation/user.rb index fac8920a3f242937056f556b2a4af2be4a122303..43fa1deee6d8d2bca7236bc7150eaa13d40e53c3 100644 --- a/lib/gitlab/github_import/representation/user.rb +++ b/lib/gitlab/github_import/representation/user.rb @@ -3,33 +3,20 @@ module Gitlab module GithubImport module Representation - class User - include ToHash - include ExposeAttribute - - attr_reader :attributes - + class User < Base expose_attribute :id, :login - # Builds a user from a GitHub API response. - # - # user - An instance of `Sawyer::Resource` containing the user details. - def self.from_api_response(user) - new( + def parse(user) + super( id: user.id, login: user.login ) end - # Builds a user using a Hash that was built from a JSON payload. - def self.from_json_hash(raw_hash) - new(Representation.symbolize_hash(raw_hash)) - end - - # attributes - A Hash containing the user details. The keys of this - # Hash (and any nested hashes) must be symbols. - def initialize(attributes) - @attributes = attributes + def github_identifiers + { + id: id + } end end end diff --git a/spec/lib/gitlab/github_import/representation/diff_note_spec.rb b/spec/lib/gitlab/github_import/representation/diff_note_spec.rb index 81722c0eba740caec04d4b5d0d09bed2099b9e1e..ec64d9b3a5381487b71d88d25cfd50d39f68e271 100644 --- a/spec/lib/gitlab/github_import/representation/diff_note_spec.rb +++ b/spec/lib/gitlab/github_import/representation/diff_note_spec.rb @@ -3,257 +3,212 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::Representation::DiffNote do + let(:context) { Gitlab::GithubImport::Context.new(project: double, client: double) } + let(:start_line) { nil } + let(:end_line) { 23 } + let(:note_body) { 'Hello world' } + let(:user_data) { { 'id' => 4, 'login' => 'alice' } } + let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } + let(:created_at) { Time.new(2017, 1, 1, 12, 00) } + let(:hunk) do '@@ -1 +1 @@ -Hello +Hello world' end - let(:created_at) { Time.new(2017, 1, 1, 12, 00) } - let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } + subject(:representation) { described_class.new(context) } shared_examples 'a DiffNote' do it 'returns an instance of DiffNote' do - expect(note).to be_an_instance_of(described_class) + expect(subject).to be_an_instance_of(described_class) end context 'the returned DiffNote' do it 'includes the number of the note' do - expect(note.noteable_id).to eq(42) + expect(subject.noteable_id).to eq(42) end it 'includes the file path of the diff' do - expect(note.file_path).to eq('README.md') + expect(subject.file_path).to eq('README.md') end it 'includes the commit ID' do - expect(note.commit_id).to eq('123abc') + expect(subject.commit_id).to eq('123abc') end it 'includes the user details' do - expect(note.author) + expect(subject.author) .to be_an_instance_of(Gitlab::GithubImport::Representation::User) - expect(note.author.id).to eq(4) - expect(note.author.login).to eq('alice') + expect(subject.author.id).to eq(4) + expect(subject.author.login).to eq('alice') end it 'includes the note body' do - expect(note.note).to eq('Hello world') + expect(subject.note).to eq('Hello world') end it 'includes the created timestamp' do - expect(note.created_at).to eq(created_at) + expect(subject.created_at).to eq(created_at) end it 'includes the updated timestamp' do - expect(note.updated_at).to eq(updated_at) + expect(subject.updated_at).to eq(updated_at) end it 'includes the GitHub ID' do - expect(note.note_id).to eq(1) + expect(subject.note_id).to eq(1) end it 'returns the noteable type' do - expect(note.noteable_type).to eq('MergeRequest') + expect(subject.noteable_type).to eq('MergeRequest') end - end - end - describe '.from_api_response' do - let(:response) do - double( - :response, - html_url: 'https://github.com/foo/bar/pull/42', - path: 'README.md', - commit_id: '123abc', - original_commit_id: 'original123abc', - diff_hunk: hunk, - user: double(:user, id: 4, login: 'alice'), - body: 'Hello world', - created_at: created_at, - updated_at: updated_at, - line: 23, - start_line: nil, - id: 1 - ) - end - - it_behaves_like 'a DiffNote' do - let(:note) { described_class.from_api_response(response) } - end - - it 'does not set the user if the response did not include a user' do - allow(response) - .to receive(:user) - .and_return(nil) + describe '#line_code' do + it { expect(subject.line_code).to eq('8ec9a00bfd09b3190ac6b22251dbb1aa95a0579d_2_2') } + end - note = described_class.from_api_response(response) + describe '#github_identifiers' do + it do + expect(subject.github_identifiers).to eq( + noteable_id: 42, + noteable_type: 'MergeRequest', + note_id: 1 + ) + end + end - expect(note.author).to be_nil - end + describe '#note' do + it 'includes the note body' do + expect(subject.note).to eq('Hello world') + end + + context 'when the note have a suggestion' do + let(:note_body) do + <<~BODY + ```suggestion + Hello World + ``` + BODY + end + + it 'returns the suggestion formatted in the note' do + expect(subject.note).to eq <<~BODY + ```suggestion:-0+0 + Hello World + ``` + BODY + end + end + + context 'when the note have a multiline suggestion' do + let(:start_line) { 20 } + let(:end_line) { 23 } + let(:note_body) do + <<~BODY + ```suggestion + Hello World + ``` + BODY + end + + it 'returns the multi-line suggestion formatted in the note' do + expect(subject.note).to eq <<~BODY + ```suggestion:-3+0 + Hello World + ``` + BODY + end + end + end - it 'formats a suggestion in the note body' do - allow(response) - .to receive(:body) - .and_return <<~BODY - ```suggestion - Hello World - ``` - BODY + describe '#author' do + let(:user_data) { nil } - note = described_class.from_api_response(response) + it 'does not convert the author if it was not specified' do + expect(subject.author).to be_nil + end + end - expect(note.note).to eq <<~BODY - ```suggestion:-0+0 - Hello World - ``` - BODY + describe '#diff_hash' do + it 'returns a Hash containing the diff details' do + subject.deserialize( + 'noteable_type' => 'MergeRequest', + 'noteable_id' => 42, + 'file_path' => 'README.md', + 'commit_id' => '123abc', + 'original_commit_id' => 'original123abc', + 'diff_hunk' => hunk, + 'author' => { 'id' => 4, 'login' => 'alice' }, + 'note' => 'Hello world', + 'created_at' => created_at.to_s, + 'updated_at' => updated_at.to_s, + 'note_id' => 1 + ) + + expect(subject.diff_hash).to eq( + diff: hunk, + new_path: 'README.md', + old_path: 'README.md', + a_mode: '100644', + b_mode: '100644', + new_file: false + ) + end + end end end - describe '.from_json_hash' do - let(:hash) do - { - 'noteable_type' => 'MergeRequest', - 'noteable_id' => 42, - 'file_path' => 'README.md', - 'commit_id' => '123abc', - 'original_commit_id' => 'original123abc', - 'diff_hunk' => hunk, - 'author' => { 'id' => 4, 'login' => 'alice' }, - 'note' => 'Hello world', - 'created_at' => created_at.to_s, - 'updated_at' => updated_at.to_s, - 'note_id' => 1 - } - end - + describe '#parse' do it_behaves_like 'a DiffNote' do - let(:note) { described_class.from_json_hash(hash) } - end - - it 'does not convert the author if it was not specified' do - hash.delete('author') - - note = described_class.from_json_hash(hash) - - expect(note.author).to be_nil - end - - it 'formats a suggestion in the note body' do - hash['note'] = <<~BODY - ```suggestion - Hello World - ``` - BODY - - note = described_class.from_json_hash(hash) - - expect(note.note).to eq <<~BODY - ```suggestion:-0+0 - Hello World - ``` - BODY - end - end - - describe '#line_code' do - it 'returns a String' do - note = described_class.new(diff_hunk: hunk, file_path: 'README.md') - - expect(note.line_code).to be_an_instance_of(String) - end - end - - describe '#diff_hash' do - it 'returns a Hash containing the diff details' do - note = described_class.from_json_hash( - 'noteable_type' => 'MergeRequest', - 'noteable_id' => 42, - 'file_path' => 'README.md', - 'commit_id' => '123abc', - 'original_commit_id' => 'original123abc', - 'diff_hunk' => hunk, - 'author' => { 'id' => 4, 'login' => 'alice' }, - 'note' => 'Hello world', - 'created_at' => created_at.to_s, - 'updated_at' => updated_at.to_s, - 'note_id' => 1 - ) - - expect(note.diff_hash).to eq( - diff: hunk, - new_path: 'README.md', - old_path: 'README.md', - a_mode: '100644', - b_mode: '100644', - new_file: false - ) - end - end - - describe '#github_identifiers' do - it 'returns a hash with needed identifiers' do - github_identifiers = { - noteable_id: 42, - noteable_type: 'MergeRequest', - note_id: 1 - } - other_attributes = { something_else: '_something_else_' } - note = described_class.new(github_identifiers.merge(other_attributes)) + let(:response) do + double( + :response, + id: 1, + html_url: 'https://github.com/foo/bar/pull/42', + path: 'README.md', + commit_id: '123abc', + original_commit_id: 'original123abc', + user: user_data && double(:user, user_data), + diff_hunk: hunk, + body: note_body, + created_at: created_at, + updated_at: updated_at, + line: end_line, + start_line: start_line + ) + end - expect(note.github_identifiers).to eq(github_identifiers) + before do + subject.parse(response) + end end end - describe '#note' do - it 'returns the given note' do - hash = { - 'note': 'simple text' - } - - note = described_class.new(hash) - - expect(note.note).to eq 'simple text' - end - - it 'returns the suggestion formatted in the note' do - hash = { - 'note': <<~BODY - ```suggestion - Hello World - ``` - BODY - } - - note = described_class.new(hash) - - expect(note.note).to eq <<~BODY - ```suggestion:-0+0 - Hello World - ``` - BODY - end - - it 'returns the multi-line suggestion formatted in the note' do - hash = { - 'start_line': 20, - 'end_line': 23, - 'note': <<~BODY - ```suggestion - Hello World - ``` - BODY - } - - note = described_class.new(hash) + describe '#deserialize' do + it_behaves_like 'a DiffNote' do + let(:hash) do + { + 'note_id' => 1, + 'noteable_type' => 'MergeRequest', + 'noteable_id' => 42, + 'file_path' => 'README.md', + 'commit_id' => '123abc', + 'original_commit_id' => 'original123abc', + 'author' => user_data, + 'diff_hunk' => hunk, + 'note' => note_body, + 'created_at' => created_at.to_s, + 'updated_at' => updated_at.to_s, + 'end_line' => end_line, + 'start_line' => start_line + } + end - expect(note.note).to eq <<~BODY - ```suggestion:-3+0 - Hello World - ``` - BODY + before do + subject.deserialize(hash) + end end end end diff --git a/spec/lib/gitlab/github_import/representation/issue_spec.rb b/spec/lib/gitlab/github_import/representation/issue_spec.rb index f3052efea70623752f0ac8c176b2c20aaa880bc4..4f14e8d773866ebb17461fd05de19092622fcbf8 100644 --- a/spec/lib/gitlab/github_import/representation/issue_spec.rb +++ b/spec/lib/gitlab/github_import/representation/issue_spec.rb @@ -3,89 +3,149 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::Representation::Issue do + let(:context) { Gitlab::GithubImport::Context.new(project: double, client: double) } + + let(:label_names) { ['bug'] } + let(:title) { 'My Issue' } + let(:pull_request) { false } let(:created_at) { Time.new(2017, 1, 1, 12, 00) } let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } - shared_examples 'an Issue' do + subject(:representation) { described_class.new(context) } + + shared_examples 'an Issue Representation' do it 'returns an instance of Issue' do - expect(issue).to be_an_instance_of(described_class) + expect(subject).to be_an_instance_of(described_class) end context 'the returned Issue' do it 'includes the issue number' do - expect(issue.iid).to eq(42) + expect(subject.iid).to eq(42) end it 'includes the issue title' do - expect(issue.title).to eq('My Issue') + expect(subject.title).to eq('My Issue') end it 'includes the issue description' do - expect(issue.description).to eq('This is my issue') + expect(subject.description).to eq('This is my issue') end it 'includes the milestone number' do - expect(issue.milestone_number).to eq(4) + expect(subject.milestone_number).to eq(4) end it 'includes the issue state' do - expect(issue.state).to eq(:opened) + expect(subject.state).to eq(:opened) end it 'includes the issue assignees' do - expect(issue.assignees[0]) + expect(subject.assignees[0]) .to be_an_instance_of(Gitlab::GithubImport::Representation::User) - expect(issue.assignees[0].id).to eq(4) - expect(issue.assignees[0].login).to eq('alice') - end - - it 'includes the label names' do - expect(issue.label_names).to eq(%w[bug]) + expect(subject.assignees[0].id).to eq(4) + expect(subject.assignees[0].login).to eq('alice') end it 'includes the author details' do - expect(issue.author) + expect(subject.author) .to be_an_instance_of(Gitlab::GithubImport::Representation::User) - expect(issue.author.id).to eq(4) - expect(issue.author.login).to eq('alice') + expect(subject.author.id).to eq(4) + expect(subject.author.login).to eq('alice') end it 'includes the created timestamp' do - expect(issue.created_at).to eq(created_at) + expect(subject.created_at).to eq(created_at) end it 'includes the updated timestamp' do - expect(issue.updated_at).to eq(updated_at) + expect(subject.updated_at).to eq(updated_at) end it 'is not a pull request' do - expect(issue.pull_request?).to eq(false) + expect(subject.pull_request?).to eq(false) + end + + describe '#pull_request?' do + context 'when it is not a pull request' do + let(:pull_request) { false } + + it { expect(subject.pull_request?).to eq(false) } + end + + context 'when it is a pull request' do + let(:pull_request) { true } + + it { expect(subject.pull_request?).to eq(true) } + end + end + + describe '#truncated_title' do + context 'truncates the title to 255 characters' do + let(:title) { 'm' * 300 } + + it { expect(subject.truncated_title.length).to eq(255) } + end + + context 'does not truncate the title if it is shorter than 255 characters' do + let(:title) { 'foo' } + + it { expect(subject.truncated_title).to eq('foo') } + end + end + + describe '#github_identifiers' do + it 'returns a hash with needed identifiers' do + expect(subject.github_identifiers).to eq( + iid: 42, + issuable_type: 'Issue' + ) + end + end + + describe '#labels?' do + it 'includes the label names' do + expect(subject.label_names).to eq(label_names) + end + + it 'returns true when the issue has labels assigned' do + expect(subject.labels?).to eq(true) + end + + context 'when no label is given' do + let(:label_names) { [] } + + it 'returns false when the issue has no labels assigned' do + expect(subject.labels?).to eq(false) + end + end end end end - describe '.from_api_response' do + describe '#parse' do let(:response) do double( :response, number: 42, - title: 'My Issue', + title: title, body: 'This is my issue', milestone: double(:milestone, number: 4), state: 'open', assignees: [double(:user, id: 4, login: 'alice')], - labels: [double(:label, name: 'bug')], + labels: label_names.map { double(:label, name: _1) }, user: double(:user, id: 4, login: 'alice'), created_at: created_at, updated_at: updated_at, - pull_request: false + pull_request: pull_request ) end - it_behaves_like 'an Issue' do - let(:issue) { described_class.from_api_response(response) } + it_behaves_like 'an Issue Representation' do + before do + subject.parse(response) + end end it 'does not set the user if the response did not include a user' do @@ -93,105 +153,41 @@ .to receive(:user) .and_return(nil) - issue = described_class.from_api_response(response) + subject.parse(response) - expect(issue.author).to be_nil + expect(subject.author).to be_nil end end - describe '.from_json_hash' do - it_behaves_like 'an Issue' do - let(:hash) do - { - 'iid' => 42, - 'title' => 'My Issue', - 'description' => 'This is my issue', - 'milestone_number' => 4, - 'state' => 'opened', - 'assignees' => [{ 'id' => 4, 'login' => 'alice' }], - 'label_names' => %w[bug], - 'author' => { 'id' => 4, 'login' => 'alice' }, - 'created_at' => created_at.to_s, - 'updated_at' => updated_at.to_s, - 'pull_request' => false - } - end - - let(:issue) { described_class.from_json_hash(hash) } - end - - it 'does not convert the author if it was not specified' do - hash = { + describe '#deserialize' do + let(:hash) do + { 'iid' => 42, - 'title' => 'My Issue', + 'title' => title, 'description' => 'This is my issue', 'milestone_number' => 4, 'state' => 'opened', 'assignees' => [{ 'id' => 4, 'login' => 'alice' }], - 'label_names' => %w[bug], + 'label_names' => label_names, + 'author' => { 'id' => 4, 'login' => 'alice' }, 'created_at' => created_at.to_s, 'updated_at' => updated_at.to_s, - 'pull_request' => false + 'pull_request' => pull_request } - - issue = described_class.from_json_hash(hash) - - expect(issue.author).to be_nil - end - end - - describe '#labels?' do - it 'returns true when the issue has labels assigned' do - issue = described_class.new(label_names: %w[bug]) - - expect(issue.labels?).to eq(true) - end - - it 'returns false when the issue has no labels assigned' do - issue = described_class.new(label_names: []) - - expect(issue.labels?).to eq(false) - end - end - - describe '#pull_request?' do - it 'returns false for an issue' do - issue = described_class.new(pull_request: false) - - expect(issue.pull_request?).to eq(false) - end - - it 'returns true for a pull request' do - issue = described_class.new(pull_request: true) - - expect(issue.pull_request?).to eq(true) end - end - - describe '#truncated_title' do - it 'truncates the title to 255 characters' do - object = described_class.new(title: 'm' * 300) - expect(object.truncated_title.length).to eq(255) + it_behaves_like 'an Issue Representation' do + before do + subject.deserialize(hash) + end end - it 'does not truncate the title if it is shorter than 255 characters' do - object = described_class.new(title: 'foo') - - expect(object.truncated_title).to eq('foo') - end - end + it 'does not convert the author if it was not specified' do + hash.delete('author') - describe '#github_identifiers' do - it 'returns a hash with needed identifiers' do - github_identifiers = { - iid: 42, - issuable_type: 'MergeRequest' - } - other_attributes = { pull_request: true, something_else: '_something_else_' } - issue = described_class.new(github_identifiers.merge(other_attributes)) + subject.deserialize(hash) - expect(issue.github_identifiers).to eq(github_identifiers) + expect(subject.author).to be_nil end end end diff --git a/spec/lib/gitlab/github_import/representation/lfs_object_spec.rb b/spec/lib/gitlab/github_import/representation/lfs_object_spec.rb index b59ea5134369d1db63cd054781d1a73389acf226..d48a9a964a5bef7391ff047df959f8326b0f1237 100644 --- a/spec/lib/gitlab/github_import/representation/lfs_object_spec.rb +++ b/spec/lib/gitlab/github_import/representation/lfs_object_spec.rb @@ -3,15 +3,38 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::Representation::LfsObject do - describe '#github_identifiers' do - it 'returns a hash with needed identifiers' do - github_identifiers = { - oid: 42 - } - other_attributes = { something_else: '_something_else_' } - lfs_object = described_class.new(github_identifiers.merge(other_attributes)) - - expect(lfs_object.github_identifiers).to eq(github_identifiers) + let(:context) { Gitlab::GithubImport::Context.new(project: double, client: double) } + let(:lfs_data) { { 'oid' => 42, 'link' => 'link', 'size' => 256 } } + + subject(:representation) { described_class.new(context) } + + shared_examples 'a LFS Representation' do + context 'exposed attributes' do + it { expect(subject.oid).to eq(42) } + it { expect(subject.link).to eq('link') } + it { expect(subject.size).to eq(256) } + end + + describe '#github_identifiers' do + it { expect(subject.github_identifiers).to eq(oid: 42) } end end + + describe '#parse' do + let(:response) { double(:response, lfs_data) } + + before do + subject.parse(response) + end + + it_behaves_like 'a LFS Representation' + end + + describe '#deserialize' do + before do + subject.deserialize(lfs_data) + end + + it_behaves_like 'a LFS Representation' + end end diff --git a/spec/lib/gitlab/github_import/representation/note_spec.rb b/spec/lib/gitlab/github_import/representation/note_spec.rb index 97addcc1c98c595b7cc0205f0a725c3b5f0d3e33..a17d973bd0a96b8d1eda47e12fc5251a9fe4c0f5 100644 --- a/spec/lib/gitlab/github_import/representation/note_spec.rb +++ b/spec/lib/gitlab/github_import/representation/note_spec.rb @@ -3,84 +3,97 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::Representation::Note do + let(:context) { Gitlab::GithubImport::Context.new(project: double, client: double) } + + let(:user_data) { { id: 4, login: 'alice' } } let(:created_at) { Time.new(2017, 1, 1, 12, 00) } let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } - shared_examples 'a Note' do + subject(:representation) { described_class.new(context) } + + shared_examples 'a Note Representation' do it 'returns an instance of Note' do - expect(note).to be_an_instance_of(described_class) + expect(subject).to be_an_instance_of(described_class) end context 'the returned Note' do it 'includes the noteable ID' do - expect(note.noteable_id).to eq(42) + expect(subject.noteable_id).to eq(42) end it 'includes the noteable type' do - expect(note.noteable_type).to eq('Issue') + expect(subject.noteable_type).to eq('Issue') end it 'includes the author details' do - expect(note.author) + expect(subject.author) .to be_an_instance_of(Gitlab::GithubImport::Representation::User) - expect(note.author.id).to eq(4) - expect(note.author.login).to eq('alice') + expect(subject.author.id).to eq(4) + expect(subject.author.login).to eq('alice') end it 'includes the note body' do - expect(note.note).to eq('Hello world') + expect(subject.note).to eq('Hello world') end it 'includes the created timestamp' do - expect(note.created_at).to eq(created_at) + expect(subject.created_at).to eq(created_at) end it 'includes the updated timestamp' do - expect(note.updated_at).to eq(updated_at) + expect(subject.updated_at).to eq(updated_at) end it 'includes the note ID' do - expect(note.note_id).to eq(1) + expect(subject.note_id).to eq(1) end - end - end - - describe '.from_api_response' do - let(:response) do - double( - :response, - html_url: 'https://github.com/foo/bar/issues/42', - user: double(:user, id: 4, login: 'alice'), - body: 'Hello world', - created_at: created_at, - updated_at: updated_at, - id: 1 - ) - end - it_behaves_like 'a Note' do - let(:note) { described_class.from_api_response(response) } - end + describe '#github_identifiers' do + it 'returns a hash with needed identifiers' do + expect(subject.github_identifiers).to eq( + noteable_id: 42, + noteable_type: 'Issue', + note_id: 1 + ) + end + end - it 'does not set the user if the response did not include a user' do - allow(response) - .to receive(:user) - .and_return(nil) + describe '#author' do + let(:user_data) { nil } - note = described_class.from_api_response(response) + it 'does not set the user if the response did not include a user' do + expect(subject.author).to be_nil + end + end + end + end - expect(note.author).to be_nil + describe '#parse' do + it_behaves_like 'a Note Representation' do + before { subject.parse(response) } + + let(:response) do + double( + :response, + html_url: 'https://github.com/foo/bar/issues/42', + user: user_data && double(:user, user_data), + body: 'Hello world', + created_at: created_at, + updated_at: updated_at, + id: 1 + ) + end end end - describe '.from_json_hash' do - it_behaves_like 'a Note' do + describe '#deserialize' do + it_behaves_like 'a Note Representation' do let(:hash) do { 'noteable_id' => 42, 'noteable_type' => 'Issue', - 'author' => { 'id' => 4, 'login' => 'alice' }, + 'author' => user_data, 'note' => 'Hello world', 'created_at' => created_at.to_s, 'updated_at' => updated_at.to_s, @@ -88,36 +101,9 @@ } end - let(:note) { described_class.from_json_hash(hash) } - end - - it 'does not convert the author if it was not specified' do - hash = { - 'noteable_id' => 42, - 'noteable_type' => 'Issue', - 'note' => 'Hello world', - 'created_at' => created_at.to_s, - 'updated_at' => updated_at.to_s, - 'note_id' => 1 - } - - note = described_class.from_json_hash(hash) - - expect(note.author).to be_nil - end - end - - describe '#github_identifiers' do - it 'returns a hash with needed identifiers' do - github_identifiers = { - noteable_id: 42, - noteable_type: 'Issue', - note_id: 1 - } - other_attributes = { something_else: '_something_else_' } - note = described_class.new(github_identifiers.merge(other_attributes)) - - expect(note.github_identifiers).to eq(github_identifiers) + before do + subject.deserialize(hash) + end end end end diff --git a/spec/lib/gitlab/github_import/representation/pull_request_review_spec.rb b/spec/lib/gitlab/github_import/representation/pull_request_review_spec.rb index f812fd85fbc68b8825d5b8114375b918d4c1774e..0733e706cf72cc1a7a68e34ba9fa845e35884324 100644 --- a/spec/lib/gitlab/github_import/representation/pull_request_review_spec.rb +++ b/spec/lib/gitlab/github_import/representation/pull_request_review_spec.rb @@ -3,89 +3,88 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::Representation::PullRequestReview do + let(:context) { Gitlab::GithubImport::Context.new(project: double, client: double) } + let(:submitted_at) { Time.new(2017, 1, 1, 12, 00).utc } + let(:user_data) { { id: 4, login: 'alice' } } + + subject(:representation) { described_class.new(context) } shared_examples 'a PullRequest review' do it 'returns an instance of PullRequest' do - expect(review).to be_an_instance_of(described_class) - expect(review.author).to be_an_instance_of(Gitlab::GithubImport::Representation::User) - expect(review.author.id).to eq(4) - expect(review.author.login).to eq('alice') - expect(review.note).to eq('note') - expect(review.review_type).to eq('APPROVED') - expect(review.submitted_at).to eq(submitted_at) - expect(review.review_id).to eq(999) - expect(review.merge_request_id).to eq(42) + expect(subject).to be_an_instance_of(described_class) + expect(subject.author).to be_an_instance_of(Gitlab::GithubImport::Representation::User) + expect(subject.author.id).to eq(4) + expect(subject.author.login).to eq('alice') + expect(subject.note).to eq('note') + expect(subject.review_type).to eq('APPROVED') + expect(subject.submitted_at).to eq(submitted_at) + expect(subject.review_id).to eq(999) + expect(subject.merge_request_id).to eq(42) end - end - describe '.from_api_response' do - let(:response) do - double( - :response, - id: 999, - merge_request_id: 42, - body: 'note', - state: 'APPROVED', - user: double(:user, id: 4, login: 'alice'), - submitted_at: submitted_at - ) - end + describe '#submitted_at' do + let(:submitted_at) { nil } - it_behaves_like 'a PullRequest review' do - let(:review) { described_class.from_api_response(response) } + it 'does not fail when submitted_at is blank' do + expect(subject.submitted_at).to be_nil + end end - it 'does not set the user if the response did not include a user' do - allow(response) - .to receive(:user) - .and_return(nil) - - review = described_class.from_api_response(response) + describe '#author' do + let(:user_data) { nil } - expect(review.author).to be_nil + it 'does not set the user if the response did not include a user' do + expect(subject.author).to be_nil + end end - end - describe '.from_json_hash' do - let(:hash) do - { - 'review_id' => 999, - 'merge_request_id' => 42, - 'note' => 'note', - 'review_type' => 'APPROVED', - 'author' => { 'id' => 4, 'login' => 'alice' }, - 'submitted_at' => submitted_at.to_s - } + describe '#github_identifiers' do + it 'returns a hash with needed identifiers' do + expect(subject.github_identifiers).to eq( + review_id: 999, + merge_request_id: 42 + ) + end end + end + describe '#parse' do it_behaves_like 'a PullRequest review' do - let(:review) { described_class.from_json_hash(hash) } - end - - it 'does not set the user if the response did not include a user' do - review = described_class.from_json_hash(hash.except('author')) - - expect(review.author).to be_nil - end - - it 'does not fail when submitted_at is blank' do - review = described_class.from_json_hash(hash.except('submitted_at')) - - expect(review.submitted_at).to be_nil + let(:response) do + double( + :response, + id: 999, + merge_request_id: 42, + body: 'note', + state: 'APPROVED', + user: user_data && double(:user, user_data), + submitted_at: submitted_at + ) + end + + before do + subject.parse(response) + end end end - describe '#github_identifiers' do - it 'returns a hash with needed identifiers' do - github_identifiers = { - review_id: 999, - merge_request_id: 42 - } - other_attributes = { something_else: '_something_else_' } - review = described_class.new(github_identifiers.merge(other_attributes)) - - expect(review.github_identifiers).to eq(github_identifiers) + describe '#deserialize' do + it_behaves_like 'a PullRequest review' do + let(:hash) do + { + 'review_id' => 999, + 'merge_request_id' => 42, + 'note' => 'note', + 'review_type' => 'APPROVED', + 'author' => user_data, + 'submitted_at' => submitted_at.to_s + } + end + + before do + subject.deserialize(hash) + end end end end diff --git a/spec/lib/gitlab/github_import/representation/pull_request_spec.rb b/spec/lib/gitlab/github_import/representation/pull_request_spec.rb index 925dba5b5a7b64bebd69db40596649141cca5caa..e579c5f99e1aa9da19c6ff61c3defe10a4478db8 100644 --- a/spec/lib/gitlab/github_import/representation/pull_request_spec.rb +++ b/spec/lib/gitlab/github_import/representation/pull_request_spec.rb @@ -3,301 +3,279 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::Representation::PullRequest do + let(:context) { Gitlab::GithubImport::Context.new(project: double, client: double) } + let(:created_at) { Time.new(2017, 1, 1, 12, 00) } let(:updated_at) { Time.new(2017, 1, 1, 12, 15) } - let(:merged_at) { Time.new(2017, 1, 1, 12, 17) } + let(:user_data) { { id: 4, login: 'alice' } } + let(:merged_at) { nil } + let(:title) { 'My Pull Request' } + let(:state) { :closed } + let(:source_repository_owner) { 'alice' } + let(:source_repository_id) { 200 } + let(:source_branch) { 'my-feature' } + let(:target_repository_id) { 400 } + let(:target_branch) { 'master' } + + subject(:representation) { described_class.new(context) } shared_examples 'a PullRequest' do it 'returns an instance of PullRequest' do - expect(pr).to be_an_instance_of(described_class) + expect(subject).to be_an_instance_of(described_class) end context 'the returned PullRequest' do it 'includes the pull request number' do - expect(pr.iid).to eq(42) + expect(subject.iid).to eq(42) end it 'includes the pull request title' do - expect(pr.title).to eq('My Pull Request') + expect(subject.title).to eq('My Pull Request') end it 'includes the pull request description' do - expect(pr.description).to eq('This is my pull request') + expect(subject.description).to eq('This is my pull request') end it 'includes the source branch name' do - expect(pr.source_branch).to eq('my-feature') + expect(subject.source_branch).to eq('my-feature') end it 'includes the source branch SHA' do - expect(pr.source_branch_sha).to eq('123abc') + expect(subject.source_branch_sha).to eq('123abc') end it 'includes the target branch name' do - expect(pr.target_branch).to eq('master') + expect(subject.target_branch).to eq('master') end it 'includes the target branch SHA' do - expect(pr.target_branch_sha).to eq('456def') + expect(subject.target_branch_sha).to eq('456def') end it 'includes the milestone number' do - expect(pr.milestone_number).to eq(4) + expect(subject.milestone_number).to eq(4) end it 'includes the user details' do - expect(pr.author) - .to be_an_instance_of(Gitlab::GithubImport::Representation::User) + expect(subject.author).to be_an_instance_of( + Gitlab::GithubImport::Representation::User + ) - expect(pr.author.id).to eq(4) - expect(pr.author.login).to eq('alice') + expect(subject.author.id).to eq(4) + expect(subject.author.login).to eq('alice') end it 'includes the assignee details' do - expect(pr.assignee) + expect(subject.assignee) .to be_an_instance_of(Gitlab::GithubImport::Representation::User) - expect(pr.assignee.id).to eq(4) - expect(pr.assignee.login).to eq('alice') + expect(subject.assignee.id).to eq(4) + expect(subject.assignee.login).to eq('alice') end it 'includes the created timestamp' do - expect(pr.created_at).to eq(created_at) + expect(subject.created_at).to eq(created_at) end it 'includes the updated timestamp' do - expect(pr.updated_at).to eq(updated_at) + expect(subject.updated_at).to eq(updated_at) end it 'includes the merged timestamp' do - expect(pr.merged_at).to eq(merged_at) + expect(subject.merged_at).to eq(merged_at) end it 'includes the source repository ID' do - expect(pr.source_repository_id).to eq(400) + expect(subject.source_repository_id).to eq(source_repository_id) end it 'includes the target repository ID' do - expect(pr.target_repository_id).to eq(200) + expect(subject.target_repository_id).to eq(target_repository_id) end it 'includes the source repository owner name' do - expect(pr.source_repository_owner).to eq('alice') + expect(subject.source_repository_owner).to eq('alice') end - it 'includes the pull request state' do - expect(pr.state).to eq(:merged) + describe '#github_identifiers' do + it 'returns a hash with needed identifiers' do + expect(subject.github_identifiers).to eq( + iid: 42, + issuable_type: 'MergeRequest' + ) + end end - end - end - describe '.from_api_response' do - let(:response) do - double( - :response, - number: 42, - title: 'My Pull Request', - body: 'This is my pull request', - state: 'closed', - head: double( - :head, - sha: '123abc', - ref: 'my-feature', - repo: double(:repo, id: 400), - user: double(:user, id: 4, login: 'alice') - ), - base: double( - :base, - sha: '456def', - ref: 'master', - repo: double(:repo, id: 200) - ), - milestone: double(:milestone, number: 4), - user: double(:user, id: 4, login: 'alice'), - assignee: double(:user, id: 4, login: 'alice'), - merged_by: double(:user, id: 4, login: 'alice'), - created_at: created_at, - updated_at: updated_at, - merged_at: merged_at - ) - end + describe '#state' do + context 'returns :closed for a closed pull request' do + let(:state) { 'closed' } - it_behaves_like 'a PullRequest' do - let(:pr) { described_class.from_api_response(response) } - end + it { expect(subject.state).to eq(:closed) } + end - it 'does not set the user if the response did not include a user' do - allow(response) - .to receive(:user) - .and_return(nil) + context 'returns :merged for a merged pull request' do + let(:merged_at) { Time.new(2017, 1, 1, 12, 17) } + let(:state) { 'closed' } - pr = described_class.from_api_response(response) + it { expect(subject.state).to eq(:merged) } + end - expect(pr.author).to be_nil - end - end + context 'returns :opened for an open pull request' do + let(:state) { 'open' } - describe '.from_json_hash' do - it_behaves_like 'a PullRequest' do - let(:hash) do - { - 'iid' => 42, - 'title' => 'My Pull Request', - 'description' => 'This is my pull request', - 'source_branch' => 'my-feature', - 'source_branch_sha' => '123abc', - 'target_branch' => 'master', - 'target_branch_sha' => '456def', - 'source_repository_id' => 400, - 'target_repository_id' => 200, - 'source_repository_owner' => 'alice', - 'state' => 'closed', - 'milestone_number' => 4, - 'author' => { 'id' => 4, 'login' => 'alice' }, - 'assignee' => { 'id' => 4, 'login' => 'alice' }, - 'created_at' => created_at.to_s, - 'updated_at' => updated_at.to_s, - 'merged_at' => merged_at.to_s - } + it { expect(subject.state).to eq(:opened) } + end end - let(:pr) { described_class.from_json_hash(hash) } - end + describe '#cross_project?' do + context 'returns false for a pull request submitted from the target project' do + let(:source_repository_id) { 1 } + let(:target_repository_id) { 1 } - it 'does not convert the author if it was not specified' do - hash = { - 'iid' => 42, - 'title' => 'My Pull Request', - 'description' => 'This is my pull request', - 'source_branch' => 'my-feature', - 'source_branch_sha' => '123abc', - 'target_branch' => 'master', - 'target_branch_sha' => '456def', - 'source_repository_id' => 400, - 'target_repository_id' => 200, - 'source_repository_owner' => 'alice', - 'state' => 'closed', - 'milestone_number' => 4, - 'assignee' => { 'id' => 4, 'login' => 'alice' }, - 'created_at' => created_at.to_s, - 'updated_at' => updated_at.to_s, - 'merged_at' => merged_at.to_s - } - - pr = described_class.from_json_hash(hash) - - expect(pr.author).to be_nil - end - end + it { expect(subject).not_to be_cross_project } + end - describe '#state' do - it 'returns :opened for an open pull request' do - pr = described_class.new(state: :opened) + context 'returns true for a pull request submitted from a different project' do + let(:source_repository_id) { 1 } + let(:target_repository_id) { 2 } - expect(pr.state).to eq(:opened) - end + it { expect(subject).to be_cross_project } + end - it 'returns :closed for a closed pull request' do - pr = described_class.new(state: :closed) + context 'returns true if no source repository is present' do + let(:target_repository_id) { 2 } - expect(pr.state).to eq(:closed) - end - - it 'returns :merged for a merged pull request' do - pr = described_class.new(state: :closed, merged_at: merged_at) - - expect(pr.state).to eq(:merged) - end - end - - describe '#cross_project?' do - it 'returns false for a pull request submitted from the target project' do - pr = described_class.new(source_repository_id: 1, target_repository_id: 1) - - expect(pr).not_to be_cross_project - end + it { expect(subject).to be_cross_project } + end + end - it 'returns true for a pull request submitted from a different project' do - pr = described_class.new(source_repository_id: 1, target_repository_id: 2) + describe '#formatted_source_branch' do + context 'for a cross-project pull request' do + let(:source_repository_owner) { 'foo' } + let(:source_branch) { 'branch' } + let(:target_branch) { 'master' } + let(:source_repository_id) { 1 } + let(:target_repository_id) { 2 } + + it 'includes the owner name in the branch name' do + expect(subject.formatted_source_branch).to eq('github/fork/foo/branch') + end + end + + context 'for a regular pull request' do + let(:source_repository_owner) { 'foo' } + let(:source_branch) { 'branch' } + let(:target_branch) { 'master' } + let(:source_repository_id) { 1 } + let(:target_repository_id) { 1 } + + it 'returns the source branch name' do + expect(subject.formatted_source_branch).to eq('branch') + end + end + + context 'for a pull request with the same source and target branches' do + let(:source_branch) { 'branch' } + let(:target_branch) { 'branch' } + let(:source_repository_id) { 1 } + let(:target_repository_id) { 1 } + + it 'returns a generated source branch name' do + expect(subject.formatted_source_branch).to eq('branch-42') + end + end + end - expect(pr).to be_cross_project - end + describe '#truncated_title' do + context 'truncates the title to 255 characters' do + let(:title) { 'm' * 300 } - it 'returns true if no source repository is present' do - pr = described_class.new(target_repository_id: 2) + it { expect(subject.truncated_title.length).to eq(255) } + end - expect(pr).to be_cross_project - end - end + context 'does not truncate the title if it is shorter than 255 characters' do + let(:title) { 'foo' } - describe '#formatted_source_branch' do - context 'for a cross-project pull request' do - it 'includes the owner name in the branch name' do - pr = described_class.new( - source_repository_owner: 'foo', - source_branch: 'branch', - target_branch: 'master', - source_repository_id: 1, - target_repository_id: 2 - ) - - expect(pr.formatted_source_branch).to eq('github/fork/foo/branch') + it { expect(subject.truncated_title).to eq('foo') } + end end - end - context 'for a regular pull request' do - it 'returns the source branch name' do - pr = described_class.new( - source_repository_owner: 'foo', - source_branch: 'branch', - target_branch: 'master', - source_repository_id: 1, - target_repository_id: 1 - ) + describe '#author' do + let(:user_data) { nil } - expect(pr.formatted_source_branch).to eq('branch') + it 'does not convert the author if it was not specified' do + expect(subject.author).to be_nil + end end end + end - context 'for a pull request with the same source and target branches' do - it 'returns a generated source branch name' do - pr = described_class.new( - iid: 1, - source_repository_owner: 'foo', - source_branch: 'branch', - target_branch: 'branch', - source_repository_id: 1, - target_repository_id: 1 + describe '#parse' do + it_behaves_like 'a PullRequest' do + let(:response) do + double( + :response, + number: 42, + title: title, + body: 'This is my pull request', + state: state, + head: double( + :head, + sha: '123abc', + ref: source_branch, + repo: double(:repo, id: source_repository_id), + user: double(:user, id: 4, login: source_repository_owner) + ), + base: double( + :base, + sha: '456def', + ref: target_branch, + repo: double(:repo, id: target_repository_id) + ), + milestone: double(:milestone, number: 4), + user: user_data && double(:user, user_data), + assignee: double(:user, id: 4, login: 'alice'), + merged_by: double(:user, id: 4, login: 'alice'), + created_at: created_at, + updated_at: updated_at, + merged_at: merged_at ) - - expect(pr.formatted_source_branch).to eq('branch-1') end - end - end - - describe '#truncated_title' do - it 'truncates the title to 255 characters' do - object = described_class.new(title: 'm' * 300) - - expect(object.truncated_title.length).to eq(255) - end - it 'does not truncate the title if it is shorter than 255 characters' do - object = described_class.new(title: 'foo') - - expect(object.truncated_title).to eq('foo') + before do + subject.parse(response) + end end end - describe '#github_identifiers' do - it 'returns a hash with needed identifiers' do - github_identifiers = { - iid: 1 - } - other_attributes = { something_else: '_something_else_' } - pr = described_class.new(github_identifiers.merge(other_attributes)) + describe '#deserialize' do + it_behaves_like 'a PullRequest' do + let(:hash) do + { + 'iid' => 42, + 'title' => title, + 'description' => 'This is my pull request', + 'source_branch' => source_branch, + 'source_branch_sha' => '123abc', + 'target_branch' => target_branch, + 'target_branch_sha' => '456def', + 'source_repository_id' => source_repository_id, + 'target_repository_id' => target_repository_id, + 'source_repository_owner' => source_repository_owner, + 'state' => state, + 'milestone_number' => 4, + 'author' => user_data, + 'assignee' => { 'id' => 4, 'login' => 'alice' }, + 'created_at' => created_at.to_s, + 'updated_at' => updated_at.to_s, + 'merged_at' => merged_at.to_s + } + end - expect(pr.github_identifiers).to eq(github_identifiers.merge(issuable_type: 'MergeRequest')) + before do + subject.deserialize(hash) + end end end end diff --git a/spec/lib/gitlab/github_import/representation/user_spec.rb b/spec/lib/gitlab/github_import/representation/user_spec.rb index 14204886e9b98675fd5a9e65782e75ba1f634d50..8b73b7c39091cbf47eb5823a826451de67c21fbe 100644 --- a/spec/lib/gitlab/github_import/representation/user_spec.rb +++ b/spec/lib/gitlab/github_import/representation/user_spec.rb @@ -3,33 +3,46 @@ require 'spec_helper' RSpec.describe Gitlab::GithubImport::Representation::User do - shared_examples 'a User' do + let(:context) { Gitlab::GithubImport::Context.new(project: double, client: double) } + let(:user_data) { { 'id' => 42, 'login' => 'alice' } } + + subject(:representation) { described_class.new(context) } + + shared_examples 'a User Representation' do it 'returns an instance of User' do - expect(user).to be_an_instance_of(described_class) + expect(subject).to be_an_instance_of(described_class) end context 'the returned User' do it 'includes the user ID' do - expect(user.id).to eq(42) + expect(subject.id).to eq(42) end it 'includes the username' do - expect(user.login).to eq('alice') + expect(subject.login).to eq('alice') + end + + describe '#github_identifiers' do + it { expect(subject.github_identifiers).to eq(id: 42) } end end end - describe '.from_api_response' do - it_behaves_like 'a User' do - let(:response) { double(:response, id: 42, login: 'alice') } - let(:user) { described_class.from_api_response(response) } + describe '#parse' do + let(:response) { double(:response, user_data) } + + before do + subject.parse(response) end + + it_behaves_like 'a User Representation' end - describe '.from_json_hash' do - it_behaves_like 'a User' do - let(:hash) { { 'id' => 42, 'login' => 'alice' } } - let(:user) { described_class.from_json_hash(hash) } + describe '#deserialize' do + before do + subject.deserialize(user_data) end + + it_behaves_like 'a User Representation' end end