Remove commits/diff store login from MergeRequest model

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

Remove commits/diff store login from MergeRequest model
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Dmitriy Zaporozhets
1 parent d9be420e
Showing 2 changed files with 6 additions and 114 deletions Show diff stats
app/models/merge_request.rb
app/observers/merge_request_observer.rb
@@ -30,6 +30,9 @@ class MergeRequest &lt; ActiveRecord::Base
   belongs_to :target_project, foreign_key: :target_project_id, class_name: "Project"
   belongs_to :source_project, foreign_key: :source_project_id, class_name: "Project"
+  has_one :merge_request_diff, dependent: :destroy
+
+  delegate :commits, :diffs, :last_commit, :last_commit_short_sha, to: :merge_request_diff, prefix: nil
   attr_accessible :title, :assignee_id, :source_project_id, :source_branch, :target_project_id, :target_branch, :milestone_id, :author_id_of_changes, :state_event, :description
@@ -53,11 +56,8 @@ class MergeRequest &lt; ActiveRecord::Base
     end
     state :opened
-
     state :reopened
-
     state :closed
-
     state :merged
   end
@@ -75,15 +75,10 @@ class MergeRequest &lt; ActiveRecord::Base
     end
     state :unchecked
-
     state :can_be_merged
-
     state :cannot_be_merged
   end
-  serialize :st_commits
-  serialize :st_diffs
-
   validates :source_project, presence: true, unless: :allow_broken
   validates :source_branch, presence: true
   validates :target_project, presence: true
@@ -105,7 +100,7 @@ class MergeRequest &lt; ActiveRecord::Base
   scope :closed, -> { with_states(:closed, :merged) }
   def validate_branches
-    if target_project==source_project && target_branch == source_branch
+    if target_project == source_project && target_branch == source_branch
       errors.add :branch_conflict, "You can not use same project/branch for source and target"
     end
@@ -120,8 +115,7 @@ class MergeRequest &lt; ActiveRecord::Base
   end
   def reload_code
-    self.reloaded_commits
-    self.reloaded_diffs
+    merge_request_diff.reload_content if opened?
   end
   def check_if_can_be_merged
@@ -132,42 +126,6 @@ class MergeRequest &lt; ActiveRecord::Base
     end
   end
-  def diffs
-    @diffs ||= (load_diffs(st_diffs) || [])
-  end
-
-  def reloaded_diffs
-    if opened? && unmerged_diffs.any?
-      self.st_diffs = dump_diffs(unmerged_diffs)
-      self.save
-    end
-  end
-
-  def broken_diffs?
-    diffs == broken_diffs
-  rescue
-    true
-  end
-
-  def valid_diffs?
-    !broken_diffs?
-  end
-
-  def unmerged_diffs
-    diffs = if for_fork?
-              Gitlab::Satellite::MergeAction.new(author, self).diffs_between_satellite
-            else
-              Gitlab::Git::Diff.between(target_project.repository, source_branch, target_branch)
-            end
-
-    diffs ||= []
-    diffs
-  end
-
-  def last_commit
-    commits.first
-  end
-
   def merge_event
     self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::MERGED).last
   end
@@ -176,39 +134,6 @@ class MergeRequest &lt; ActiveRecord::Base
     self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last
   end
-  def commits
-    load_commits(st_commits || [])
-  end
-
-  def probably_merged?
-    unmerged_commits.empty? &&
-      commits.any? && opened?
-  end
-
-  def reloaded_commits
-    if opened? && unmerged_commits.any?
-      self.st_commits = dump_commits(unmerged_commits)
-      save
-
-    end
-    commits
-  end
-
-  def unmerged_commits
-    if for_fork?
-      commits = Gitlab::Satellite::MergeAction.new(self.author, self).commits_between
-    else
-      commits = target_project.repository.commits_between(self.target_branch, self.source_branch)
-    end
-
-    if commits.present?
-      commits = Commit.decorate(commits).
-      sort_by(&:created_at).
-      reverse
-    end
-    commits
-  end
-
   def merge!(user_id)
     self.author_id_of_changes = user_id
     self.merge
@@ -247,10 +172,6 @@ class MergeRequest &lt; ActiveRecord::Base
     Gitlab::Satellite::MergeAction.new(current_user, self).format_patch
   end
-  def last_commit_short_sha
-    @last_commit_short_sha ||= last_commit.sha[0..10]
-  end
-
   def for_fork?
     target_project != source_project
   end
@@ -327,34 +248,4 @@ class MergeRequest &lt; ActiveRecord::Base
     message << description.to_s
     message
   end
