fixes a bug that users can agree to issues several times.

* Issue

Users can agree to issues many times if the user clicks agree button very fast. Because there is no primary key on issue_voter table and logics checking voters duplicaiton.

* Solution

The primary key is added to issue_voter table.
And to prevent vote duplication, protect codes are added.

Altough issue_comment_voter table has a primary key, there are possibilities that primary key violation exception occur. So protect codes are added to issue_comment_voter table as issue_voter table.

Private-issue: 2060

app/controllers/VoteApp.java

@@ -33,6 +33,7 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
 
 /**
  * The Controller which plays a role in voting in the issue.
@@ -117,22 +118,17 @@ public static Result unvoteComment(String user, String project, Long number, Lon
         return redirect(RouteUtil.getUrl(issueComment));
     }
 
-    public static List<User> getVotersForAvatar(List<User> voters, int size){
+    public static List<User> getVotersForAvatar(Set<User> voters, int size){
         return getSubList(voters, 0, size);
     }
 
-    public static List<User> getVotersForName(List<User> voters, int fromIndex, int size){
+    public static List<User> getVotersForName(Set<User> voters, int fromIndex, int size){
         return getSubList(voters, fromIndex, fromIndex + size);
     }
 
-    public static List<User> getVotersExceptCurrentUser(List<User> voters){
-        List<User> result = new ArrayList<User>(voters);
-
-        while(result.contains(UserApp.currentUser())){
-            result.remove(UserApp.currentUser());
-        }
-
-        return result;
+    public static Set<User> getVotersExceptCurrentUser(Set<User> voters){
+        voters.remove(UserApp.currentUser());
+        return voters;
     }
 
     /**
@@ -142,9 +138,9 @@ public static List<User> getVotersExceptCurrentUser(List<User> voters){
      * @param toIndex
      * @return
      */
-    private static List<User> getSubList(List<User> voters, int fromIndex, int toIndex) {
+    private static List<User> getSubList(Set<User> voters, int fromIndex, int toIndex) {
         try {
-            return voters.subList(
+            return new ArrayList<>(voters).subList(
                     Math.max(0, fromIndex),
                     Math.min(voters.size(), toIndex)
             );

app/models/Issue.java

@@ -95,7 +95,7 @@ public class Issue extends AbstractPosting implements LabelOwner {
             joinColumns = @JoinColumn(name = "issue_id"),
             inverseJoinColumns = @JoinColumn(name = "user_id")
     )
-    public List<User> voters = new ArrayList<>();
+    public Set<User> voters = new HashSet<>();
 
     @Transient
     @Formula(select = "case when due_date is null then cast('0001-01-01 00:00:00' as timestamp) else due_date end")
@@ -470,8 +470,9 @@ public boolean canBeDeleted() {
      * @param user
      */
     public void addVoter(User user) {
-        this.voters.add(user);
-        this.update();
+        if (voters.add(user)) {
+            update();
+        }
     }
 
     /**
@@ -480,8 +481,9 @@ public void addVoter(User user) {
      * @param user
      */
     public void removeVoter(User user) {
-        this.voters.remove(user);
-        this.update();
+        if (voters.remove(user)) {
+            update();
+        }
     }
 
     /**

app/models/IssueComment.java

@@ -24,7 +24,8 @@
 import models.resource.Resource;
 
 import javax.persistence.*;
-import java.util.List;
+import java.util.HashSet;
+import java.util.Set;
 
 @Entity
 public class IssueComment extends Comment {
@@ -40,7 +41,7 @@ public class IssueComment extends Comment {
             joinColumns = @JoinColumn(name = "issue_comment_id"),
             inverseJoinColumns = @JoinColumn(name = "user_id")
     )
-    public List<User> voters;
+    public Set<User> voters = new HashSet<>();
 
     public IssueComment(Issue issue, User author, String contents) {
         super(author, contents);
@@ -88,16 +89,14 @@ public Resource getContainer() {
     }
 
     public void addVoter(User user) {
-        if (!this.voters.contains(user)) {
-            this.voters.add(user);
-            this.update();
+        if (voters.add(user)) {
+            update();
         }
     }
 
     public void removeVoter(User user) {
-        if (this.voters.contains(user)) {
-            this.voters.remove(user);
-            this.update();
+        if (voters.remove(user)) {
+            update();
         }
     }
 }

conf/evolutions/default/97.sql

@@ -0,0 +1,18 @@
+# --- !Ups
+
+create table issue_voter_temp (
+  issue_id bigint not null,
+  user_id  bigint not null
+);
+
+insert into issue_voter_temp select * from issue_voter group by issue_id, user_id;
+drop table issue_voter;
+alter table issue_voter_temp rename to issue_voter;
+
+alter table issue_voter add constraint pk_issue_voter primary key (issue_id, user_id);
+alter table issue_voter add constraint fk_issue_voter_issue_1 foreign key (issue_id) references issue (id) on delete restrict on update restrict;
+alter table issue_voter add constraint fk_issue_voter_n4user_2 foreign key (user_id) references n4user (id) on delete restrict on update restrict;
+
+# --- !Downs
+
+alter table issue_voter drop constraint if exists pk_issue_voter;