From 4e4571fa7aba30e9a0cde1effb594afb437a175a Mon Sep 17 00:00:00 2001 From: Elias Kuiter Date: Mon, 10 Jun 2019 22:50:14 +0200 Subject: [PATCH] Optionally, allow collaborators involved in a conflict to vote --- .../components/overlays/CommandPalette.tsx | 25 ++++++-- client/src/i18n.tsx | 2 + client/src/store/actions.ts | 4 +- client/src/store/types.ts | 2 +- kernel/src/kernel/api.cljc | 8 +-- .../src/kernel/core/conflict_resolution.cljc | 13 ++++- kernel/src/kernel/shell/server.cljc | 4 +- .../spldev/varied/CollaborativeSession.java | 26 +++++++-- .../de/ovgu/spldev/varied/VotingPhase.java | 57 ++++++++++++++----- .../de/ovgu/spldev/varied/messaging/Api.java | 3 + 10 files changed, 109 insertions(+), 35 deletions(-) diff --git a/client/src/components/overlays/CommandPalette.tsx b/client/src/components/overlays/CommandPalette.tsx index 1e136e2a..6fe3ba20 100644 --- a/client/src/components/overlays/CommandPalette.tsx +++ b/client/src/components/overlays/CommandPalette.tsx @@ -71,7 +71,8 @@ interface ArgumentDescriptor { items?: PaletteItemsFunction, allowFreeform?: boolean, transformFreeform?: (value: string) => string, - title?: string + title?: string, + skipArguments?: (value: string) => number }; function clearLocalStorage() { @@ -128,12 +129,16 @@ export default class extends React.Component { toPaletteItemsFunction = (items?: PaletteItemsFunction) => items || (() => []), argumentDescriptor: ArgumentDescriptor = toArgumentDescriptor(args[0]), // bind current argument and recurse (until all arguments are bound) - recurse = (value: string) => - this.actionWithArguments( - args.slice(1).map(toArgumentDescriptor).map(argument => ({ + recurse = (value: string) => { + const currentArgument = toArgumentDescriptor(args[0]); + return this.actionWithArguments( + args.slice(currentArgument.skipArguments ? currentArgument.skipArguments(value) + 1 : 1) + .map(toArgumentDescriptor) + .map(argument => ({ ...argument, items: toPaletteItemsFunction(argument.items).bind(undefined, value) })), action.bind(undefined, value)); + }; this.setState({ rerenderPalette: +new Date(), @@ -595,9 +600,17 @@ export default class extends React.Component { [{ title: i18n.t('commandPalette.featureDiagram.votingStrategy'), items: () => Object.values(VotingStrategy).map(votingStrategy => - ({text: i18n.t('commandPalette.featureDiagram', votingStrategy), key: votingStrategy})) + ({text: i18n.t('commandPalette.featureDiagram', votingStrategy), key: votingStrategy})), + skipArguments: (votingStrategy: string) => votingStrategy === VotingStrategy.reject ? 1 : 0 + }, { + title: i18n.t('commandPalette.featureDiagram.votingStrategy'), + items: () => [ + {text: i18n.t('commandPalette.featureDiagram.everyone'), key: 'false'}, + {text: i18n.t('commandPalette.featureDiagram.onlyInvolved'), key: 'true'} + ] }], - votingStrategy => this.props.onSetVotingStrategy({votingStrategy})) + (votingStrategy, onlyInvolved) => this.props.onSetVotingStrategy( + {votingStrategy, onlyInvolved: onlyInvolved === 'true'})) }, { text: i18n.t('commandPalette.developer.debug'), icon: 'DeveloperTools', diff --git a/client/src/i18n.tsx b/client/src/i18n.tsx index 29e1127e..7d6700c5 100644 --- a/client/src/i18n.tsx +++ b/client/src/i18n.tsx @@ -174,6 +174,8 @@ const translationMap = { }, votingStrategy: 'Voting strategy', setVotingStrategy: 'Set voting strategy', + onlyInvolved: 'Only collaborators involved in a conflict may vote', + everyone: 'Everyone may vote (default)', reject: 'Reject conflicts', firstVote: 'First vote wins', plurality: 'Plurality vote', diff --git a/client/src/store/actions.ts b/client/src/store/actions.ts index c93f8cae..258b2d35 100644 --- a/client/src/store/actions.ts +++ b/client/src/store/actions.ts @@ -156,8 +156,8 @@ const actions = { }, vote: createMessageAction(({versionID}: {versionID?: string}) => ({type: MessageType.VOTE, versionID})), - setVotingStrategy: createMessageAction(({votingStrategy}: {votingStrategy: string}) => - ({type: MessageType.SET_VOTING_STRATEGY, votingStrategy})) + setVotingStrategy: createMessageAction(({votingStrategy, onlyInvolved}: {votingStrategy: string, onlyInvolved: boolean}) => + ({type: MessageType.SET_VOTING_STRATEGY, votingStrategy, onlyInvolved})) } } }; diff --git a/client/src/store/types.ts b/client/src/store/types.ts index cf90171c..2d817ed5 100644 --- a/client/src/store/types.ts +++ b/client/src/store/types.ts @@ -109,7 +109,7 @@ export type OnToggleFeatureGroupTypeFunction = (payload: {feature: Feature}) => export type OnSetUserProfileFunction = (payload: {name: string}) => Promise; export type OnResetFunction = () => Promise; export type OnVoteFunction = (payload: {versionID?: string}) => Promise; -export type OnSetVotingStrategyFunction = (payload: {votingStrategy: string}) => Promise; +export type OnSetVotingStrategyFunction = (payload: {votingStrategy: string, onlyInvolved: boolean}) => Promise; // Props that may derived from the state to use in React components. // This enforces the convention that a prop called 'on...' has the same type in all components. diff --git a/kernel/src/kernel/api.cljc b/kernel/src/kernel/api.cljc index 830ffa96..bbf848bf 100644 --- a/kernel/src/kernel/api.cljc +++ b/kernel/src/kernel/api.cljc @@ -150,9 +150,9 @@ [message] (profile {} - (let [[voting? message] (server/forward-message! (helpers/decode message))] + (let [[involved-site-IDs message] (server/forward-message! (helpers/decode message))] (into-array Object - [voting? + [(when involved-site-IDs (into-array involved-site-IDs)) (helpers/encode message)])))) (defn serverSiteJoined @@ -184,9 +184,9 @@ [site-ID] (profile {} - (let [[voting? message] (server/site-left! site-ID)] + (let [[involved-site-IDs message] (server/site-left! site-ID)] (into-array Object - [voting? + [(when involved-site-IDs (into-array involved-site-IDs)) (helpers/encode message)])))) diff --git a/kernel/src/kernel/core/conflict_resolution.cljc b/kernel/src/kernel/core/conflict_resolution.cljc index 67511054..5a66fe20 100644 --- a/kernel/src/kernel/core/conflict_resolution.cljc +++ b/kernel/src/kernel/core/conflict_resolution.cljc @@ -1,6 +1,7 @@ (ns kernel.core.conflict-resolution "Utilities for assisting in conflict resolution." - (:require [kernel.core.vector-clock :as VC] + (:require [clojure.set :as set] + [kernel.core.vector-clock :as VC] [kernel.core.history-buffer :as HB] [kernel.core.conflict-cache :as CC] [kernel.core.garbage-collector :as GC] @@ -97,6 +98,16 @@ (let [_conflict-descriptor combined-effect] (_conflict-descriptor :synchronized))))) +(defn involved-site-IDs + "Returns all sites which are involved in any conflict. + This is useful for only allowing those sites to participate in the voting phase." + [MCGS HB combined-effect] + (p ::involved-site-IDs + (when (voting? MCGS combined-effect) + ; these are all operations involved in some conflict (as they are not in the neutral CG) + (let [conflicting-operations (set/difference (MOVIC/get-all-operations MCGS) (MOVIC/neutral-CG MCGS))] + (distinct (map #(CO/get-site-ID (HB/lookup HB %)) conflicting-operations)))))) + (defn resolved-MCG "For an agreed resolved version, extracts the according MCG from an MCGS." [MCGS MCG-ID] diff --git a/kernel/src/kernel/shell/server.cljc b/kernel/src/kernel/shell/server.cljc index d9ff740e..5d069c4e 100644 --- a/kernel/src/kernel/shell/server.cljc +++ b/kernel/src/kernel/shell/server.cljc @@ -69,7 +69,7 @@ (site/receive-message! new-message) ; ignore returned feature model on the server (generate-heartbeat!) (swap! (*context* :GC) #(GC/insert % :server (GC-filter @(*context* :VC)))) - [(conflict-resolution/voting? @(*context* :MCGS) @(*context* :combined-effect)) + [(conflict-resolution/involved-site-IDs @(*context* :MCGS) @(*context* :HB) @(*context* :combined-effect)) (message/with-server-VC new-message (GC-filter @(*context* :VC)))]))) (defn site-joined! @@ -113,7 +113,7 @@ (let [message (message/make-leave site-ID)] (site/receive-leave! message) (swap! (*context* :offline-sites) #(conj % site-ID)) - [(conflict-resolution/voting? @(*context* :MCGS) @(*context* :combined-effect)) + [(conflict-resolution/involved-site-IDs @(*context* :MCGS) @(*context* :HB) @(*context* :combined-effect)) message]))) (defn resolve-conflict! diff --git a/server/src/main/java/de/ovgu/spldev/varied/CollaborativeSession.java b/server/src/main/java/de/ovgu/spldev/varied/CollaborativeSession.java index 6e76461e..4792d57a 100644 --- a/server/src/main/java/de/ovgu/spldev/varied/CollaborativeSession.java +++ b/server/src/main/java/de/ovgu/spldev/varied/CollaborativeSession.java @@ -8,6 +8,8 @@ import org.pmw.tinylog.Logger; import java.util.*; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** * A collaborative session consists of a set of collaborators that view and edit a artifact together. @@ -60,6 +62,7 @@ void onMessage(Collaborator collaborator, Message message) throws Message.Invali static class FeatureModel extends CollaborativeSession { private Kernel kernel; private String votingStrategy = "consensus"; + private boolean onlyInvolved = false; private VotingPhase votingPhase; FeatureModel(Artifact.Path artifactPath, IFeatureModel initialFeatureModel) { @@ -68,12 +71,21 @@ static class FeatureModel extends CollaborativeSession { this.kernel = new Kernel(artifactPath, initialFeatureModel); } - private void broadcastResponse(Collaborator collaborator, Object[] votingAndMessage) { - boolean isVoting = (boolean) votingAndMessage[0]; - String newMessage = (String) votingAndMessage[1]; + private void broadcastResponse(Collaborator collaborator, Object[] involvedSiteIDsAndMessage) { + String[] involvedSiteIDs = (String[]) involvedSiteIDsAndMessage[0]; + String newMessage = (String) involvedSiteIDsAndMessage[1]; CollaboratorUtils.broadcastToOtherCollaborators(collaborators, new Api.Kernel(artifactPath, newMessage), collaborator); - if (isVoting && votingPhase == null) { - votingPhase = new VotingPhase(VotingPhase.VotingStrategy.fromString(votingStrategy, collaborators)); + if (involvedSiteIDs != null && votingPhase == null) { + Logger.info("{} collaborators involved in the conflict", involvedSiteIDs.length); + Collection involvedCollaborators = + Stream.of(involvedSiteIDs) + .map(siteID -> collaborators.stream() + .filter(_collaborator -> _collaborator.getSiteID().equals(UUID.fromString(siteID))) + .findFirst() + .get()) + .collect(Collectors.toCollection(HashSet::new)); + votingPhase = new VotingPhase(VotingPhase.VotingStrategy.createInstance( + votingStrategy, onlyInvolved, collaborators, involvedCollaborators)); broadcastVoters(); updateVotingPhase(); } @@ -102,8 +114,10 @@ protected boolean _onMessage(Collaborator collaborator, Message.IDecodable messa if (votingPhase != null) throw new RuntimeException("can not change voting strategy while in voting phase"); String votingStrategy = ((Api.SetVotingStrategy) message).votingStrategy; - Logger.info("setting voting strategy to {}", votingStrategy); + boolean onlyInvolved = ((Api.SetVotingStrategy) message).onlyInvolved; + Logger.info("setting voting strategy to {} ({})", votingStrategy, onlyInvolved ? "only involved" : "everyone"); this.votingStrategy = votingStrategy; + this.onlyInvolved = onlyInvolved; return true; } diff --git a/server/src/main/java/de/ovgu/spldev/varied/VotingPhase.java b/server/src/main/java/de/ovgu/spldev/varied/VotingPhase.java index 944cfab5..49fea722 100644 --- a/server/src/main/java/de/ovgu/spldev/varied/VotingPhase.java +++ b/server/src/main/java/de/ovgu/spldev/varied/VotingPhase.java @@ -52,6 +52,32 @@ public Collection getVoters() { return voters; } } + + class OnlyInvolved implements IVoters { + Set involvedCollaborators, voters; + + OnlyInvolved(Collection involvedCollaborators) { + this.involvedCollaborators = new HashSet<>(involvedCollaborators); + voters = new HashSet<>(involvedCollaborators); + } + + public void onJoin(Collaborator collaborator) { + if (involvedCollaborators.contains(collaborator)) + voters.add(collaborator); + } + + public void onLeave(Collaborator collaborator) { + voters.remove(collaborator); + } + + public boolean isEligible(Collaborator collaborator) { + return voters.contains(collaborator); + } + + public Collection getVoters() { + return voters; + } + } } interface IResolutionCriterion { @@ -173,46 +199,51 @@ static VotingStrategy reject() { new IResolutionOutcome.Neutral()); } - static VotingStrategy firstVote(Collection collaborators) { + static VotingStrategy firstVote(IVoters voters) { return new VotingStrategy( - new IVoters.Everyone(collaborators), + voters, new IResolutionCriterion.OnFirstVote(), new IResolutionOutcome.Any()); } - static VotingStrategy plurality(Collection collaborators) { + static VotingStrategy plurality(IVoters voters) { return new VotingStrategy( - new IVoters.Everyone(collaborators), + voters, new IResolutionCriterion.OnLastVote(), new IResolutionOutcome.Plurality()); } - static VotingStrategy majority(Collection collaborators) { + static VotingStrategy majority(IVoters voters) { return new VotingStrategy( - new IVoters.Everyone(collaborators), + voters, new IResolutionCriterion.OnLastVote(), new IResolutionOutcome.Majority()); } - static VotingStrategy consensus(Collection collaborators) { + static VotingStrategy consensus(IVoters voters) { return new VotingStrategy( - new IVoters.Everyone(collaborators), + voters, new IResolutionCriterion.OnLastVoteOrDissent(), new IResolutionOutcome.Consensus()); } - static VotingStrategy fromString(String votingStrategy, Collection collaborators) { + static VotingStrategy createInstance( + String votingStrategy, boolean onlyInvolved, + Collection collaborators, Collection involvedCollaborators) { + IVoters voters = onlyInvolved + ? new IVoters.OnlyInvolved(involvedCollaborators) + : new IVoters.Everyone(collaborators); switch (votingStrategy) { case "reject": return reject(); case "firstVote": - return firstVote(collaborators); + return firstVote(voters); case "plurality": - return plurality(collaborators); + return plurality(voters); case "majority": - return majority(collaborators); + return majority(voters); case "consensus": - return consensus(collaborators); + return consensus(voters); default: throw new RuntimeException("invalid voting strategy given"); } diff --git a/server/src/main/java/de/ovgu/spldev/varied/messaging/Api.java b/server/src/main/java/de/ovgu/spldev/varied/messaging/Api.java index 12e04b67..616787a7 100644 --- a/server/src/main/java/de/ovgu/spldev/varied/messaging/Api.java +++ b/server/src/main/java/de/ovgu/spldev/varied/messaging/Api.java @@ -157,5 +157,8 @@ public ResolutionOutcome(Artifact.Path artifactPath, String versionID) { public static class SetVotingStrategy extends Message implements Message.IDecodable { @Expose public String votingStrategy; + + @Expose + public boolean onlyInvolved; } }