-
-  private
-
-  def dump_commits(commits)
-    commits.map(&:to_hash)
-  end
-
-  def load_commits(array)
-    array.map { |hash| Commit.new(Gitlab::Git::Commit.new(hash)) }
-  end
-
-  def dump_diffs(diffs)
-    if diffs == broken_diffs
-      broken_diffs
-    elsif diffs.respond_to?(:map)
-      diffs.map(&:to_hash)
-    end
-  end
-
-  def load_diffs(raw)
-    if raw == broken_diffs
-      broken_diffs
-    elsif raw.respond_to?(:map)
-      raw.map { |hash| Gitlab::Git::Diff.new(hash) }
-    end
-  end
-
-  def broken_diffs
-    [Gitlab::Git::Diff::BROKEN_DIFF]
-  end
 end
@@ -6,6 +6,7 @@ class MergeRequestObserver &lt; ActivityObserver
       create_event(merge_request, Event.determine_action(merge_request))
     end
+    merge_request.create_merge_request_diff
     notification.new_merge_request(merge_request, current_user)
     merge_request.create_cross_references!(merge_request.project, current_user)
     execute_hooks(merge_request)
	@@ -30,6 +30,9 @@ class MergeRequest < ActiveRecord::Base		@@ -30,6 +30,9 @@ class MergeRequest < ActiveRecord::Base
30		30
31	belongs_to :target_project, foreign_key: :target_project_id, class_name: "Project"	31	belongs_to :target_project, foreign_key: :target_project_id, class_name: "Project"
32	belongs_to :source_project, foreign_key: :source_project_id, class_name: "Project"	32	belongs_to :source_project, foreign_key: :source_project_id, class_name: "Project"
		33	+ has_one :merge_request_diff, dependent: :destroy
		34	+
		35	+ delegate :commits, :diffs, :last_commit, :last_commit_short_sha, to: :merge_request_diff, prefix: nil
33		36
34	attr_accessible :title, :assignee_id, :source_project_id, :source_branch, :target_project_id, :target_branch, :milestone_id, :author_id_of_changes, :state_event, :description	37	attr_accessible :title, :assignee_id, :source_project_id, :source_branch, :target_project_id, :target_branch, :milestone_id, :author_id_of_changes, :state_event, :description
35		38
	@@ -53,11 +56,8 @@ class MergeRequest < ActiveRecord::Base		@@ -53,11 +56,8 @@ class MergeRequest < ActiveRecord::Base
53	end	56	end
54		57
55	state :opened	58	state :opened
56	-
57	state :reopened	59	state :reopened
58	-
59	state :closed	60	state :closed
60	-
61	state :merged	61	state :merged
62	end	62	end
63		63
	@@ -75,15 +75,10 @@ class MergeRequest < ActiveRecord::Base		@@ -75,15 +75,10 @@ class MergeRequest < ActiveRecord::Base
75	end	75	end
76		76
77	state :unchecked	77	state :unchecked
78	-
79	state :can_be_merged	78	state :can_be_merged
80	-
81	state :cannot_be_merged	79	state :cannot_be_merged
82	end	80	end
83		81
84	- serialize :st_commits
85	- serialize :st_diffs
86	-
87	validates :source_project, presence: true, unless: :allow_broken	82	validates :source_project, presence: true, unless: :allow_broken
88	validates :source_branch, presence: true	83	validates :source_branch, presence: true
89	validates :target_project, presence: true	84	validates :target_project, presence: true
	@@ -105,7 +100,7 @@ class MergeRequest < ActiveRecord::Base		@@ -105,7 +100,7 @@ class MergeRequest < ActiveRecord::Base
105	scope :closed, -> { with_states(:closed, :merged) }	100	scope :closed, -> { with_states(:closed, :merged) }
106		101
107	def validate_branches	102	def validate_branches
108	- if target_project==source_project && target_branch == source_branch	103	+ if target_project == source_project && target_branch == source_branch
109	errors.add :branch_conflict, "You can not use same project/branch for source and target"	104	errors.add :branch_conflict, "You can not use same project/branch for source and target"
110	end	105	end
111		106
	@@ -120,8 +115,7 @@ class MergeRequest < ActiveRecord::Base		@@ -120,8 +115,7 @@ class MergeRequest < ActiveRecord::Base
120	end	115	end
121		116
122	def reload_code	117	def reload_code
123	- self.reloaded_commits
124	- self.reloaded_diffs	118	+ merge_request_diff.reload_content if opened?
125	end	119	end
126		120
127	def check_if_can_be_merged	121	def check_if_can_be_merged
	@@ -132,42 +126,6 @@ class MergeRequest < ActiveRecord::Base		@@ -132,42 +126,6 @@ class MergeRequest < ActiveRecord::Base
