diff --git a/commons.util/src/main/java/com/pmease/commons/util/namedentity/EntityPatternSet.java b/commons.util/src/main/java/com/pmease/commons/util/namedentity/EntityPatternSet.java index f683b3b9fd..80eb81c1fb 100644 --- a/commons.util/src/main/java/com/pmease/commons/util/namedentity/EntityPatternSet.java +++ b/commons.util/src/main/java/com/pmease/commons/util/namedentity/EntityPatternSet.java @@ -64,6 +64,11 @@ public class EntityPatternSet extends PatternSet implements Trimmable { return input; } + @Override + public String toString() { + return new PatternSet(asInput()).toString(); + } + public static EntityPatternSet fromInput(List input, EntityLoader entityLoader) { List stored = new ArrayList(); for (ExclusiveAwarePattern eachInput: input) { diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/CorePlugin.java b/gitop.core/src/main/java/com/pmease/gitop/core/CorePlugin.java index 8318956f31..3b674a4424 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/CorePlugin.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/CorePlugin.java @@ -15,6 +15,7 @@ import com.pmease.commons.jetty.extensionpoints.ServletContextConfigurator; import com.pmease.commons.loader.AbstractPlugin; import com.pmease.commons.persistence.AbstractEntity; import com.pmease.commons.persistence.extensionpoints.ModelContribution; +import com.pmease.commons.util.ClassUtils; import com.pmease.gitop.core.model.User; import com.pmease.gitop.core.web.asset.AssetLocator; @@ -42,7 +43,7 @@ public class CorePlugin extends AbstractPlugin { public Collection> getModelClasses() { Collection> modelClasses = new HashSet>(); - modelClasses.add(User.class); + modelClasses.addAll(ClassUtils.findSubClasses(AbstractEntity.class, User.class)); return modelClasses; } }); diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/AbstractGateKeeper.java b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/AbstractGateKeeper.java index cb4e9c1171..6a4bb3ae14 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/AbstractGateKeeper.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/AbstractGateKeeper.java @@ -1,5 +1,7 @@ package com.pmease.gitop.core.gatekeeper; +import java.util.List; + import javax.annotation.Nullable; public abstract class AbstractGateKeeper implements GateKeeper { @@ -9,4 +11,36 @@ public abstract class AbstractGateKeeper implements GateKeeper { return this; } + protected CheckResult accept(String reason) { + return new CheckResult.Accept(reason); + } + + protected CheckResult reject(String reason) { + return new CheckResult.Reject(reason); + } + + protected CheckResult pending(String reason) { + return new CheckResult.Pending(reason); + } + + protected CheckResult block(String reason) { + return new CheckResult.Block(reason); + } + + protected CheckResult accept(List reasons) { + return new CheckResult.Accept(reasons); + } + + protected CheckResult reject(List reasons) { + return new CheckResult.Reject(reasons); + } + + protected CheckResult pending(List reasons) { + return new CheckResult.Pending(reasons); + } + + protected CheckResult block(List reasons) { + return new CheckResult.Block(reasons); + } + } diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/AlwaysAccept.java b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/AlwaysAccept.java index 3cb7890b35..02867d4a8d 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/AlwaysAccept.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/AlwaysAccept.java @@ -6,7 +6,7 @@ public class AlwaysAccept extends AbstractGateKeeper { @Override public CheckResult check(MergeRequest request) { - return CheckResult.ACCEPT; + return accept("always"); } } diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/AndGateKeeper.java b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/AndGateKeeper.java index aaa8fe76f2..40fe544f61 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/AndGateKeeper.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/AndGateKeeper.java @@ -7,7 +7,7 @@ import com.pmease.commons.util.trimmable.AndOrConstruct; import com.pmease.commons.util.trimmable.TrimUtils; import com.pmease.gitop.core.model.MergeRequest; -public class AndGateKeeper implements GateKeeper { +public class AndGateKeeper extends AbstractGateKeeper { private List gateKeepers = new ArrayList(); @@ -21,20 +21,27 @@ public class AndGateKeeper implements GateKeeper { @Override public CheckResult check(MergeRequest request) { - boolean pending = false; + List pendingReasons = new ArrayList(); + List acceptReasons = new ArrayList(); for (GateKeeper each: getGateKeepers()) { CheckResult result = each.check(request); - if (result == CheckResult.REJECT || result == CheckResult.PENDING_BLOCK) + if (result.isAccept()) { + acceptReasons.addAll(result.getReasons()); + } else if (result.isReject()) { return result; - else if (result == CheckResult.PENDING) - pending = true; + } else if (result.isBlock()) { + result.getReasons().addAll(pendingReasons); + return result; + } else { + pendingReasons.addAll(result.getReasons()); + } } - if (pending) - return CheckResult.PENDING; + if (!pendingReasons.isEmpty()) + return pending(pendingReasons); else - return CheckResult.ACCEPT; + return accept(acceptReasons); } @Override diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/ApprovedByAuthorizedUsers.java b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/ApprovedByAuthorizedUsers.java index cacc702890..15c1b68638 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/ApprovedByAuthorizedUsers.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/ApprovedByAuthorizedUsers.java @@ -20,8 +20,14 @@ public class ApprovedByAuthorizedUsers extends AbstractGateKeeper { CheckResult result = or.check(request); - if (result.isPending()) { - request.requestVote(authorizedUsers); + if (result.isAccept()) { + result = accept("Approved by user with push permission."); + } else if (result.isReject()) { + result = reject("Not approved by any users with push permission."); + } else if (result.isPending()) { + result = pending("To be approved by someone with push permission."); + } else { + result = block("To be approved by someone with push permission."); } return result; } diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/ApprovedByMajoritiesOfSpecifiedTeam.java b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/ApprovedByMajoritiesOfSpecifiedTeam.java index 2822495693..9d993bf8ad 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/ApprovedByMajoritiesOfSpecifiedTeam.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/ApprovedByMajoritiesOfSpecifiedTeam.java @@ -11,7 +11,14 @@ public class ApprovedByMajoritiesOfSpecifiedTeam extends TeamAwareGateKeeper { gateKeeper.setRequireVoteOfAllMembers(true); gateKeeper.setTeamId(getTeamId()); - return gateKeeper.check(request); + CheckResult result = gateKeeper.check(request); + + if (result.isAccept()) + result = accept("Approved by majorities of team '" + getTeam().getName() + "'."); + else if (result.isReject()) + result = reject("Not approved by majorities of team '" + getTeam().getName() + "'."); + + return result; } } diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/ApprovedByRepositoryOwner.java b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/ApprovedByRepositoryOwner.java index a0c2580e56..96c5e7484a 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/ApprovedByRepositoryOwner.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/ApprovedByRepositoryOwner.java @@ -3,6 +3,7 @@ package com.pmease.gitop.core.gatekeeper; import com.pmease.commons.util.EasySet; import com.pmease.gitop.core.model.MergeRequest; import com.pmease.gitop.core.model.User; +import com.pmease.gitop.core.model.Vote; public class ApprovedByRepositoryOwner extends AbstractGateKeeper { @@ -10,10 +11,16 @@ public class ApprovedByRepositoryOwner extends AbstractGateKeeper { public CheckResult check(MergeRequest request) { User repositoryOwner = request.getDestination().getRepository().getOwner(); - CheckResult result = repositoryOwner.checkApprovalSince(request.getBaseUpdate()); - if (result.isPending()) - request.requestVote(EasySet.of(repositoryOwner)); - return result; + Vote.Result result = repositoryOwner.checkVoteSince(request.getBaseUpdate()); + + if (result == null) { + request.inviteToVote(EasySet.of(repositoryOwner), 1); + return pending("To be approved by user '" + repositoryOwner.getName() + "'."); + } else if (result.isAccept()) { + return accept("Approved by user '" + repositoryOwner.getName() + "'."); + } else { + return reject("Rejected by user '" + repositoryOwner.getName() + "'."); + } } } diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/ApprovedBySpecifiedTeam.java b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/ApprovedBySpecifiedTeam.java index 60422ec957..02cde6ae21 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/ApprovedBySpecifiedTeam.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/ApprovedBySpecifiedTeam.java @@ -5,12 +5,10 @@ import java.util.HashSet; import javax.validation.constraints.Min; -import com.pmease.commons.loader.AppLoader; -import com.pmease.gitop.core.manager.TeamManager; import com.pmease.gitop.core.model.MergeRequest; -import com.pmease.gitop.core.model.Team; import com.pmease.gitop.core.model.TeamMembership; import com.pmease.gitop.core.model.User; +import com.pmease.gitop.core.model.Vote; public class ApprovedBySpecifiedTeam extends TeamAwareGateKeeper { @@ -27,41 +25,31 @@ public class ApprovedBySpecifiedTeam extends TeamAwareGateKeeper { @Override public CheckResult check(MergeRequest request) { - TeamManager teamManager = AppLoader.getInstance(TeamManager.class); - Team team = teamManager.load(getTeamId()); - Collection members = new HashSet(); - for (TeamMembership membership: team.getMemberships()) + for (TeamMembership membership: getTeam().getMemberships()) members.add(membership.getUser()); - boolean pendingBlock = false; - int approvals = 0; int pendings = 0; for (User member: members) { - CheckResult result = member.checkApprovalSince(request.getBaseUpdate()); - if (result == CheckResult.ACCEPT) { + Vote.Result result = member.checkVoteSince(request.getBaseUpdate()); + if (result == null) { + pendings++; + } else if (result.isAccept()) { approvals++; - } else if (result == CheckResult.PENDING_BLOCK) { - pendingBlock = true; - pendings++; - } else if (result == CheckResult.PENDING) { - pendings++; } } if (approvals >= getLeastApprovals()) { - return CheckResult.ACCEPT; + return accept("Get at least " + getLeastApprovals() + " approvals from team '" + getTeam().getName() + "'."); } else if (getLeastApprovals() - approvals > pendings) { - return CheckResult.REJECT; + return reject("Can not get at least " + getLeastApprovals() + " approvals from team '" + getTeam().getName() + "'."); } else { - for (int i=0; i reasons; + + public CheckResult(List reasons) { + this.reasons = reasons; + } + + public CheckResult(String reason) { + this.reasons = EasyList.of(reason); + } + + public List getReasons() { + return reasons; + } + + public abstract boolean isAccept(); + + public abstract boolean isReject(); + + public abstract boolean isPending(); + + public abstract boolean isBlock(); + + /* accept the merge request. */ + public static class Accept extends CheckResult { + + public Accept(List reasons) { + super(reasons); + } + + public Accept(String reason) { + super(reason); + } + + @Override + public boolean isPending() { + return false; + } + + @Override + public boolean isAccept() { + return true; + } + + @Override + public boolean isReject() { + return false; + } + + @Override + public boolean isBlock() { + return false; + } + + }; + + /* reject the merge request. */ + public static class Reject extends CheckResult { + + public Reject(List reasons) { + super(reasons); + } + + public Reject(String reason) { + super(reason); + } + + @Override + public boolean isAccept() { + return false; + } + + @Override + public boolean isReject() { + return true; + } + + @Override + public boolean isPending() { + return false; + } + + @Override + public boolean isBlock() { + return false; + } + + }; + + /* merge request acceptance check is pending and result is unknown yet */ + public static class Pending extends CheckResult { + + public Pending(List reasons) { + super(reasons); + } + + public Pending(String reason) { + super(reason); + } + + @Override + public boolean isAccept() { + return false; + } + + @Override + public boolean isReject() { + return false; + } + + @Override + public boolean isPending() { + return true; + } + + @Override + public boolean isBlock() { + return false; + } + }; + + /* + * same as Pending, but followed gate keeper should not be checked unless result + * of this gate keeper has been determined. + */ + public static class Block extends CheckResult { + + public Block(List reasons) { + super(reasons); + } + + public Block(String reason) { + super(reason); + } + + @Override + public boolean isPending() { + return false; + } + + @Override + public boolean isAccept() { + return false; + } + + @Override + public boolean isReject() { + return false; + } + + @Override + public boolean isBlock() { + return true; + } + }; + +} \ No newline at end of file diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/GateKeeper.java b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/GateKeeper.java index 90a6143eaa..dd930333fe 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/GateKeeper.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/GateKeeper.java @@ -5,45 +5,5 @@ import com.pmease.gitop.core.model.MergeRequest; public interface GateKeeper extends Trimmable { - public enum CheckResult { - /* accept the merge request. */ - ACCEPT { - @Override - public boolean isPending() { - return false; - } - }, - - /* reject the merge request. */ - REJECT { - @Override - public boolean isPending() { - return false; - } - }, - - /* merge request acceptance check is pending and result is unknown yet */ - PENDING { - @Override - public boolean isPending() { - return true; - } - }, - - /* - * same as PENDING, but followed gate keeper should not be checked unless result - * of this gate keeper has been determined. - */ - PENDING_BLOCK { - @Override - public boolean isPending() { - return true; - } - }; - - public abstract boolean isPending(); - - }; - CheckResult check(MergeRequest request); } diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/GetMinScoreFromSpecifiedTeam.java b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/GetMinScoreFromSpecifiedTeam.java index 4630dc7b6d..0a9c94ee56 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/GetMinScoreFromSpecifiedTeam.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/GetMinScoreFromSpecifiedTeam.java @@ -5,12 +5,10 @@ import java.util.HashSet; import javax.validation.constraints.Min; -import com.pmease.gitop.core.Gitop; -import com.pmease.gitop.core.manager.TeamManager; import com.pmease.gitop.core.model.MergeRequest; -import com.pmease.gitop.core.model.Team; import com.pmease.gitop.core.model.TeamMembership; import com.pmease.gitop.core.model.User; +import com.pmease.gitop.core.model.Vote; public class GetMinScoreFromSpecifiedTeam extends TeamAwareGateKeeper { @@ -37,56 +35,46 @@ public class GetMinScoreFromSpecifiedTeam extends TeamAwareGateKeeper { @Override public CheckResult check(MergeRequest request) { - TeamManager teamManager = Gitop.getInstance(TeamManager.class); - Team team = teamManager.load(getTeamId()); Collection members = new HashSet(); - for (TeamMembership membership: team.getMemberships()) + for (TeamMembership membership: getTeam().getMemberships()) members.add(membership.getUser()); - boolean pendingBlock = false; - int score = 0; - int pending = 0; + int pendings = 0; + for (User member: members) { - CheckResult result = member.checkApprovalSince(request.getBaseUpdate()); - if (result == CheckResult.ACCEPT) { + Vote.Result result = member.checkVoteSince(request.getBaseUpdate()); + if (result == null) { + pendings++; + } else if (result.isAccept()) { score++; - } else if (result == CheckResult.REJECT) { - score--; - } else if (result == CheckResult.PENDING_BLOCK) { - pendingBlock = true; - pending++; } else { - pending++; + score--; } } - if (score + pending < getMinScore()) { - return CheckResult.REJECT; + if (score + pendings < getMinScore()) { + return reject("Can not get min score " + getMinScore() + " from team '" + getTeam().getName() + "'."); } else { int lackApprovals; if (isRequireVoteOfAllMembers()) { - if (score - pending >= getMinScore()) - return CheckResult.ACCEPT; - int temp = getMinScore() + pending - score; + if (score - pendings >= getMinScore()) + return accept("Get min score " + getMinScore() + " from team '" + getTeam().getName() + "'."); + int temp = getMinScore() + pendings - score; lackApprovals = temp / 2; if (temp % 2 != 0) lackApprovals++; - if (lackApprovals > pending) - lackApprovals = pending; + if (lackApprovals > pendings) + lackApprovals = pendings; } else { if (score >= getMinScore()) - return CheckResult.ACCEPT; + return accept("Get min score " + getMinScore() + " from team '" + getTeam().getName() + "'."); lackApprovals = getMinScore() - score; } - for (int i=0; i gateKeepers = new ArrayList(); @@ -22,20 +22,27 @@ public class OrGateKeeper implements GateKeeper { @Override public CheckResult check(MergeRequest request) { - boolean pending = false; + List pendingReasons = new ArrayList(); + List rejectReasons = new ArrayList(); for (GateKeeper each: getGateKeepers()) { CheckResult result = each.check(request); - if (result == CheckResult.ACCEPT || result == CheckResult.PENDING_BLOCK) + if (result.isReject()) { + rejectReasons.addAll(result.getReasons()); + } else if (result.isAccept()) { return result; - else if (result == CheckResult.PENDING) - pending = true; + } else if (result.isBlock()) { + result.getReasons().addAll(pendingReasons); + return result; + } else { + pendingReasons.addAll(result.getReasons()); + } } - if (pending) - return CheckResult.PENDING; + if (!pendingReasons.isEmpty()) + return pending(pendingReasons); else - return CheckResult.REJECT; + return reject(rejectReasons); } @Override diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/SubmittedToSpecifiedBranch.java b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/SubmittedToSpecifiedBranch.java index 60822aa7cc..9e294ad8ff 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/SubmittedToSpecifiedBranch.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/SubmittedToSpecifiedBranch.java @@ -11,7 +11,7 @@ import com.pmease.gitop.core.manager.BranchManager; import com.pmease.gitop.core.model.MergeRequest; import com.pmease.gitop.core.model.Repository; -public class SubmittedToSpecifiedBranch implements GateKeeper { +public class SubmittedToSpecifiedBranch extends AbstractGateKeeper { private String branchPatterns; @@ -31,10 +31,12 @@ public class SubmittedToSpecifiedBranch implements GateKeeper { EntityMatcher entityMatcher = new EntityMatcher(entityLoader, new WildcardPathMatcher()); PatternSetMatcher patternSetMatcher = new PatternSetMatcher(entityMatcher); + EntityPatternSet patternSet = EntityPatternSet.fromStored(getBranchPatterns(), entityLoader); + if (patternSetMatcher.matches(getBranchPatterns(), request.getDestination().getName())) - return CheckResult.ACCEPT; + return accept("Target branch matches pattern '" + patternSet + "'."); else - return CheckResult.REJECT; + return reject("Target branch does not match pattern '" + patternSet + "'."); } @Override diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/SubmittedViaPush.java b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/SubmittedViaPush.java index 85dd148944..a2e680b036 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/SubmittedViaPush.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/SubmittedViaPush.java @@ -7,9 +7,9 @@ public class SubmittedViaPush extends AbstractGateKeeper { @Override public CheckResult check(MergeRequest request) { if (request.isAutoCreated()) - return CheckResult.ACCEPT; + return accept("Submitted via push."); else - return CheckResult.REJECT; + return reject("Not submitted via push."); } } diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/TeamAwareGateKeeper.java b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/TeamAwareGateKeeper.java index b1e78b5ee5..d35ff1d38f 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/TeamAwareGateKeeper.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/TeamAwareGateKeeper.java @@ -2,6 +2,7 @@ package com.pmease.gitop.core.gatekeeper; import com.pmease.gitop.core.Gitop; import com.pmease.gitop.core.manager.TeamManager; +import com.pmease.gitop.core.model.Team; public abstract class TeamAwareGateKeeper extends AbstractGateKeeper { @@ -14,6 +15,10 @@ public abstract class TeamAwareGateKeeper extends AbstractGateKeeper { public void setTeamId(Long teamId) { this.teamId = teamId; } + + public Team getTeam() { + return Gitop.getInstance(TeamManager.class).load(getTeamId()); + } @Override public Object trim(Object context) { diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/TouchSpecifiedFiles.java b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/TouchSpecifiedFiles.java index 402811c59a..766772f378 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/TouchSpecifiedFiles.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/gatekeeper/TouchSpecifiedFiles.java @@ -46,12 +46,12 @@ public class TouchSpecifiedFiles extends AbstractGateKeeper { for (String file: touchedFiles) { if (WildcardUtils.matchPath(getFilePaths(), file)) { request.setBaseUpdate(update); - return CheckResult.ACCEPT; + return accept("Touched files match pattern '" + getFilePaths() + "'."); } } } - return CheckResult.REJECT; + return reject("No touched files match pattern '" + getFilePaths() + "'."); } } diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/manager/PendingVoteManager.java b/gitop.core/src/main/java/com/pmease/gitop/core/manager/PendingVoteManager.java deleted file mode 100644 index d54fc81759..0000000000 --- a/gitop.core/src/main/java/com/pmease/gitop/core/manager/PendingVoteManager.java +++ /dev/null @@ -1,15 +0,0 @@ -package com.pmease.gitop.core.manager; - -import com.google.inject.ImplementedBy; -import com.pmease.commons.persistence.dao.GenericDao; -import com.pmease.gitop.core.manager.impl.DefaultPendingVoteManager; -import com.pmease.gitop.core.model.MergeRequest; -import com.pmease.gitop.core.model.PendingVote; -import com.pmease.gitop.core.model.User; - -@ImplementedBy(DefaultPendingVoteManager.class) -public interface PendingVoteManager extends GenericDao { - - PendingVote find(User reviewer, MergeRequest request); - -} diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/manager/VoteInvitationManager.java b/gitop.core/src/main/java/com/pmease/gitop/core/manager/VoteInvitationManager.java new file mode 100644 index 0000000000..6c23f2ae37 --- /dev/null +++ b/gitop.core/src/main/java/com/pmease/gitop/core/manager/VoteInvitationManager.java @@ -0,0 +1,15 @@ +package com.pmease.gitop.core.manager; + +import com.google.inject.ImplementedBy; +import com.pmease.commons.persistence.dao.GenericDao; +import com.pmease.gitop.core.manager.impl.DefaultVoteInvitationManager; +import com.pmease.gitop.core.model.MergeRequest; +import com.pmease.gitop.core.model.VoteInvitation; +import com.pmease.gitop.core.model.User; + +@ImplementedBy(DefaultVoteInvitationManager.class) +public interface VoteInvitationManager extends GenericDao { + + VoteInvitation find(User reviewer, MergeRequest request); + +} diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/manager/impl/DefaultPendingVoteManager.java b/gitop.core/src/main/java/com/pmease/gitop/core/manager/impl/DefaultPendingVoteManager.java deleted file mode 100644 index 0d42f3964a..0000000000 --- a/gitop.core/src/main/java/com/pmease/gitop/core/manager/impl/DefaultPendingVoteManager.java +++ /dev/null @@ -1,41 +0,0 @@ -package com.pmease.gitop.core.manager.impl; - -import javax.inject.Provider; -import javax.inject.Singleton; - -import org.hibernate.Session; -import org.hibernate.criterion.Criterion; -import org.hibernate.criterion.Restrictions; - -import com.pmease.commons.persistence.Transactional; -import com.pmease.commons.persistence.dao.DefaultGenericDao; -import com.pmease.commons.persistence.dao.GeneralDao; -import com.pmease.gitop.core.manager.PendingVoteManager; -import com.pmease.gitop.core.model.MergeRequest; -import com.pmease.gitop.core.model.PendingVote; -import com.pmease.gitop.core.model.User; - -@Singleton -public class DefaultPendingVoteManager extends DefaultGenericDao implements PendingVoteManager { - - public DefaultPendingVoteManager(GeneralDao generalDao, Provider sessionProvider) { - super(generalDao, sessionProvider); - } - - @Transactional - @Override - public PendingVote find(User reviewer, MergeRequest request) { - return find(new Criterion[]{Restrictions.eq("reviewer", reviewer), Restrictions.eq("request", request)}); - } - - @Transactional - @Override - public void save(PendingVote pendingVote) { - if (pendingVote.getId() == null) { - pendingVote.getRequest().getPendingVotes().add(pendingVote); - pendingVote.getReviewer().getPendingVotes().add(pendingVote); - } - super.save(pendingVote); - } - -} diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/manager/impl/DefaultVoteInvitationManager.java b/gitop.core/src/main/java/com/pmease/gitop/core/manager/impl/DefaultVoteInvitationManager.java new file mode 100644 index 0000000000..68ec62bf44 --- /dev/null +++ b/gitop.core/src/main/java/com/pmease/gitop/core/manager/impl/DefaultVoteInvitationManager.java @@ -0,0 +1,49 @@ +package com.pmease.gitop.core.manager.impl; + +import javax.inject.Provider; +import javax.inject.Singleton; + +import org.hibernate.Session; +import org.hibernate.criterion.Criterion; +import org.hibernate.criterion.Restrictions; + +import com.pmease.commons.persistence.Transactional; +import com.pmease.commons.persistence.dao.DefaultGenericDao; +import com.pmease.commons.persistence.dao.GeneralDao; +import com.pmease.gitop.core.manager.VoteInvitationManager; +import com.pmease.gitop.core.model.MergeRequest; +import com.pmease.gitop.core.model.VoteInvitation; +import com.pmease.gitop.core.model.User; + +@Singleton +public class DefaultVoteInvitationManager extends DefaultGenericDao implements VoteInvitationManager { + + public DefaultVoteInvitationManager(GeneralDao generalDao, Provider sessionProvider) { + super(generalDao, sessionProvider); + } + + @Transactional + @Override + public VoteInvitation find(User reviewer, MergeRequest request) { + return find(new Criterion[]{Restrictions.eq("reviewer", reviewer), Restrictions.eq("request", request)}); + } + + @Transactional + @Override + public void save(VoteInvitation voteInvitation) { + if (voteInvitation.getId() == null) { + voteInvitation.getRequest().getVoteInvitations().add(voteInvitation); + voteInvitation.getReviewer().getVoteInvitations().add(voteInvitation); + } + super.save(voteInvitation); + } + + @Transactional + @Override + public void delete(VoteInvitation voteInvitation) { + voteInvitation.getRequest().getVoteInvitations().remove(voteInvitation); + voteInvitation.getReviewer().getVoteInvitations().remove(voteInvitation); + super.delete(voteInvitation); + } + +} diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/model/MergeRequest.java b/gitop.core/src/main/java/com/pmease/gitop/core/model/MergeRequest.java index 3e9128dba1..3089b59947 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/model/MergeRequest.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/model/MergeRequest.java @@ -19,6 +19,7 @@ import javax.persistence.OneToMany; import org.hibernate.annotations.FetchMode; +import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.pmease.commons.git.CalcMergeBaseCommand; import com.pmease.commons.git.CheckAncestorCommand; @@ -26,7 +27,9 @@ import com.pmease.commons.git.FindChangedFilesCommand; import com.pmease.commons.git.Git; import com.pmease.commons.persistence.AbstractEntity; import com.pmease.gitop.core.Gitop; -import com.pmease.gitop.core.manager.PendingVoteManager; +import com.pmease.gitop.core.gatekeeper.CheckResult; +import com.pmease.gitop.core.gatekeeper.GateKeeper; +import com.pmease.gitop.core.manager.VoteInvitationManager; import com.pmease.gitop.core.manager.RepositoryManager; @SuppressWarnings("serial") @@ -54,6 +57,8 @@ public class MergeRequest extends AbstractEntity { @Column(nullable=false) private Status status = Status.OPEN; + + private transient Optional checkResult; private transient Boolean merged; @@ -64,12 +69,14 @@ public class MergeRequest extends AbstractEntity { private transient List effectiveUpdates; private transient MergeRequestUpdate baseUpdate; - + + private transient Collection potentialVoters; + @OneToMany(mappedBy="request") private Collection updates = new ArrayList(); @OneToMany(mappedBy="request") - private Collection pendingVotes = new ArrayList(); + private Collection voteInvitations = new ArrayList(); public String getTitle() { return title; @@ -115,12 +122,12 @@ public class MergeRequest extends AbstractEntity { this.updates = updates; } - public Collection getPendingVotes() { - return pendingVotes; + public Collection getVoteInvitations() { + return voteInvitations; } - public void setPendingVotes(Collection pendingVotes) { - this.pendingVotes = pendingVotes; + public void setVoteInvitations(Collection voteInvitations) { + this.voteInvitations = voteInvitations; } public Status getStatus() { @@ -238,15 +245,61 @@ public class MergeRequest extends AbstractEntity { } return mergeBase; } - - public void requestVote(Collection candidates) { - Set pendingUsers = new HashSet(); - for (PendingVote each: getPendingVotes()) - pendingUsers.add(each.getReviewer()); - pendingUsers.retainAll(candidates); + /** + * Find potential voters for this request. Potential voters will be + * presented with the vote/reject button when they review the request. + * However they may not receive vote invitation. + *

+ * @return + * a collection of potential voters + */ + public Collection findPotentialVoters() { + check(false); + return getPotentialVoters(); + } + + private Collection getPotentialVoters() { + if (potentialVoters == null) + potentialVoters = new HashSet(); + return potentialVoters; + } + + /** + * Invite specified number of users to vote for this request. + *

+ * @param users + * a collection of users to invite users from + * @param count + * number of users to invite + */ + public void inviteToVote(final Collection users, int count) { + Collection candidates = new HashSet(users); + + // submitter is not allowed to vote for this request + candidates.remove(getSubmitter()); - if (pendingUsers.isEmpty()) { + // users already voted for latest update should be excluded + for (Vote vote: getLatestUpdate().getVotes()) + candidates.remove(vote.getReviewer()); + + getPotentialVoters().addAll(candidates); + + /* + * users already voted since base update should be excluded from + * invitation list as their votes are still valid + */ + for (Vote vote: getBaseUpdate().listVotesOnwards()) { + candidates.remove(vote.getReviewer()); + } + + Set invited = new HashSet(); + for (VoteInvitation each: getVoteInvitations()) + invited.add(each.getReviewer()); + + invited.retainAll(candidates); + + for (int i=0; i() { @Override @@ -255,14 +308,49 @@ public class MergeRequest extends AbstractEntity { } }); + + candidates.remove(selected); - PendingVote pendingVote = new PendingVote(); - pendingVote.setRequest(this); - pendingVote.setReviewer(selected); - - Gitop.getInstance(PendingVoteManager.class).save(pendingVote); + VoteInvitation invitation = new VoteInvitation(); + invitation.setRequest(this); + invitation.setReviewer(selected); + + Gitop.getInstance(VoteInvitationManager.class).save(invitation); } - + + } + + /** + * Check this request with gate keeper of target repository. + *

+ * @param force + * whether or not to force the check. Since the check might be time-consuming, Gitop + * caches the check result, and returns the cached result for subsequent checks if + * force flag is not set. + * @return + * check result of gate keeper, or Optional.absent() if no gate keeper is defined + */ + public Optional check(boolean force) { + if (checkResult == null || force) { + GateKeeper gateKeeper = getDestination().getRepository().getGateKeeper(); + if (gateKeeper != null) { + checkResult = Optional.of(gateKeeper.check(this)); + + Collection withdraws = new HashSet(); + for (VoteInvitation invitation: getVoteInvitations()) { + if (!getPotentialVoters().contains(invitation.getReviewer())) { + withdraws.add(invitation); + } + } + + for (VoteInvitation invitation: withdraws) { + Gitop.getInstance(VoteInvitationManager.class).delete(invitation); + } + } else { + checkResult = Optional.absent(); + } + } + return checkResult; } } diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/model/MergeRequestUpdate.java b/gitop.core/src/main/java/com/pmease/gitop/core/model/MergeRequestUpdate.java index 4f9a81e4b5..f72541172b 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/model/MergeRequestUpdate.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/model/MergeRequestUpdate.java @@ -34,7 +34,7 @@ public class MergeRequestUpdate extends AbstractEntity { @OneToMany(mappedBy="update") private Collection votes = new ArrayList(); - + public MergeRequest getRequest() { return request; } diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/model/Repository.java b/gitop.core/src/main/java/com/pmease/gitop/core/model/Repository.java index 5ef9713b73..c58cc4a69c 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/model/Repository.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/model/Repository.java @@ -72,6 +72,8 @@ public class Repository extends AbstractEntity implements UserBelonging { } public GateKeeper getGateKeeper() { + if (gateKeeper != null) + gateKeeper = (GateKeeper) gateKeeper.trim(this); return gateKeeper; } diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/model/User.java b/gitop.core/src/main/java/com/pmease/gitop/core/model/User.java index 0d985b2233..55143d717d 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/model/User.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/model/User.java @@ -7,7 +7,6 @@ import javax.persistence.Entity; import javax.persistence.OneToMany; import com.pmease.commons.security.AbstractUser; -import com.pmease.gitop.core.gatekeeper.GateKeeper.CheckResult; import com.pmease.gitop.core.permission.object.ProtectedObject; import com.pmease.gitop.core.permission.object.UserBelonging; @@ -48,7 +47,7 @@ public class User extends AbstractUser implements ProtectedObject { private Collection votes = new ArrayList(); @OneToMany(mappedBy="reviewer") - private Collection pendingVotes = new ArrayList(); + private Collection voteVitations = new ArrayList(); public String getDescription() { return description; @@ -106,41 +105,38 @@ public class User extends AbstractUser implements ProtectedObject { this.votes = votes; } - public Collection getPendingVotes() { - return pendingVotes; + public Collection getVoteInvitations() { + return voteVitations; } - public void setPendingVotes(Collection pendingVotes) { - this.pendingVotes = pendingVotes; + public void setVoteInvitations(Collection voteInvitations) { + this.voteVitations = voteInvitations; } @Override public boolean has(ProtectedObject object) { if (object instanceof User) { User user = (User) object; - return user.getId().equals(getId()); + return user.equals(this); } else if (object instanceof UserBelonging) { UserBelonging userBelonging = (UserBelonging) object; - return userBelonging.getUser().getId().equals(getId()); + return userBelonging.getUser().equals(this); } else { return false; } } - public CheckResult checkApprovalSince(MergeRequestUpdate update) { - if (update.getRequest().getSubmitter().getId().equals(getId())) - return CheckResult.ACCEPT; + public Vote.Result checkVoteSince(MergeRequestUpdate update) { + if (update.getRequest().getSubmitter().equals(this)) + return Vote.Result.ACCEPT; for (Vote vote: update.listVotesOnwards()) { if (vote.getReviewer().equals(this)) { - if (vote.getResult() == Vote.Result.ACCEPT) - return CheckResult.ACCEPT; - else - return CheckResult.REJECT; + return vote.getResult(); } } - return CheckResult.PENDING; + return null; } } diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/model/Vote.java b/gitop.core/src/main/java/com/pmease/gitop/core/model/Vote.java index d7aae69737..9a88a39ea1 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/model/Vote.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/model/Vote.java @@ -19,7 +19,34 @@ import com.pmease.commons.persistence.AbstractEntity; }) public class Vote extends AbstractEntity { - public static enum Result {ACCEPT, REJECT}; + public static enum Result { + ACCEPT { + @Override + public boolean isAccept() { + return true; + } + + @Override + public boolean isReject() { + return false; + } + }, + REJECT { + @Override + public boolean isAccept() { + return false; + } + + @Override + public boolean isReject() { + return true; + } + }; + + public abstract boolean isAccept(); + + public abstract boolean isReject(); + }; @ManyToOne(fetch=FetchType.EAGER) @org.hibernate.annotations.Fetch(FetchMode.SELECT) diff --git a/gitop.core/src/main/java/com/pmease/gitop/core/model/PendingVote.java b/gitop.core/src/main/java/com/pmease/gitop/core/model/VoteInvitation.java similarity index 95% rename from gitop.core/src/main/java/com/pmease/gitop/core/model/PendingVote.java rename to gitop.core/src/main/java/com/pmease/gitop/core/model/VoteInvitation.java index 03cd92e2c5..dcd4967d0e 100644 --- a/gitop.core/src/main/java/com/pmease/gitop/core/model/PendingVote.java +++ b/gitop.core/src/main/java/com/pmease/gitop/core/model/VoteInvitation.java @@ -16,7 +16,7 @@ import com.pmease.commons.persistence.AbstractEntity; @Table(uniqueConstraints={ @UniqueConstraint(columnNames={"reviewer", "request"}) }) -public class PendingVote extends AbstractEntity { +public class VoteInvitation extends AbstractEntity { @ManyToOne(fetch=FetchType.EAGER) @org.hibernate.annotations.Fetch(FetchMode.SELECT)