From e956d914840a9508c0b45bbb49ce61afe7d97f41 Mon Sep 17 00:00:00 2001 From: Axel Uhl Date: Mon, 14 May 2018 18:33:18 +0200 Subject: [PATCH] bug 4574: fixing multiple registration of leaderboard cache invalidators; added boat chance-dependent leaderboard invalidation; fixed boat update routine Change-Id: I88a351768ec5bf3cdd3f63dafed0e3ec1004d80e --- .../impl/MongoObjectFactoryImpl.java | 3 +- .../domain/base/BoatChangeListener.java | 4 +- .../domain/base/CompetitorAndBoatStore.java | 2 +- .../domain/base/CompetitorFactory.java | 6 ++ .../TransientCompetitorAndBoatStoreImpl.java | 11 +-- .../impl/DomainFactoryImpl.java | 2 +- .../impl/DomainFactoryImpl.java | 3 +- .../leaderboard/LeaderboardCacheManager.java | 82 ++++++++++++++----- .../gwt/ui/server/SailingServiceImpl.java | 1 - .../PersistentCompetitorAndBoatStore.java | 4 +- .../AllowCompetitorResetToDefaults.java | 4 + 11 files changed, 87 insertions(+), 35 deletions(-) diff --git a/java/com.sap.sailing.domain.persistence/src/com/sap/sailing/domain/persistence/impl/MongoObjectFactoryImpl.java b/java/com.sap.sailing.domain.persistence/src/com/sap/sailing/domain/persistence/impl/MongoObjectFactoryImpl.java index f518dc4be43..5abc071082d 100644 --- a/java/com.sap.sailing.domain.persistence/src/com/sap/sailing/domain/persistence/impl/MongoObjectFactoryImpl.java +++ b/java/com.sap.sailing.domain.persistence/src/com/sap/sailing/domain/persistence/impl/MongoObjectFactoryImpl.java @@ -1305,7 +1305,8 @@ public class MongoObjectFactoryImpl implements MongoObjectFactory { @Override public void storeCompetitor(Competitor competitor) { if (competitor.hasBoat()) { - storeCompetitorWithBoat((CompetitorWithBoat) competitor); + storeCompetitorWithBoat((CompetitorWithBoat) competitor); + storeBoat(((CompetitorWithBoat) competitor).getBoat()); } else { storeCompetitorWithoutBoat(competitor); } diff --git a/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/base/BoatChangeListener.java b/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/base/BoatChangeListener.java index 6127dad6b2e..d87477acf0a 100755 --- a/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/base/BoatChangeListener.java +++ b/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/base/BoatChangeListener.java @@ -1,8 +1,10 @@ package com.sap.sailing.domain.base; +import java.io.Serializable; + import com.sap.sse.common.Color; -public interface BoatChangeListener { +public interface BoatChangeListener extends Serializable { void nameChanged(String oldName, String newName); void colorChanged(Color oldColor, Color newColor); diff --git a/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/base/CompetitorAndBoatStore.java b/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/base/CompetitorAndBoatStore.java index 391a9fae47d..034d32766cb 100755 --- a/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/base/CompetitorAndBoatStore.java +++ b/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/base/CompetitorAndBoatStore.java @@ -140,7 +140,7 @@ public interface CompetitorAndBoatStore extends CompetitorFactory, BoatFactory { */ CompetitorWithBoat updateCompetitorWithBoat(String idAsString, String newName, String newShortName, Color newDisplayColor, String newEmail, Nationality newNationality, URI newTeamImageUri, URI newFlagImageUri, - Double timeOnTimeFactor, Duration timeOnDistanceAllowancePerNauticalMile, String searchTag, DynamicBoat boat); + Double timeOnTimeFactor, Duration timeOnDistanceAllowancePerNauticalMile, String searchTag); /** * If a valid boat is returned and the caller has information available that could be used to update the boat, diff --git a/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/base/CompetitorFactory.java b/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/base/CompetitorFactory.java index 6baa7f7989e..1ad970ac700 100644 --- a/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/base/CompetitorFactory.java +++ b/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/base/CompetitorFactory.java @@ -35,6 +35,12 @@ public interface CompetitorFactory { DynamicCompetitor getOrCreateCompetitor(Serializable competitorId, String name, String shortName, Color displayColor, String email, URI flagImageURI, DynamicTeam team, Double timeOnTimeFactor, Duration timeOnDistanceAllowancePerNauticalMile, String searchTag); + /** + * Updates only the competitor fields; the {@code boat} parameter is used only when the competitor needs to be + * created because it doesn't exist yet. In this case the boat is assigned to the new competitor and stored in the + * competitor and boat store. If changes to an existing competitor's {@link Boat} object shall be stored, use + * {@link CompetitorAndBoatStore#getOrCreateBoat(Serializable, String, BoatClass, String, Color)}. + */ DynamicCompetitorWithBoat getOrCreateCompetitorWithBoat(Serializable competitorId, String name, String shortName, Color displayColor, String email, URI flagImageURI, DynamicTeam team, Double timeOnTimeFactor, Duration timeOnDistanceAllowancePerNauticalMile, String searchTag, DynamicBoat boat); diff --git a/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/base/impl/TransientCompetitorAndBoatStoreImpl.java b/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/base/impl/TransientCompetitorAndBoatStoreImpl.java index 36e8279f3a9..a93acb78791 100755 --- a/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/base/impl/TransientCompetitorAndBoatStoreImpl.java +++ b/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/base/impl/TransientCompetitorAndBoatStoreImpl.java @@ -440,8 +440,9 @@ public class TransientCompetitorAndBoatStoreImpl implements CompetitorAndBoatSto LockUtil.unlockAfterWrite(lock); } } else if (isCompetitorToUpdateDuringGetOrCreate(result)) { + assert result.getBoat() == boat; updateCompetitorWithBoat(result.getId().toString(), name, shortName, displayColor, email, team.getNationality(), - team.getImage(), flagImage, timeOnTimeFactor, timeOnDistanceAllowancePerNauticalMile, searchTag, boat); + team.getImage(), flagImage, timeOnTimeFactor, timeOnDistanceAllowancePerNauticalMile, searchTag); competitorNoLongerToUpdateDuringGetOrCreate(result); } return result; @@ -450,7 +451,7 @@ public class TransientCompetitorAndBoatStoreImpl implements CompetitorAndBoatSto @Override public CompetitorWithBoat updateCompetitorWithBoat(String idAsString, String newName, String newShortName, Color newDisplayColor, String newEmail, Nationality newNationality, URI newTeamImageUri, URI newFlagImageUri, - Double timeOnTimeFactor, Duration timeOnDistanceAllowancePerNauticalMile, String newSearchTag, DynamicBoat boat) { + Double timeOnTimeFactor, Duration timeOnDistanceAllowancePerNauticalMile, String newSearchTag) { DynamicCompetitorWithBoat competitor = (DynamicCompetitorWithBoat) getExistingCompetitorByIdAsString(idAsString); if (competitor != null) { LockUtil.lockForWrite(lock); @@ -465,12 +466,6 @@ public class TransientCompetitorAndBoatStoreImpl implements CompetitorAndBoatSto competitor.setTimeOnTimeFactor(timeOnTimeFactor); competitor.setTimeOnDistanceAllowancePerNauticalMile(timeOnDistanceAllowancePerNauticalMile); competitor.setSearchTag(newSearchTag); - DynamicBoat boatOfCompetitor = competitor.getBoat(); - if (boatOfCompetitor != null && boat != null) { - boatOfCompetitor.setName(boat.getName()); - boatOfCompetitor.setSailId(boat.getSailID()); - boatOfCompetitor.setColor(boat.getColor()); - } weakCompetitorDTOCache.remove(competitor); } finally { LockUtil.unlockAfterWrite(lock); diff --git a/java/com.sap.sailing.domain.swisstimingadapter/src/com/sap/sailing/domain/swisstimingadapter/impl/DomainFactoryImpl.java b/java/com.sap.sailing.domain.swisstimingadapter/src/com/sap/sailing/domain/swisstimingadapter/impl/DomainFactoryImpl.java index 5554aaf2e82..8c8a96f8a0c 100644 --- a/java/com.sap.sailing.domain.swisstimingadapter/src/com/sap/sailing/domain/swisstimingadapter/impl/DomainFactoryImpl.java +++ b/java/com.sap.sailing.domain.swisstimingadapter/src/com/sap/sailing/domain/swisstimingadapter/impl/DomainFactoryImpl.java @@ -133,7 +133,7 @@ public class DomainFactoryImpl implements DomainFactory { teamMembers.add(person); } DynamicTeam team = new TeamImpl(competitor.getName(), teamMembers, /* coach */ null); - DynamicBoat domainBoat = new BoatImpl(competitorId, null, boatClass, competitor.getBoatID(), null); + final DynamicBoat domainBoat = competitorAndBoatStore.getOrCreateBoat(competitorId, /* name */ null, boatClass, competitor.getBoatID(), /* color */ null); domainCompetitor = competitorAndBoatStore.getOrCreateCompetitorWithBoat(competitorId, competitor.getName(), null /* shortName */, null /*displayColor*/, null /*email*/, null, team, /* timeOnTimeFactor */ null, /* timeOnDistanceAllowancePerNauticalMile */ null, null, domainBoat); } diff --git a/java/com.sap.sailing.domain.tractracadapter/src/com/sap/sailing/domain/tractracadapter/impl/DomainFactoryImpl.java b/java/com.sap.sailing.domain.tractracadapter/src/com/sap/sailing/domain/tractracadapter/impl/DomainFactoryImpl.java index f187aa68bf3..4d250e758a0 100755 --- a/java/com.sap.sailing.domain.tractracadapter/src/com/sap/sailing/domain/tractracadapter/impl/DomainFactoryImpl.java +++ b/java/com.sap.sailing.domain.tractracadapter/src/com/sap/sailing/domain/tractracadapter/impl/DomainFactoryImpl.java @@ -310,7 +310,8 @@ public class DomainFactoryImpl implements DomainFactory { } // TODO bug2822: if only boat is to be updated then this would currently fail here; consider competitorStore.isBoatToUpdateDuringGetOrCreate(...) DynamicTeam team = createTeam(name, nationality, competitorId); - DynamicBoat boat = (DynamicBoat) competitorStore.getOrCreateBoat(UUID.randomUUID(), null /* no boat name available */, boatClass, sailId, /* color */ null); + DynamicBoat boat = (DynamicBoat) competitorStore.getOrCreateBoat(domainCompetitor == null ? UUID.randomUUID() : domainCompetitor.getBoat().getId(), + null /* no boat name available */, boatClass, sailId, /* color */ null); domainCompetitor = competitorStore.getOrCreateCompetitorWithBoat(competitorId, name, shortName, null /* displayColor */, null /* email */, null /* flagImag */, team, (double) timeOnTimeFactor, new MillisecondsDurationImpl((long) (timeOnDistanceAllowanceInSecondsPerNauticalMile*1000)), searchTag, (DynamicBoat) boat); diff --git a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/leaderboard/LeaderboardCacheManager.java b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/leaderboard/LeaderboardCacheManager.java index d1cd9ef8ff1..b624bc86790 100755 --- a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/leaderboard/LeaderboardCacheManager.java +++ b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/leaderboard/LeaderboardCacheManager.java @@ -9,6 +9,8 @@ import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import com.sap.sailing.domain.base.Boat; +import com.sap.sailing.domain.base.BoatChangeListener; import com.sap.sailing.domain.base.Competitor; import com.sap.sailing.domain.base.CompetitorChangeListener; import com.sap.sailing.domain.base.Fleet; @@ -44,6 +46,7 @@ public class LeaderboardCacheManager { private final WeakHashMap nameChangeListeners; private final NamedReentrantReadWriteLock scoreCorrectionAndCompetitorAndNameChangeListenersLock; private final WeakHashMap competitorChangeListeners; + private final WeakHashMap boatChangeListeners; private final ConcurrentWeakHashMap>> invalidationListenersPerLeaderboard; private final WeakHashMap raceColumnListeners; @@ -84,10 +87,52 @@ public class LeaderboardCacheManager { } } + private class CacheInvalidationUponBoatChangeListener implements BoatChangeListener { + private static final long serialVersionUID = -8117073993497852698L; + private final Leaderboard leaderboard; + private final Set boatsToStopObserving; + + public CacheInvalidationUponBoatChangeListener(Leaderboard leaderboard) { + super(); + this.leaderboard = leaderboard; + boatsToStopObserving = registerBoatListeners(); + } + + public Set registerBoatListeners() { + Set boatsToStopObserving = new HashSet<>(); + for (Boat boat : leaderboard.getAllBoats()) { + boat.addBoatChangeListener(this); // start observing boat + boatsToStopObserving.add(boat); + } + return boatsToStopObserving; + } + + public synchronized void removeFromAllBoats() { + for (Boat boat : boatsToStopObserving) { + boat.removeBoatChangeListener(this); + } + } + + @Override + public void nameChanged(String oldName, String newName) { + removeFromCache(leaderboard); + } + + @Override + public void colorChanged(Color oldColor, Color newColor) { + removeFromCache(leaderboard); + } + + @Override + public void sailIdChanged(String oldSailId, String newSailId) { + removeFromCache(leaderboard); + } + } + private class CacheInvalidationUponCompetitorChangeListener implements CompetitorChangeListener { private static final long serialVersionUID = 8308312509904366143L; private final Leaderboard leaderboard; - private final Set observedCompetitors; + private final Set competitorsToStopObserving; /** * Registers this listener as listener for all of the leaderboard's current @@ -95,8 +140,7 @@ public class LeaderboardCacheManager { */ public CacheInvalidationUponCompetitorChangeListener(Leaderboard leaderboard) { this.leaderboard = leaderboard; - observedCompetitors = new HashSet<>(); - updateCompetitorListeners(); + competitorsToStopObserving = registerCompetitorListeners(); } @Override @@ -119,23 +163,17 @@ public class LeaderboardCacheManager { removeFromCache(leaderboard); } - public synchronized void updateCompetitorListeners() { - Set competitorsToStopObserving = new HashSet<>(observedCompetitors); + public Set registerCompetitorListeners() { + Set competitorsToStopObserving = new HashSet<>(); for (Competitor competitor : leaderboard.getCompetitors()) { - if (!observedCompetitors.contains(competitor)) { - competitor.addCompetitorChangeListener(this); // start observing competitor - } else { - competitorsToStopObserving.remove(competitor); // keep observing competitor - } - } - for (Competitor competitorToStopObserving : competitorsToStopObserving) { - competitorToStopObserving.removeCompetitorChangeListener(this); - observedCompetitors.remove(competitorToStopObserving); + competitor.addCompetitorChangeListener(this); // start observing competitor + competitorsToStopObserving.add(competitor); } + return competitorsToStopObserving; } public synchronized void removeFromAllCompetitors() { - for (Competitor competitor : observedCompetitors) { + for (Competitor competitor : competitorsToStopObserving) { competitor.removeCompetitorChangeListener(this); } } @@ -218,6 +256,7 @@ public class LeaderboardCacheManager { this.scoreCorrectionAndCompetitorAndNameChangeListenersLock = new NamedReentrantReadWriteLock( "Score correction and competitor and name change listeners", /* fair */false); this.competitorChangeListeners = new WeakHashMap<>(); + this.boatChangeListeners = new WeakHashMap<>(); } private void removeFromCache(Leaderboard leaderboard) { @@ -237,10 +276,12 @@ public class LeaderboardCacheManager { LockUtil.lockForWrite(scoreCorrectionAndCompetitorAndNameChangeListenersLock); final CacheInvalidationUponScoreCorrectionListener removedScoreCorrectionListener; final CacheInvalidationUponCompetitorChangeListener removedCompetitorChangeListener; + final CacheInvalidationUponBoatChangeListener removedBoatChangeListener; final NameChangeListener removedNameChangeListener; try { removedScoreCorrectionListener = scoreCorrectionListeners.remove(leaderboard); removedCompetitorChangeListener = competitorChangeListeners.remove(leaderboard); + removedBoatChangeListener = boatChangeListeners.remove(leaderboard); removedNameChangeListener = nameChangeListeners.remove(leaderboard); } finally { LockUtil.unlockAfterWrite(scoreCorrectionAndCompetitorAndNameChangeListenersLock); @@ -251,6 +292,9 @@ public class LeaderboardCacheManager { if (removedCompetitorChangeListener != null) { removedCompetitorChangeListener.removeFromAllCompetitors(); } + if (removedBoatChangeListener != null) { + removedBoatChangeListener.removeFromAllBoats(); + } if (removedNameChangeListener != null) { leaderboard.removeLeaderboardChangeListener(removedNameChangeListener); } @@ -286,15 +330,15 @@ public class LeaderboardCacheManager { for (TrackedRace trackedRace : leaderboard.getTrackedRaces()) { registerListener(leaderboard, trackedRace); } - final CacheInvalidationUponScoreCorrectionListener scoreCorrectionListener = new CacheInvalidationUponScoreCorrectionListener( - leaderboard); - final CacheInvalidationUponCompetitorChangeListener competitorChangeListener = new CacheInvalidationUponCompetitorChangeListener( - leaderboard); + final CacheInvalidationUponScoreCorrectionListener scoreCorrectionListener = new CacheInvalidationUponScoreCorrectionListener(leaderboard); + final CacheInvalidationUponCompetitorChangeListener competitorChangeListener = new CacheInvalidationUponCompetitorChangeListener(leaderboard); + final CacheInvalidationUponBoatChangeListener boatChangeListener = new CacheInvalidationUponBoatChangeListener(leaderboard); final NameChangeListener nameChangeListener = new NameChangeListener(leaderboard); leaderboard.addScoreCorrectionListener(scoreCorrectionListener); leaderboard.addLeaderboardChangeListener(nameChangeListener); scoreCorrectionListeners.put(leaderboard, scoreCorrectionListener); competitorChangeListeners.put(leaderboard, competitorChangeListener); + boatChangeListeners.put(leaderboard, boatChangeListener); nameChangeListeners.put(leaderboard, nameChangeListener); final RaceColumnListener raceColumnListener = new RaceColumnListenerWithDefaultAction() { private static final long serialVersionUID = 8165124797028386317L; 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 2036eb3334b..ed522291f6c 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 @@ -5174,7 +5174,6 @@ public class SailingServiceImpl extends ProxiedRemoteServiceServlet implements S DynamicTeam team = new TeamImpl(competitor.getName() + " team", Collections.singleton(sailor), null); // new boat DynamicBoat boat = (DynamicBoat) addOrUpdateBoatInternal(competitor.getBoat()); - result = getBaseDomainFactory().getOrCreateCompetitorWithBoat(competitorUUID, competitor.getName(), competitor.getShortName(), competitor.getColor(), competitor.getEmail(), competitor.getFlagImageURL() == null ? null : new URI(competitor.getFlagImageURL()), team, diff --git a/java/com.sap.sailing.server/src/com/sap/sailing/server/impl/PersistentCompetitorAndBoatStore.java b/java/com.sap.sailing.server/src/com/sap/sailing/server/impl/PersistentCompetitorAndBoatStore.java index 051963b06f5..d9ba4499c99 100755 --- a/java/com.sap.sailing.server/src/com/sap/sailing/server/impl/PersistentCompetitorAndBoatStore.java +++ b/java/com.sap.sailing.server/src/com/sap/sailing/server/impl/PersistentCompetitorAndBoatStore.java @@ -165,9 +165,9 @@ public class PersistentCompetitorAndBoatStore extends TransientCompetitorAndBoat @Override public CompetitorWithBoat updateCompetitorWithBoat(String idAsString, String newName, String newShortName, Color newRgbDisplayColor, String newEmail, Nationality newNationality, URI newTeamImageUri, URI newFlagImageUri, Double timeOnTimeFactor, Duration timeOnDistanceAllowancePerNauticalMile, - String searchTag, DynamicBoat boat) { + String searchTag) { CompetitorWithBoat result = super.updateCompetitorWithBoat(idAsString, newName, newShortName, newRgbDisplayColor, newEmail, newNationality, - newTeamImageUri, newFlagImageUri, timeOnTimeFactor, timeOnDistanceAllowancePerNauticalMile, searchTag, boat); + newTeamImageUri, newFlagImageUri, timeOnTimeFactor, timeOnDistanceAllowancePerNauticalMile, searchTag); storeTo.storeCompetitor(result); return result; } diff --git a/java/com.sap.sailing.server/src/com/sap/sailing/server/operationaltransformation/AllowCompetitorResetToDefaults.java b/java/com.sap.sailing.server/src/com/sap/sailing/server/operationaltransformation/AllowCompetitorResetToDefaults.java index 2c097b7ae42..79087617707 100755 --- a/java/com.sap.sailing.server/src/com/sap/sailing/server/operationaltransformation/AllowCompetitorResetToDefaults.java +++ b/java/com.sap.sailing.server/src/com/sap/sailing/server/operationaltransformation/AllowCompetitorResetToDefaults.java @@ -4,6 +4,7 @@ import java.util.ArrayList; import com.sap.sailing.domain.base.Competitor; import com.sap.sailing.domain.base.CompetitorAndBoatStore; +import com.sap.sailing.domain.base.impl.DynamicCompetitorWithBoat; import com.sap.sailing.server.RacingEventService; import com.sap.sailing.server.RacingEventServiceOperation; import com.sap.sse.common.Util; @@ -26,6 +27,9 @@ public class AllowCompetitorResetToDefaults extends AbstractRacingEventServiceOp Competitor competitor = competitorStore.getExistingCompetitorByIdAsString(competitorIdAsString); if (competitor != null) { competitorStore.allowCompetitorResetToDefaults(competitor); + if (competitor.hasBoat()) { + competitorStore.allowBoatResetToDefaults(((DynamicCompetitorWithBoat) competitor).getBoat()); + } } } return null;