Commit @f3e011267b2a21a4be6dd06c1258d11ffc532345

[Notice] Announcing the End of Demo Server [Read me]

Yi EungJun 2014-05-19

issue: Fix an exception while posting many issues

Reason: Every issue posting requires updating of project.lastIssueNumber
and it may occur OptimisticLockexception if the project.lastIssueNumber
has been changed already by other issue posting.

Solution: In project.increaseLastIssueNumber(), do not update self but
the project just fetched from database by Project.find.byId()

To make the solution works correctly, I did following works as well:

1. Add getter or setter to access the lastIssueNumber field. The
   increaseLastIssueNumber method cannot update lastIssueNumber directly
   because Ebean does not set the project to dirty even if the method
   modified the field.  When a private field is accessed directly
   (Ebean's "Field Access") in a static method or the other instance's
   method, Ebean does not call preSetter and/or preGetter so the
   instance will not be set to dirty.  I guess it is a bug of Ebean.

2. Refresh project before updating it, as the fixing of
   test/controllers/IssueAppTest.java in this commit. Posting an issue
   makes every project instance to have smaller lastIssueNumber than the
   actual value stored in database.

3. Make project.increaseLastIssueNumber static. It looks unnatural that
   non-static project.increaseLastIsseNumber method does not increase
   lastIssueNumber field of self.

4. Apply these fixes to Posting too.

@f3e011267b2a21a4be6dd06c1258d11ffc532345

fc78a78

f3e0112

app/models/Issue.java

--- app/models/Issue.java

+++ app/models/Issue.java


      */
     @Override
     protected Long increaseNumber() {
-        return project.increaseLastIssueNumber();
+        return Project.increaseLastIssueNumber(project.id);
     }
 
     protected void fixLastNumber() {
-        project.fixLastIssueNumber();
+        Project.fixLastIssueNumber(project.id);
     }
 
     public String assigneeName() {

fc78a78

f3e0112

app/models/Posting.java

--- app/models/Posting.java

+++ app/models/Posting.java


      */
     @Override
     protected Long increaseNumber() {
-        return project.increaseLastPostingNumber();
+        return Project.increaseLastPostingNumber(project.id);
     }
 
     protected void fixLastNumber() {
-        project.fixLastPostingNumber();
+        Project.fixLastPostingNumber(project.id);
     }
 
     /**

fc78a78

f3e0112

app/models/Project.java

--- app/models/Project.java

+++ app/models/Project.java


         return null;
     }
 
-    @Transactional
-    public Long increaseLastIssueNumber() {
-        if (this.issues != null && lastIssueNumber < this.issues.size() ){
-            fixLastIssueNumber();
-        }
-
-        lastIssueNumber++;
-        update();
+    private Long getLastIssueNumber() {
         return lastIssueNumber;
     }
 
-    public void fixLastIssueNumber() {
-        Issue issue = Issue.finder.where().eq("project.id", id).order().desc("number").findList().get(0);
-        issue.refresh();
-        if (issue.number == null) {
-            lastIssueNumber = 0;
-        } else {
-            lastIssueNumber = issue.number;
-        }
+    private void setLastIssueNumber(Long number) {
+        lastIssueNumber = number;
     }
 
-    @Transactional
-    public Long increaseLastPostingNumber() {
-        if (this.posts != null && lastPostingNumber < this.posts.size() ){
-            fixLastPostingNumber();
-        }
-        lastPostingNumber++;
-        update();
+    private Long getLastPostingNumber() {
         return lastPostingNumber;
     }
 
-    public void fixLastPostingNumber() {
-        Posting posting = Posting.finder.where().eq("project.id", id).order().desc("number").findList().get(0);
-        posting.refresh();
-        if (posting.number == null) {
-            lastPostingNumber = 0;
-        } else {
-            lastPostingNumber = posting.number;
+    private void setLastPostingNumber(Long number) {
+        lastPostingNumber = number;
+    }
+
+    /**
+     * 마지막 이슈번호를 증가시킨다.
+     *
+     * 이슈 추가시 사용한다.
+     *
+     * @return {@code lastIssueNumber}
+     */
+    public static Long increaseLastIssueNumber(Long projectId) {
+        Project project = find.byId(projectId);
+
+        if (project.issues != null && project.lastIssueNumber < project.issues.size() ){
+            fixLastIssueNumber(projectId);
         }
+
+        project.setLastIssueNumber(project.getLastIssueNumber() + 1);
+        project.update();
+
+        return project.lastIssueNumber;
+    }
+
+    public static void fixLastIssueNumber(Long projectId) {
+        Project project = find.byId(projectId);
+        project.refresh();
+
+        Issue issue = Issue.finder.where().eq("project.id", projectId).order().desc("number").findList().get(0);
+        issue.refresh();
+
+        if (issue.number == null) {
+            project.lastIssueNumber = 0;
+        } else {
+            project.lastIssueNumber = issue.number;
+        }
+
+        project.update();
+    }
+
+    public static Long increaseLastPostingNumber(Long projectId) {
+        Project project = find.byId(projectId);
+
+        if (project.posts != null && project.lastPostingNumber < project.posts.size() ){
+            fixLastPostingNumber(projectId);
+        }
+
+        project.setLastPostingNumber(project.getLastPostingNumber() + 1);
+        project.update();
+
+        return project.lastPostingNumber;
+    }
+
+    public static void fixLastPostingNumber(Long projectId) {
+        Project project = find.byId(projectId);
+        project.refresh();
+
+        Posting posting = Posting.finder.where().eq("project.id", projectId).order().desc("number").findList().get(0);
+        posting.refresh();
+
+        if (posting.number == null) {
+            project.lastPostingNumber = 0;
+        } else {
+            project.lastPostingNumber = posting.number;
+        }
+
+        project.update();
     }
 
     public Resource labelsAsResource() {

fc78a78

f3e0112

test/controllers/IssueAppTest.java

--- test/controllers/IssueAppTest.java

+++ test/controllers/IssueAppTest.java


     @Test
     public void editByNonmember() {
         // Given
+        project.refresh();
         project.setProjectScope(ProjectScope.PUBLIC);
         project.update();
 

     @Test
     public void deleteByNonmember() {
         // Given
+        project.refresh();
         project.setProjectScope(ProjectScope.PUBLIC);
         project.update();
 

     @Test
     public void postByAnonymous() {
         // Given
+        project.refresh();
         project.setProjectScope(ProjectScope.PUBLIC);
         project.update();
 

     @Test
     public void postByNonmember() {
         // Given
+        project.refresh();
         project.setProjectScope(ProjectScope.PUBLIC);
         project.update();
 

     @Test
     public void commentByAnonymous() {
         // Given
+        project.refresh();
         project.setProjectScope(ProjectScope.PUBLIC);
         project.update();
 

     @Test
     public void commentByNonmember() {
         // Given
+        project.refresh();
         project.setProjectScope(ProjectScope.PUBLIC);
         project.update();
 

     public void watch() {
         // Given
         Resource resource = issue.asResource();
+        project.refresh();
         project.setProjectScope(ProjectScope.PUBLIC);
         project.update();
 

     public void unwatch() {
         // Given
         Resource resource = issue.asResource();
+        project.refresh();
         project.setProjectScope(ProjectScope.PUBLIC);
         project.update();
 

fc78a78

f3e0112

test/models/IssueTest.java

--- test/models/IssueTest.java

+++ test/models/IssueTest.java

...	...	@@ -236,4 +236,20 @@
236	236	event.created = DateUtils.parseDate(str, "yyyy-MM-dd");
237	237	return event;
238	238	}
	239	+
	240	+ @Test
	241	+ public void optimisticLockException() {
	242	+ Project project1 = Project.findByOwnerAndProjectName("yobi", "projectYobi");
	243	+ Project project2 = Project.findByOwnerAndProjectName("yobi", "projectYobi");
	244	+
	245	+ issue = new Issue();
	246	+ issue.setProject(project1);
	247	+ issue.setTitle("a");
	248	+ issue.save();
	249	+
	250	+ issue = new Issue();
	251	+ issue.setProject(project2);
	252	+ issue.setTitle("b");
	253	+ issue.save();
	254	+ }
239	255	}

Add a comment

Open 0
Closed 0

List

...	...	@@ -103,11 +103,11 @@
103	103	*/
104	104	@Override
105	105	protected Long increaseNumber() {
106		- return project.increaseLastIssueNumber();
	106	+ return Project.increaseLastIssueNumber(project.id);
107	107	}
108	108
109	109	protected void fixLastNumber() {
110		- project.fixLastIssueNumber();
	110	+ Project.fixLastIssueNumber(project.id);
111	111	}
112	112
113	113	public String assigneeName() {

...	...	@@ -305,45 +305,85 @@
305	305	return null;
306	306	}
307	307
308		- @Transactional
309		- public Long increaseLastIssueNumber() {
310		- if (this.issues != null && lastIssueNumber < this.issues.size() ){
311		- fixLastIssueNumber();
312		- }
313		-
314		- lastIssueNumber++;
315		- update();
	308	+ private Long getLastIssueNumber() {
316	309	return lastIssueNumber;
317	310	}
318	311
319		- public void fixLastIssueNumber() {
320		- Issue issue = Issue.finder.where().eq("project.id", id).order().desc("number").findList().get(0);
321		- issue.refresh();
322		- if (issue.number == null) {
323		- lastIssueNumber = 0;
324		- } else {
325		- lastIssueNumber = issue.number;
326		- }
	312	+ private void setLastIssueNumber(Long number) {
	313	+ lastIssueNumber = number;
327	314	}
328	315
329		- @Transactional
330		- public Long increaseLastPostingNumber() {
331		- if (this.posts != null && lastPostingNumber < this.posts.size() ){
332		- fixLastPostingNumber();
333		- }
334		- lastPostingNumber++;
335		- update();
	316	+ private Long getLastPostingNumber() {
336	317	return lastPostingNumber;
337	318	}
338	319
339		- public void fixLastPostingNumber() {
340		- Posting posting = Posting.finder.where().eq("project.id", id).order().desc("number").findList().get(0);
341		- posting.refresh();
342		- if (posting.number == null) {
343		- lastPostingNumber = 0;
344		- } else {
345		- lastPostingNumber = posting.number;
	320	+ private void setLastPostingNumber(Long number) {
	321	+ lastPostingNumber = number;
	322	+ }
	323	+
	324	+ /**
	325	+ * 마지막 이슈번호를 증가시킨다.
	326	+ *
	327	+ * 이슈 추가시 사용한다.
	328	+ *
	329	+ * @return {@code lastIssueNumber}
	330	+ */
	331	+ public static Long increaseLastIssueNumber(Long projectId) {
	332	+ Project project = find.byId(projectId);
	333	+
	334	+ if (project.issues != null && project.lastIssueNumber < project.issues.size() ){
	335	+ fixLastIssueNumber(projectId);
346	336	}
	337	+
	338	+ project.setLastIssueNumber(project.getLastIssueNumber() + 1);
	339	+ project.update();
	340	+
	341	+ return project.lastIssueNumber;
	342	+ }
	343	+
	344	+ public static void fixLastIssueNumber(Long projectId) {
	345	+ Project project = find.byId(projectId);
	346	+ project.refresh();
	347	+
	348	+ Issue issue = Issue.finder.where().eq("project.id", projectId).order().desc("number").findList().get(0);
	349	+ issue.refresh();
	350	+
	351	+ if (issue.number == null) {
	352	+ project.lastIssueNumber = 0;
	353	+ } else {
	354	+ project.lastIssueNumber = issue.number;
	355	+ }
	356	+
	357	+ project.update();
	358	+ }
	359	+
	360	+ public static Long increaseLastPostingNumber(Long projectId) {
	361	+ Project project = find.byId(projectId);
	362	+
	363	+ if (project.posts != null && project.lastPostingNumber < project.posts.size() ){
	364	+ fixLastPostingNumber(projectId);
	365	+ }
	366	+
	367	+ project.setLastPostingNumber(project.getLastPostingNumber() + 1);
	368	+ project.update();
	369	+
	370	+ return project.lastPostingNumber;
	371	+ }
	372	+
	373	+ public static void fixLastPostingNumber(Long projectId) {
	374	+ Project project = find.byId(projectId);
	375	+ project.refresh();
	376	+
	377	+ Posting posting = Posting.finder.where().eq("project.id", projectId).order().desc("number").findList().get(0);
	378	+ posting.refresh();
	379	+
	380	+ if (posting.number == null) {
	381	+ project.lastPostingNumber = 0;
	382	+ } else {
	383	+ project.lastPostingNumber = posting.number;
	384	+ }
	385	+
	386	+ project.update();
347	387	}
348	388
349	389	public Resource labelsAsResource() {

...	...	@@ -156,6 +156,7 @@
156	156	@Test
157	157	public void editByNonmember() {
158	158	// Given
	159	+ project.refresh();
159	160	project.setProjectScope(ProjectScope.PUBLIC);
160	161	project.update();
161	162
...	...	@@ -215,6 +216,7 @@
215	216	@Test
216	217	public void deleteByNonmember() {
217	218	// Given
	219	+ project.refresh();
218	220	project.setProjectScope(ProjectScope.PUBLIC);
219	221	project.update();
220	222
...	...	@@ -274,6 +276,7 @@
274	276	@Test
275	277	public void postByAnonymous() {
276	278	// Given
	279	+ project.refresh();
277	280	project.setProjectScope(ProjectScope.PUBLIC);
278	281	project.update();
279	282
...	...	@@ -287,6 +290,7 @@
287	290	@Test
288	291	public void postByNonmember() {
289	292	// Given
	293	+ project.refresh();
290	294	project.setProjectScope(ProjectScope.PUBLIC);
291	295	project.update();
292	296
...	...	@@ -327,6 +331,7 @@
327	331	@Test
328	332	public void commentByAnonymous() {
329	333	// Given
	334	+ project.refresh();
330	335	project.setProjectScope(ProjectScope.PUBLIC);
331	336	project.update();
332	337
...	...	@@ -340,6 +345,7 @@
340	345	@Test
341	346	public void commentByNonmember() {
342	347	// Given
	348	+ project.refresh();
343	349	project.setProjectScope(ProjectScope.PUBLIC);
344	350	project.update();
345	351
...	...	@@ -388,6 +394,7 @@
388	394	public void watch() {
389	395	// Given
390	396	Resource resource = issue.asResource();
	397	+ project.refresh();
391	398	project.setProjectScope(ProjectScope.PUBLIC);
392	399	project.update();
393	400
...	...	@@ -443,6 +450,7 @@
443	450	public void unwatch() {
444	451	// Given
445	452	Resource resource = issue.asResource();
	453	+ project.refresh();
446	454	project.setProjectScope(ProjectScope.PUBLIC);
447	455	project.update();
448	456

...	...	@@ -31,11 +31,11 @@
31	31	*/
32	32	@Override
33	33	protected Long increaseNumber() {
34		- return project.increaseLastPostingNumber();
	34	+ return Project.increaseLastPostingNumber(project.id);
35	35	}
36	36
37	37	protected void fixLastNumber() {
38		- project.fixLastPostingNumber();
	38	+ Project.fixLastPostingNumber(project.id);
39	39	}
40	40
41	41	/**

Delete comment