132	end	126	end
133	end	127	end
134		128
135	- def diffs
136	- @diffs \|\|= (load_diffs(st_diffs) \|\| [])
137	- end
138	-
139	- def reloaded_diffs
140	- if opened? && unmerged_diffs.any?
141	- self.st_diffs = dump_diffs(unmerged_diffs)
142	- self.save
143	- end
144	- end
145	-
146	- def broken_diffs?
147	- diffs == broken_diffs
148	- rescue
149	- true
150	- end
151	-
152	- def valid_diffs?
153	- !broken_diffs?
154	- end
155	-
156	- def unmerged_diffs
157	- diffs = if for_fork?
158	- Gitlab::Satellite::MergeAction.new(author, self).diffs_between_satellite
159	- else
160	- Gitlab::Git::Diff.between(target_project.repository, source_branch, target_branch)
161	- end
162	-
163	- diffs \|\|= []
164	- diffs
165	- end
166	-
167	- def last_commit
168	- commits.first
169	- end
170	-
171	def merge_event	129	def merge_event
172	self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::MERGED).last	130	self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::MERGED).last
173	end	131	end
	@@ -176,39 +134,6 @@ class MergeRequest < ActiveRecord::Base		@@ -176,39 +134,6 @@ class MergeRequest < ActiveRecord::Base
176	self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last	134	self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last
177	end	135	end
178		136
179	- def commits
180	- load_commits(st_commits \|\| [])
181	- end
182	-
183	- def probably_merged?
184	- unmerged_commits.empty? &&
185	- commits.any? && opened?
186	- end
187	-
188	- def reloaded_commits
189	- if opened? && unmerged_commits.any?
190	- self.st_commits = dump_commits(unmerged_commits)
191	- save
192	-
193	- end
194	- commits
195	- end
196	-
197	- def unmerged_commits
198	- if for_fork?
199	- commits = Gitlab::Satellite::MergeAction.new(self.author, self).commits_between
200	- else
201	- commits = target_project.repository.commits_between(self.target_branch, self.source_branch)
202	- end
203	-
204	- if commits.present?
205	- commits = Commit.decorate(commits).
206	- sort_by(&:created_at).
207	- reverse
208	- end
209	- commits
210	- end
211	-
212	def merge!(user_id)	137	def merge!(user_id)
213	self.author_id_of_changes = user_id	138	self.author_id_of_changes = user_id
214	self.merge	139	self.merge
	@@ -247,10 +172,6 @@ class MergeRequest < ActiveRecord::Base		@@ -247,10 +172,6 @@ class MergeRequest < ActiveRecord::Base
247	Gitlab::Satellite::MergeAction.new(current_user, self).format_patch	172	Gitlab::Satellite::MergeAction.new(current_user, self).format_patch
248	end	173	end
249		174
250	- def last_commit_short_sha
251	- @last_commit_short_sha \|\|= last_commit.sha[0..10]
252	- end
253	-
254	def for_fork?	175	def for_fork?
255	target_project != source_project	176	target_project != source_project
256	end	177	end
	@@ -327,34 +248,4 @@ class MergeRequest < ActiveRecord::Base		@@ -327,34 +248,4 @@ class MergeRequest < ActiveRecord::Base
327	message << description.to_s	248	message << description.to_s
328	message	249	message
329	end	250	end
330	-
331	- private
332	-
333	- def dump_commits(commits)
334	- commits.map(&:to_hash)
335	- end
336	-
337	- def load_commits(array)
338	- array.map { \|hash\| Commit.new(Gitlab::Git::Commit.new(hash)) }
339	- end
340	-
341	- def dump_diffs(diffs)
342	- if diffs == broken_diffs
343	- broken_diffs
344	- elsif diffs.respond_to?(:map)
345	- diffs.map(&:to_hash)
346	- end
347	- end
348	-
349	- def load_diffs(raw)
350	- if raw == broken_diffs
351	- broken_diffs
352	- elsif raw.respond_to?(:map)
353	- raw.map { \|hash\| Gitlab::Git::Diff.new(hash) }
354	- end
355	- end
356	-
357	- def broken_diffs
358	- [Gitlab::Git::Diff::BROKEN_DIFF]
359	- end
360	end	251	end
	@@ -6,6 +6,7 @@ class MergeRequestObserver < ActivityObserver		@@ -6,6 +6,7 @@ class MergeRequestObserver < ActivityObserver
6	create_event(merge_request, Event.determine_action(merge_request))	6	create_event(merge_request, Event.determine_action(merge_request))
7	end	7	end
8		8
		9	+ merge_request.create_merge_request_diff
9	notification.new_merge_request(merge_request, current_user)	10	notification.new_merge_request(merge_request, current_user)
10	merge_request.create_cross_references!(merge_request.project, current_user)	11	merge_request.create_cross_references!(merge_request.project, current_user)
11	execute_hooks(merge_request)	12	execute_hooks(merge_request)