From db03b51bfad1e3807febd2d4e73d5979a33ebea7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kai=20B=C3=B6rnert?= Date: Wed, 12 Dec 2018 11:28:13 +0100 Subject: [PATCH] changed removeRegatta to cause less issues due to sideeffects --- .../gwt/ui/server/SailingServiceImpl.java | 63 ++++++++----------- .../server/test/RemoveLeaderboardTest.java | 3 - .../server/impl/RacingEventServiceImpl.java | 14 ++++- .../com/sap/sse/security/SecurityService.java | 2 + .../security/impl/SecurityServiceImpl.java | 5 ++ 5 files changed, 46 insertions(+), 41 deletions(-) diff --git a/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/server/SailingServiceImpl.java b/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/server/SailingServiceImpl.java index e9e749db750..c2cd6fed2cd 100755 --- a/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/server/SailingServiceImpl.java +++ b/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/server/SailingServiceImpl.java @@ -4630,45 +4630,34 @@ public class SailingServiceImpl extends ProxiedRemoteServiceServlet implements S public void removeRegatta(RegattaIdentifier regattaIdentifier) { Regatta regatta = getService().getRegatta(regattaIdentifier); if (regatta != null) { - getSecurityService().checkPermissionAndDeleteOwnershipForObjectRemoval(regatta, new Action() { - @Override - public void run() throws Exception { - // if there were any tracked races, free all ownerships - for (RaceDefinition race : regatta.getAllRaces()) { - String typeRelativeObjectIdentifier = WildcardPermissionEncoder.encode(regatta.getName(), - race.getName()); - QualifiedObjectIdentifier identifier = SecuredDomainType.TRACKED_RACE - .getQualifiedObjectIdentifier(typeRelativeObjectIdentifier); - getSecurityService().checkPermissionAndDeleteOwnershipForObjectRemoval(identifier, - new ActionWithResult() { - @Override - public Void run() throws Exception { - RegattaNameAndRaceName raceIdentifier = new RegattaNameAndRaceName( - regatta.getName(), race.getName()); - ArrayList trackedRace = new ArrayList<>(); - trackedRace.add(raceIdentifier); - removeAndUntrackRaces(trackedRace); - return null; - } - }); - - // remove all leaderboards using the proper call that will clean and remove the owner as well - Set leaderboardsToRemove = new HashSet<>(); - for (Leaderboard leaderboard : getService().getLeaderboards().values()) { - if (leaderboard instanceof RegattaLeaderboard) { - RegattaLeaderboard regattaLeaderboard = (RegattaLeaderboard) leaderboard; - if (regattaLeaderboard.getRegatta() == regatta) { - leaderboardsToRemove.add(regattaLeaderboard); - } - } - } - for (RegattaLeaderboard regattaLeaderboardToRemove : leaderboardsToRemove) { - removeLeaderboard(regattaLeaderboardToRemove.getName()); - } + Set objectsThatWillBeImplicitlyCleanedByRemoveRegatta = new HashSet<>(); + objectsThatWillBeImplicitlyCleanedByRemoveRegatta.add(regatta.getIdentifier()); + for (RaceDefinition race : regatta.getAllRaces()) { + String typeRelativeObjectIdentifier = WildcardPermissionEncoder.encode(regatta.getName(), + race.getName()); + QualifiedObjectIdentifier identifier = SecuredDomainType.TRACKED_RACE + .getQualifiedObjectIdentifier(typeRelativeObjectIdentifier); + objectsThatWillBeImplicitlyCleanedByRemoveRegatta.add(identifier); + } + for (Leaderboard leaderboard : getService().getLeaderboards().values()) { + if (leaderboard instanceof RegattaLeaderboard) { + RegattaLeaderboard regattaLeaderboard = (RegattaLeaderboard) leaderboard; + if (regattaLeaderboard.getRegatta() == regatta) { + objectsThatWillBeImplicitlyCleanedByRemoveRegatta.add(regattaLeaderboard.getIdentifier()); } - getService().apply(new RemoveRegatta(regattaIdentifier)); } - }); + } + // check if we can delete everything RemoveRegatta will remove + for (QualifiedObjectIdentifier toRemovePermissionObjects : objectsThatWillBeImplicitlyCleanedByRemoveRegatta) { + getSecurityService().checkCurrentUserDeletePermission(toRemovePermissionObjects); + } + // we have all permissions, execute + getService().apply(new RemoveRegatta(regattaIdentifier)); + // cleanup the Ownership and ACLs + for (QualifiedObjectIdentifier toRemovePermissionObjects : objectsThatWillBeImplicitlyCleanedByRemoveRegatta) { + getSecurityService().deleteOwnership(toRemovePermissionObjects); + } + } } diff --git a/java/com.sap.sailing.server.test/src/com/sap/sailing/server/test/RemoveLeaderboardTest.java b/java/com.sap.sailing.server.test/src/com/sap/sailing/server/test/RemoveLeaderboardTest.java index 7bf5f2e9d2e..709c629de66 100755 --- a/java/com.sap.sailing.server.test/src/com/sap/sailing/server/test/RemoveLeaderboardTest.java +++ b/java/com.sap.sailing.server.test/src/com/sap/sailing/server/test/RemoveLeaderboardTest.java @@ -46,7 +46,6 @@ import com.sap.sailing.domain.tracking.TrackedRegatta; import com.sap.sailing.domain.tracking.impl.EmptyWindStore; import com.sap.sailing.server.impl.RacingEventServiceImpl; import com.sap.sailing.server.operationaltransformation.CreateFlexibleLeaderboard; -import com.sap.sailing.server.operationaltransformation.RemoveAndUntrackRace; import com.sap.sailing.server.operationaltransformation.RemoveEvent; import com.sap.sailing.server.operationaltransformation.RemoveLeaderboard; import com.sap.sailing.server.operationaltransformation.RemoveLeaderboardGroup; @@ -120,8 +119,6 @@ public class RemoveLeaderboardTest { public void testRaceLogOfRaceColumnInSeriesDetachedWhenRegattaIsRemoved() { regatta.getRaceColumnByName(R1).setTrackedRace(defaultFleet, trackedRace); assertEquals(1, Util.size(trackedRace.getAttachedRaceLogs())); - // removal of races is no longer done automatically, due to restrictions based on permission-vertical - server.apply(new RemoveAndUntrackRace(trackedRace.getRaceIdentifier())); server.apply(new RemoveRegatta(new RegattaName(regatta.getName()))); assertTrue(Util.isEmpty(trackedRace.getAttachedRaceLogs())); } diff --git a/java/com.sap.sailing.server/src/com/sap/sailing/server/impl/RacingEventServiceImpl.java b/java/com.sap.sailing.server/src/com/sap/sailing/server/impl/RacingEventServiceImpl.java index b376471df70..5447ad27b13 100755 --- a/java/com.sap.sailing.server/src/com/sap/sailing/server/impl/RacingEventServiceImpl.java +++ b/java/com.sap.sailing.server/src/com/sap/sailing/server/impl/RacingEventServiceImpl.java @@ -2503,11 +2503,23 @@ public class RacingEventServiceImpl implements RacingEventService, ClearStateTes @Override public void removeRegatta(Regatta regatta) throws MalformedURLException, IOException, InterruptedException { + Set leaderboardsToRemove = new HashSet<>(); + for (Leaderboard leaderboard : getLeaderboards().values()) { + if (leaderboard instanceof RegattaLeaderboard) { + RegattaLeaderboard regattaLeaderboard = (RegattaLeaderboard) leaderboard; + if (regattaLeaderboard.getRegatta() == regatta) { + leaderboardsToRemove.add(regattaLeaderboard); + } + } + } + for (RegattaLeaderboard regattaLeaderboardToRemove : leaderboardsToRemove) { + removeLeaderboard(regattaLeaderboardToRemove.getName()); + } // avoid ConcurrentModificationException by copying the races to remove: Set racesToRemove = new HashSet<>(); Util.addAll(regatta.getAllRaces(), racesToRemove); for (RaceDefinition race : racesToRemove) { - // FIXME can this be removed//moved to the removeRace part? + removeRace(regatta, race); mongoObjectFactory.removeRegattaForRaceID(race.getName(), regatta); persistentRegattasForRaceIDs.remove(race.getId().toString()); } diff --git a/java/com.sap.sse.security/src/com/sap/sse/security/SecurityService.java b/java/com.sap.sse.security/src/com/sap/sse/security/SecurityService.java index d79db3f31e6..e7bd7e873e6 100755 --- a/java/com.sap.sse.security/src/com/sap/sse/security/SecurityService.java +++ b/java/com.sap.sse.security/src/com/sap/sse/security/SecurityService.java @@ -375,6 +375,8 @@ public interface SecurityService extends ReplicableWithObjectInputStream