From 0e0e65fb8af9cc0caf4af98f7e63a60a04642b23 Mon Sep 17 00:00:00 2001 From: Jonas Dann Date: Thu, 30 Nov 2017 13:27:32 +0000 Subject: [PATCH 1/9] Updated permission concept (markdown) --- wiki/info/security/permission-concept.md | 33 ++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/wiki/info/security/permission-concept.md b/wiki/info/security/permission-concept.md index 965ae52798c..aeff1626c06 100644 --- a/wiki/info/security/permission-concept.md +++ b/wiki/info/security/permission-concept.md @@ -201,3 +201,36 @@ With such an extensive existing system as the Sailing Analytics Suite, migration Another challenge besides the code changes is the data migration. For every existing data object an ACL has to be created and filled with the right permissions so the users do not notice a big change. Besides creating an ACL for every data objects that is access controlled, an owner has to be defined for each existing data object, so that in combination with the ACLs no user loses permissions they need to have. In order to do this, where will have to be a script that associates all data objects on a server with a tenant and a specific user as owner (e.g. on the archive all data objects are associated with the tenant “archive” and owned by the user “Axel”). Thereafter, the users on the server are assigned their respective role. + +**Role Migration** + +Following roles have to be migrated to dynamic roles. Beforehand they were hard coded in the SailingPermissionsForRoleProvider class. However, aside from the admin role, all roles listed here are only containing permissions that are checked in the frontend for showing/hiding tabs in the different menus. + +* admin + * "*" +* eventmanager + * "manage_media" + * "manage_mark_passings" + * "manage_mark_positions" + * "manage_all_competitors" + * "manage_course_layout" + * "manage_device_configuration" + * "manage_events" + * "manage_igtimi_accounts" + * "manage_leaderboard_groups" + * "manage_leaderboards" + * "manage_leaderboard_results" + * "manage_racelog_tracking" + * "manage_regattas" + * "manage_result_import_urls" + * "manage_structure_import_urls" + * "manage_tracked_races" + * "manage_wind" + * "event" + * "regatta" + * "leaderboard" + * "leaderboard_group" +* mediaeditor + * "manage_media" +* moderator + * "can_replay_during_live_races" From 36204b2ef115ab7d4fd24b420621079d6d9bf92a Mon Sep 17 00:00:00 2001 From: Jonas Dann Date: Thu, 30 Nov 2017 16:31:32 +0000 Subject: [PATCH 2/9] Updated permission concept (markdown) --- wiki/info/security/permission-concept.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/wiki/info/security/permission-concept.md b/wiki/info/security/permission-concept.md index aeff1626c06..387b164d892 100644 --- a/wiki/info/security/permission-concept.md +++ b/wiki/info/security/permission-concept.md @@ -234,3 +234,6 @@ Following roles have to be migrated to dynamic roles. Beforehand they were hard * "manage_media" * moderator * "can_replay_during_live_races" + +## Implementation + From 841f1daf94545f96f16bc43e73149075ab3f666f Mon Sep 17 00:00:00 2001 From: Frank Mittag Date: Thu, 30 Nov 2017 16:36:40 +0000 Subject: [PATCH 3/9] Updated bug2822 review (markdown) --- wiki/bug2822-review.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/wiki/bug2822-review.md b/wiki/bug2822-review.md index 32490a55d21..9b20fe6178a 100644 --- a/wiki/bug2822-review.md +++ b/wiki/bug2822-review.md @@ -10,7 +10,8 @@ Bug 2822 changes the way competitors and boats are handled. Specifically the 1:1 Check: -- Existing COMPETITORS Collection is migrated to COMPETITORS_WITH_BOAT_REFERENCES and BOATS collection +- Existing COMPETITORS Collection is renamed to COMPETITORS_BAK collection +- Competitors from COMPETITORS_BAK collection will be migrated to COMPETITORS and BOATS collection - All migrated competitors have a link to a boat from boats collection - The attribute 'name' is cleared in new all created boat objects as the old names were not meaningful and not used @@ -21,7 +22,7 @@ In case of a server instance with already created competitors we don't know to w Check: - Before server start an existing competitor with emedded boat does exist in the COMPETITORS collection -- After server start and migration according to 1.1 this competitor does exist in the COMPETITORS_WITH_BOAT_REFERENCES collection and does have a link to a boat in the BOATS collection +- After server start and migration according to 1.1 this competitor does exist in the COMPETITORS collection and does have a link to a boat in the BOATS collection - After starting the tractrac tracking of a race where this competitor is contained the tractrac connector recognizes the need for the migration and calls the corresponding function on the CompetitorStore - After the migration the competitor does not have a boat reference anymore and the migrated boat has the right properties From c7f50c389ffd3ec1250845e84bb93bfeeb061638 Mon Sep 17 00:00:00 2001 From: Jonas Dann Date: Thu, 30 Nov 2017 16:46:04 +0000 Subject: [PATCH 4/9] Updated permission concept (markdown) --- wiki/info/security/permission-concept.md | 29 +++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/wiki/info/security/permission-concept.md b/wiki/info/security/permission-concept.md index 387b164d892..c18a528f104 100644 --- a/wiki/info/security/permission-concept.md +++ b/wiki/info/security/permission-concept.md @@ -16,6 +16,8 @@ * [Use Cases](#use cases) * [Algorithm for Composite Realm](#algorithm-bool-haspermission-wildcardpermission-permission-for-composite-realm) * [Migration](#migration) +* [Implementation Details](#implementation-details) +* [TODOs](#todos) ## Introduction @@ -235,5 +237,30 @@ Following roles have to be migrated to dynamic roles. Beforehand they were hard * moderator * "can_replay_during_live_races" -## Implementation +## Implementation Details + + +## TODOs + +Dump of TODOs. More structured in Bugzilla. + +* Ownership/Access control list as expandable column in all access controlled data object tables +* Permission checking for every domain data object type +* Changing ownership (incl. replication) +* Masterdataimport +* Tenant management UI + * Tenant in create dialog check if tenant exists (red outline of text input field) + * Refresh tenant selection in tenant list on the left correctly + * String messages +* User management UI + * Auto complete for roles and permissions +* Access control list / Roles / Ownership UI +* User role for editing own user parametrized by user name +* Do we want to throw unauthorized exceptions or how should this work? (See addUserToTenant or removeUserFromTenant) / UnauthorizedException handling (i.e. popup that shows some text) +* Create ownerships from config / Server-wide default owner and default tenant owner +* Create test setup for final test +* Button to transfer ownership of all objects from one user/tenant to another user/tenant (chgroup recursive?) +* Rework meta permissions (rework to also check all permissions that are granted or revoked?) +* Permission reloading without site refresh / Frontend update elements that require permission when permissions change (Refresh call on panels) +* Default tenant input field in login popup From faf47f9d17764da57d64631738a968342fd0c45e Mon Sep 17 00:00:00 2001 From: Jonas Dann Date: Thu, 30 Nov 2017 17:04:33 +0000 Subject: [PATCH 5/9] Updated permission concept (markdown) --- wiki/info/security/permission-concept.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wiki/info/security/permission-concept.md b/wiki/info/security/permission-concept.md index c18a528f104..f2c4fa8aead 100644 --- a/wiki/info/security/permission-concept.md +++ b/wiki/info/security/permission-concept.md @@ -239,7 +239,7 @@ Following roles have to be migrated to dynamic roles. Beforehand they were hard ## Implementation Details - +Access control relevant objects are stored in the AccessControlStore, while user related objects are stored in the user, i.e. the UserStore. Tenants are currently stored as UserGroups in the user store. The algorithm outlined above is implemented in the PermissionChecker, which is one of the most central classes in the permission system. Therefrom it should be possible to find all other relevant classes. Other relevant classes include the AbstractCompositeAuthorizingRealm that implements permission checking in Shiro. Therefor it uses the PermissionChecker and is connected to the UserStore and the AccessControlStore. Furthermore the RolePermissionModel implements how roles imply permissions. The RolePermissionModel as well as the parameterization of roles need more work and are just a rough sketch. The UserManagementService exposes most of the access control relevant parts to the frontend. It works through the SecurityService with the AccessControlStore and the UserStore. Permission checking in the frontend is also done with the PermissionChecker. To ease the creation of WildcardPermissions, a PermissionBuilder was introduced that also needs some more work. All access control relevant UI parts can be found in the "Advanced" tab under "User management" and "Tenant management". A further tab "Access Control Management" should be introduced. Unit tests can be found in the package com.sap.sse.security.test. ## TODOs From 0be5519db933b4a646a2ec556e71027065ae5959 Mon Sep 17 00:00:00 2001 From: Simon Pamies Date: Fri, 1 Dec 2017 18:51:36 +0100 Subject: [PATCH 6/9] Updated HttpUrlConnectionHelper to support setting a longer read timeout. This is especially useful when expecting responses that are very big and need a long time to compute on the client. --- .../sailing/gwt/ui/server/SailingServiceImpl.java | 3 ++- .../com/sap/sse/util/HttpUrlConnectionHelper.java | 13 ++++++++++--- 2 files changed, 12 insertions(+), 4 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 cf84f828d7f..d8da768a915 100644 --- 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 @@ -4815,7 +4815,8 @@ public class SailingServiceImpl extends ProxiedRemoteServiceServlet implements S URL base = createBaseUrl(urlAsString); String path = "/sailingserver/spi/v1/masterdata/leaderboardgroups"; serverAddress = createUrl(base, path, query); - connection = HttpUrlConnectionHelper.redirectConnection(serverAddress); + // the response can take a very long time for MDI that include foiling data or such + connection = HttpUrlConnectionHelper.redirectConnection(serverAddress, Duration.ONE_HOUR.times(2)); getService().createOrUpdateDataImportProgressWithReplication(importOperationId, 0.02, DataImportSubProgress.CONNECTION_ESTABLISH, 0.5); if (compress) { diff --git a/java/com.sap.sse/src/com/sap/sse/util/HttpUrlConnectionHelper.java b/java/com.sap.sse/src/com/sap/sse/util/HttpUrlConnectionHelper.java index 777c1447674..49d5020371c 100644 --- a/java/com.sap.sse/src/com/sap/sse/util/HttpUrlConnectionHelper.java +++ b/java/com.sap.sse/src/com/sap/sse/util/HttpUrlConnectionHelper.java @@ -10,7 +10,11 @@ import com.sap.sse.common.Duration; public class HttpUrlConnectionHelper { private static final int HTTP_MAX_REDIRECTS = 5; - public static HttpURLConnection redirectConnection(URL url) throws MalformedURLException, IOException { + /** + * Redirects the connection using the Location header. Make sure to set + * the timeout if you expect the response to take longer. + */ + public static HttpURLConnection redirectConnection(URL url, Duration timeout) throws MalformedURLException, IOException { HttpURLConnection connection = null; URL nextUrl = url; for (int counterOfRedirects = 0; counterOfRedirects <= HTTP_MAX_REDIRECTS; counterOfRedirects++) { @@ -21,8 +25,7 @@ public class HttpUrlConnectionHelper { connection.setInstanceFollowRedirects(false); connection.setRequestProperty("User-Agent", "Mozilla/5.0..."); connection.setDoOutput(true); - // Initial timeout needs to be big enough to allow the first parts of the response to reach this server - connection.setReadTimeout((int) Duration.ONE_MINUTE.times(10).asMillis()); + connection.setReadTimeout((int) timeout.asMillis()); if (connection.getResponseCode() == HttpURLConnection.HTTP_MOVED_PERM || connection.getResponseCode() == HttpURLConnection.HTTP_MOVED_TEMP) { String location = connection.getHeaderField("Location"); @@ -33,4 +36,8 @@ public class HttpUrlConnectionHelper { } return connection; } + + public static HttpURLConnection redirectConnection(URL url) throws MalformedURLException, IOException { + return redirectConnection(url, Duration.ONE_MINUTE.times(10)); + } } From dc24184baa0f6f68a199030155b09eea4d1d7531 Mon Sep 17 00:00:00 2001 From: Axel Uhl Date: Sun, 3 Dec 2017 13:56:35 +0100 Subject: [PATCH 7/9] bug 4387: notify listeners about sensor track addition outside of write lock Change-Id: I1a6f8630f381848acd579787e9a8022f10438110 --- .../tracking/impl/DynamicTrackedRaceImpl.java | 7 ++--- .../domain/tracking/impl/TrackedRaceImpl.java | 26 ++++++++++++++----- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/DynamicTrackedRaceImpl.java b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/DynamicTrackedRaceImpl.java index 66312ec0b68..1639d123330 100644 --- a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/DynamicTrackedRaceImpl.java +++ b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/DynamicTrackedRaceImpl.java @@ -13,6 +13,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.NavigableSet; +import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Level; @@ -609,7 +610,7 @@ DynamicTrackedRace, GPSTrackListener { notifyListeners(listener -> listener.markPassingReceived(competitor, oldMarkPassings, markPassings)); } - private void notifyListeners(DynamicSensorFixTrack track) { + private void notifyListenersAboutSensorTrackAdded(DynamicSensorFixTrack track) { notifyListeners(listener -> listener.competitorSensorTrackAdded(track)); } @@ -1206,7 +1207,7 @@ DynamicTrackedRace, GPSTrackListener { } @Override - protected void addSensorTrackInternal(Pair key, + protected Optional addSensorTrackInternal(Pair key, DynamicSensorFixTrack track) { super.addSensorTrackInternal(key, track); track.addListener(new SensorFixTrackListener() { @@ -1222,6 +1223,6 @@ DynamicTrackedRace, GPSTrackListener { notifyListeners(item, trackName, fix); } }); - notifyListeners(track); + return Optional.of(()->notifyListenersAboutSensorTrackAdded(track)); } } diff --git a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/TrackedRaceImpl.java b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/TrackedRaceImpl.java index 3767441eff5..c33750f7576 100755 --- a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/TrackedRaceImpl.java +++ b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/TrackedRaceImpl.java @@ -19,6 +19,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.NavigableSet; +import java.util.Optional; import java.util.Set; import java.util.Timer; import java.util.TimerTask; @@ -4147,40 +4148,53 @@ public abstract class TrackedRaceImpl extends TrackedRaceWithWindEssentials impl protected > TrackT getOrCreateSensorTrack( Competitor competitor, String trackName, TrackFactory newTrackFactory) { Pair key = new Pair<>(competitor, trackName); + Optional executeAfterReleasingLock = Optional.empty(); + TrackT result; LockUtil.lockForWrite(sensorTracksLock); try { - TrackT result = getTrackInternal(key); + result = getTrackInternal(key); if (result == null && tracks.containsKey(competitor)) { // A track is only added if the given Competitor is known to participate in this race result = newTrackFactory.get(); - addSensorTrackInternal(key, result); + executeAfterReleasingLock = addSensorTrackInternal(key, result); } - return result; } finally { LockUtil.unlockAfterWrite(sensorTracksLock); } + executeAfterReleasingLock.ifPresent(r->r.run()); + return result; } protected void addSensorTrack(Competitor competitor, String trackName, DynamicSensorFixTrack track) { Pair key = new Pair<>(competitor, trackName); + Optional executeAfterReleasingLock = Optional.empty(); LockUtil.lockForWrite(sensorTracksLock); try { - if(getTrackInternal(key) != null) { + if (getTrackInternal(key) != null) { if (logger != null && logger.getLevel() != null && logger.getLevel().equals(Level.WARNING)) { logger.warning(SensorFixTrack.class.getName() + " already exists for competitor: " + competitor.getName() + "; trackName: " + trackName); } } else { - this.addSensorTrackInternal(key, track); + executeAfterReleasingLock = this.addSensorTrackInternal(key, track); } } finally { LockUtil.unlockAfterWrite(sensorTracksLock); } + executeAfterReleasingLock.ifPresent(r->r.run()); } - protected void addSensorTrackInternal(Pair key, + /** + * To call this method, the caller must have obtained the write lock of {@link #sensorTracksLock}. + * Optionally, the method may return a {@link Runnable} to execute after the lock has been released. + * This may, e.g., be a routine that notifies listeners. Callers are responsible for invoking this + * {@link Runnable} after releasing the write lock. + */ + protected Optional addSensorTrackInternal(Pair key, DynamicSensorFixTrack track) { + assert sensorTracksLock.isWriteLockedByCurrentThread(); sensorTracks.put(key, track); + return Optional.empty(); } @SuppressWarnings("unchecked") From 28ffdac19f9180a35759a9d39de75253d7b90d8e Mon Sep 17 00:00:00 2001 From: Axel Uhl Date: Sun, 3 Dec 2017 15:07:02 +0100 Subject: [PATCH 8/9] bug 4387: fix for comment #4, making gpsTrack transient on BravoFixTrackImpl, filling when track is added to TrackedRace Change-Id: Icba632c7cf033aeadf7db8cffaa1010be6183e69 --- .../impl/BravoDataFixMapper.java | 4 +-- .../impl/BravoExtendedDataFixMapper.java | 4 +-- .../impl/ExpeditionExtendedDataFixMapper.java | 4 +-- .../tracking/DynamicSensorFixTrack.java | 6 +++- .../tracking/impl/BravoFixTrackImpl.java | 28 ++++++++++++++----- .../impl/CompetitorBravoFixTrackImpl.java | 26 +++++++++++++++++ .../domain/tracking/impl/TrackedRaceImpl.java | 1 + 7 files changed, 59 insertions(+), 14 deletions(-) create mode 100755 java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/CompetitorBravoFixTrackImpl.java diff --git a/java/com.sap.sailing.domain.racelogtrackingadapter/src/com/sap/sailing/domain/racelogtracking/impl/BravoDataFixMapper.java b/java/com.sap.sailing.domain.racelogtrackingadapter/src/com/sap/sailing/domain/racelogtracking/impl/BravoDataFixMapper.java index 3415d44f166..da5934327e4 100644 --- a/java/com.sap.sailing.domain.racelogtrackingadapter/src/com/sap/sailing/domain/racelogtracking/impl/BravoDataFixMapper.java +++ b/java/com.sap.sailing.domain.racelogtrackingadapter/src/com/sap/sailing/domain/racelogtracking/impl/BravoDataFixMapper.java @@ -10,7 +10,7 @@ import com.sap.sailing.domain.racelog.tracking.SensorFixMapper; import com.sap.sailing.domain.tracking.BravoFixTrack; import com.sap.sailing.domain.tracking.DynamicSensorFixTrack; import com.sap.sailing.domain.tracking.DynamicTrackedRace; -import com.sap.sailing.domain.tracking.impl.BravoFixTrackImpl; +import com.sap.sailing.domain.tracking.impl.CompetitorBravoFixTrackImpl; /** * {@link SensorFixMapper} implementation to handle {@link BravoFix}es. @@ -20,7 +20,7 @@ public class BravoDataFixMapper implements SensorFixMapper getTrack(DynamicTrackedRace race, Competitor key) { return race.getOrCreateSensorTrack(key, BravoFixTrack.TRACK_NAME, - () -> new BravoFixTrackImpl(key, BravoFixTrack.TRACK_NAME, /* hasExtendedFixes */ false, + () -> new CompetitorBravoFixTrackImpl(key, BravoFixTrack.TRACK_NAME, /* hasExtendedFixes */ false, race.getTrack(key))); } diff --git a/java/com.sap.sailing.domain.racelogtrackingadapter/src/com/sap/sailing/domain/racelogtracking/impl/BravoExtendedDataFixMapper.java b/java/com.sap.sailing.domain.racelogtrackingadapter/src/com/sap/sailing/domain/racelogtracking/impl/BravoExtendedDataFixMapper.java index f35509b8ec3..15f96e22bc9 100644 --- a/java/com.sap.sailing.domain.racelogtrackingadapter/src/com/sap/sailing/domain/racelogtracking/impl/BravoExtendedDataFixMapper.java +++ b/java/com.sap.sailing.domain.racelogtrackingadapter/src/com/sap/sailing/domain/racelogtracking/impl/BravoExtendedDataFixMapper.java @@ -11,7 +11,7 @@ import com.sap.sailing.domain.racelog.tracking.SensorFixMapper; import com.sap.sailing.domain.tracking.BravoFixTrack; import com.sap.sailing.domain.tracking.DynamicSensorFixTrack; import com.sap.sailing.domain.tracking.DynamicTrackedRace; -import com.sap.sailing.domain.tracking.impl.BravoFixTrackImpl; +import com.sap.sailing.domain.tracking.impl.CompetitorBravoFixTrackImpl; /** * {@link SensorFixMapper} implementation to handle {@link BravoExtendedFix}es. @@ -21,7 +21,7 @@ public class BravoExtendedDataFixMapper implements SensorFixMapper getTrack(DynamicTrackedRace race, Competitor key) { return race.getOrCreateSensorTrack(key, BravoFixTrack.TRACK_NAME, - () -> new BravoFixTrackImpl(key, BravoFixTrack.TRACK_NAME, /* hasExtendedFixes */ true, + () -> new CompetitorBravoFixTrackImpl(key, BravoFixTrack.TRACK_NAME, /* hasExtendedFixes */ true, race.getTrack(key))); } diff --git a/java/com.sap.sailing.domain.racelogtrackingadapter/src/com/sap/sailing/domain/racelogtracking/impl/ExpeditionExtendedDataFixMapper.java b/java/com.sap.sailing.domain.racelogtrackingadapter/src/com/sap/sailing/domain/racelogtracking/impl/ExpeditionExtendedDataFixMapper.java index 5dcf9db8119..09405591969 100755 --- a/java/com.sap.sailing.domain.racelogtrackingadapter/src/com/sap/sailing/domain/racelogtracking/impl/ExpeditionExtendedDataFixMapper.java +++ b/java/com.sap.sailing.domain.racelogtrackingadapter/src/com/sap/sailing/domain/racelogtracking/impl/ExpeditionExtendedDataFixMapper.java @@ -11,7 +11,7 @@ import com.sap.sailing.domain.racelog.tracking.SensorFixMapper; import com.sap.sailing.domain.tracking.BravoFixTrack; import com.sap.sailing.domain.tracking.DynamicSensorFixTrack; import com.sap.sailing.domain.tracking.DynamicTrackedRace; -import com.sap.sailing.domain.tracking.impl.BravoFixTrackImpl; +import com.sap.sailing.domain.tracking.impl.CompetitorBravoFixTrackImpl; /** * {@link SensorFixMapper} implementation to product {@link BravoExtendedFix}es from {@link DoubleVectorFix}es @@ -25,7 +25,7 @@ public class ExpeditionExtendedDataFixMapper implements SensorFixMapper getTrack(DynamicTrackedRace race, Competitor key) { return race.getOrCreateSensorTrack(key, BravoFixTrack.TRACK_NAME, - () -> new BravoFixTrackImpl(key, BravoFixTrack.TRACK_NAME, /* hasExtendedFixes */ true, + () -> new CompetitorBravoFixTrackImpl(key, BravoFixTrack.TRACK_NAME, /* hasExtendedFixes */ true, race.getTrack(key))); } diff --git a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/DynamicSensorFixTrack.java b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/DynamicSensorFixTrack.java index e6dd60f720c..86afdb43785 100644 --- a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/DynamicSensorFixTrack.java +++ b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/DynamicSensorFixTrack.java @@ -13,5 +13,9 @@ import com.sap.sse.common.WithID; */ public interface DynamicSensorFixTrack extends SensorFixTrack, DynamicTrack { - + /** + * Invoked when this track is added to a tracked race. This default implementation + * does nothing. + */ + default void addedToTrackedRace(TrackedRace trackedRace) {} } diff --git a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/BravoFixTrackImpl.java b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/BravoFixTrackImpl.java index bb6e27ca4c1..f05020eefd6 100644 --- a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/BravoFixTrackImpl.java +++ b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/BravoFixTrackImpl.java @@ -45,7 +45,7 @@ public class BravoFixTrackImpl extends S * If a GPS track was provided at construction time, remember it non-transiently. It is needed when restoring * the object after de-serialization, so the cache invalidation listener can be re-established. */ - private GPSFixTrack gpsTrack; + private transient GPSFixTrack gpsTrack; private class CacheInvalidationGpsTrackListener implements GPSTrackListener { private static final long serialVersionUID = 6395529765232404414L; @@ -89,6 +89,23 @@ public class BravoFixTrackImpl extends S this.foilingTimeCache = createFoilingTimeCache(trackedItem); this.foilingDistanceCache = createFoilingDistanceCache(trackedItem); this.averageRideHeightCache = createAverageRideHeightCache(trackedItem); + setGpsTrack(gpsTrack); + } + + protected GPSFixTrack getGpsTrack() { + return gpsTrack; + } + + /** + * Sets the {@link #gpsTrack} field and registers a cache invalidation listener + * on the {@code gpsTrack} to ensure that the foiling distance is invalidated if needed.

+ * + * Must be called only once for a non-{@code null} {@code gpsTrack}. + */ + protected void setGpsTrack(GPSFixTrack gpsTrack) { + if (gpsTrack != null) { + assert this.gpsTrack == null; + } this.gpsTrack = gpsTrack; if (gpsTrack != null) { gpsTrack.addListener(new CacheInvalidationGpsTrackListener()); @@ -119,9 +136,6 @@ public class BravoFixTrackImpl extends S this.foilingTimeCache = createFoilingTimeCache(getTrackedItem()); this.foilingDistanceCache = createFoilingDistanceCache(getTrackedItem()); this.averageRideHeightCache = createAverageRideHeightCache(getTrackedItem()); - if (gpsTrack != null) { - gpsTrack.addListener(new CacheInvalidationGpsTrackListener()); - } } @Override @@ -215,8 +229,8 @@ public class BravoFixTrackImpl extends S @Override public Distance getDistanceSpentFoiling(TimePoint from, TimePoint to) { - assert gpsTrack != null; - return getValueSum(from, to, /* nullElement */ Distance.NULL, Distance::add, foilingDistanceCache, + return getGpsTrack() == null ? null : + getValueSum(from, to, /* nullElement */ Distance.NULL, Distance::add, foilingDistanceCache, /* valueCalculator */ new Track.TimeRangeValueCalculator() { @Override public Distance calculate(TimePoint from, TimePoint to) { @@ -226,7 +240,7 @@ public class BravoFixTrackImpl extends S for (final BravoFix fix : getFixes(from, true, to, true)) { final boolean fixFoils = isFoiling(fix); if (isFoiling && fixFoils) { - result = result.add(gpsTrack.getDistanceTraveled(last, fix.getTimePoint())); + result = result.add(getGpsTrack().getDistanceTraveled(last, fix.getTimePoint())); } last = fix.getTimePoint(); isFoiling = fixFoils; diff --git a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/CompetitorBravoFixTrackImpl.java b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/CompetitorBravoFixTrackImpl.java new file mode 100755 index 00000000000..261e1393c4e --- /dev/null +++ b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/CompetitorBravoFixTrackImpl.java @@ -0,0 +1,26 @@ +package com.sap.sailing.domain.tracking.impl; + +import com.sap.sailing.domain.base.Competitor; +import com.sap.sailing.domain.common.tracking.GPSFixMoving; +import com.sap.sailing.domain.tracking.GPSFixTrack; +import com.sap.sailing.domain.tracking.TrackedRace; + +public class CompetitorBravoFixTrackImpl extends BravoFixTrackImpl { + private static final long serialVersionUID = 569731925060211875L; + + public CompetitorBravoFixTrackImpl(Competitor trackedItem, String trackName, boolean hasExtendedFixes, + GPSFixTrack gpsTrack) { + super(trackedItem, trackName, hasExtendedFixes, gpsTrack); + } + + public CompetitorBravoFixTrackImpl(Competitor trackedItem, String trackName, boolean hasExtendedFixes) { + super(trackedItem, trackName, hasExtendedFixes); + } + + @Override + public void addedToTrackedRace(TrackedRace trackedRace) { + if (this.getGpsTrack() == null) { + this.setGpsTrack(trackedRace.getTrack(getTrackedItem())); + } + } +} diff --git a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/TrackedRaceImpl.java b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/TrackedRaceImpl.java index c33750f7576..934a2c1ffdb 100755 --- a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/TrackedRaceImpl.java +++ b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/TrackedRaceImpl.java @@ -4194,6 +4194,7 @@ public abstract class TrackedRaceImpl extends TrackedRaceWithWindEssentials impl DynamicSensorFixTrack track) { assert sensorTracksLock.isWriteLockedByCurrentThread(); sensorTracks.put(key, track); + track.addedToTrackedRace(this); return Optional.empty(); } From 2e071764102a6e102e2a864fe77866873ff7255e Mon Sep 17 00:00:00 2001 From: Axel Uhl Date: Sun, 3 Dec 2017 15:22:14 +0100 Subject: [PATCH 9/9] added failing test case for bug 4387 Change-Id: I5059f3cc94db585fa3a5da64ac50a285b13a1e6f --- .../tracking/DynamicSensorFixTrack.java | 5 ----- .../sailing/domain/tracking/DynamicTrack.java | 6 +++++ .../tracking/impl/BravoFixTrackImpl.java | 2 +- .../domain/tracking/impl/TrackedRaceImpl.java | 3 +++ .../TrackedRaceContentsReplicationTest.java | 22 +++++++++++++++++++ 5 files changed, 32 insertions(+), 6 deletions(-) diff --git a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/DynamicSensorFixTrack.java b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/DynamicSensorFixTrack.java index 86afdb43785..04ae5bd1059 100644 --- a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/DynamicSensorFixTrack.java +++ b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/DynamicSensorFixTrack.java @@ -13,9 +13,4 @@ import com.sap.sse.common.WithID; */ public interface DynamicSensorFixTrack extends SensorFixTrack, DynamicTrack { - /** - * Invoked when this track is added to a tracked race. This default implementation - * does nothing. - */ - default void addedToTrackedRace(TrackedRace trackedRace) {} } diff --git a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/DynamicTrack.java b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/DynamicTrack.java index 970e9fbf522..e857186da0d 100755 --- a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/DynamicTrack.java +++ b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/DynamicTrack.java @@ -25,4 +25,10 @@ public interface DynamicTrack extends Track { * @return true if the element was added, false otherwise. */ boolean add(FixType fix, boolean replace); + + /** + * Invoked when this track is added to a tracked race. The method is also invoked after de-serializing the + * containing {@link TrackedRace}. This default implementation does nothing. + */ + default void addedToTrackedRace(TrackedRace trackedRace) {} } diff --git a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/BravoFixTrackImpl.java b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/BravoFixTrackImpl.java index f05020eefd6..43f7fd881b4 100644 --- a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/BravoFixTrackImpl.java +++ b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/BravoFixTrackImpl.java @@ -92,7 +92,7 @@ public class BravoFixTrackImpl extends S setGpsTrack(gpsTrack); } - protected GPSFixTrack getGpsTrack() { + public GPSFixTrack getGpsTrack() { return gpsTrack; } diff --git a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/TrackedRaceImpl.java b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/TrackedRaceImpl.java index 934a2c1ffdb..d70a1e0f8a5 100755 --- a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/TrackedRaceImpl.java +++ b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/TrackedRaceImpl.java @@ -636,6 +636,9 @@ public abstract class TrackedRaceImpl extends TrackedRaceWithWindEssentials impl private void readObject(ObjectInputStream ois) throws ClassNotFoundException, IOException, PatchFailedException { ois.defaultReadObject(); getRace().getCourse().addCourseListener(this); + for (DynamicTrack sensorTrack : sensorTracks.values()) { + sensorTrack.addedToTrackedRace(this); + } raceStates = new WeakHashMap<>(); attachedRaceLogs = new ConcurrentHashMap<>(); attachedRegattaLogs = new ConcurrentHashMap<>(); diff --git a/java/com.sap.sailing.server.replication.test/src/com/sap/sailing/server/replication/test/TrackedRaceContentsReplicationTest.java b/java/com.sap.sailing.server.replication.test/src/com/sap/sailing/server/replication/test/TrackedRaceContentsReplicationTest.java index ddc6f533c46..3a082388195 100755 --- a/java/com.sap.sailing.server.replication.test/src/com/sap/sailing/server/replication/test/TrackedRaceContentsReplicationTest.java +++ b/java/com.sap.sailing.server.replication.test/src/com/sap/sailing/server/replication/test/TrackedRaceContentsReplicationTest.java @@ -3,6 +3,7 @@ package com.sap.sailing.server.replication.test; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; import java.net.UnknownHostException; import java.util.ArrayList; @@ -45,11 +46,13 @@ import com.sap.sailing.domain.persistence.MongoWindStore; import com.sap.sailing.domain.persistence.MongoWindStoreFactory; import com.sap.sailing.domain.persistence.PersistenceFactory; import com.sap.sailing.domain.test.PositionAssert; +import com.sap.sailing.domain.tracking.BravoFixTrack; import com.sap.sailing.domain.tracking.DynamicTrackedRace; import com.sap.sailing.domain.tracking.DynamicTrackedRegatta; import com.sap.sailing.domain.tracking.GPSFixTrack; import com.sap.sailing.domain.tracking.TrackedRace; import com.sap.sailing.domain.tracking.WindTrack; +import com.sap.sailing.domain.tracking.impl.CompetitorBravoFixTrackImpl; import com.sap.sailing.server.operationaltransformation.AddDefaultRegatta; import com.sap.sailing.server.operationaltransformation.AddRaceDefinition; import com.sap.sailing.server.operationaltransformation.CreateTrackedRace; @@ -138,6 +141,25 @@ public class TrackedRaceContentsReplicationTest extends AbstractServerReplicatio } } + /** + * See also bug4387: test that after replicating the TrackedRace the CompetitorBravoFixTrackImpl's gpsTrack object + * points to the track from the TrackedRace to which the CompetitorBravoFixTrackImpl belongs. + */ + @Test + public void testBravoFixTrackReplication() throws InterruptedException { + trackedRace.getOrCreateSensorTrack(competitor, BravoFixTrack.TRACK_NAME, () -> new CompetitorBravoFixTrackImpl(competitor, BravoFixTrack.TRACK_NAME, /* hasExtendedFixes */ false, + trackedRace.getTrack(competitor))); + Thread.sleep(1000); + DynamicTrackedRace replicaTrackedRace = replica.getTrackedRace(raceIdentifier); + Competitor replicaCompetitor = replicaTrackedRace.getRace().getCompetitors().iterator().next(); + assertNotNull(replicaCompetitor); + GPSFixTrack competitorTrack = replicaTrackedRace.getTrack(replicaCompetitor); + CompetitorBravoFixTrackImpl competitorSensorTrack = replicaTrackedRace.getOrCreateSensorTrack( + replicaCompetitor, BravoFixTrack.TRACK_NAME, () -> new CompetitorBravoFixTrackImpl(replicaCompetitor, BravoFixTrack.TRACK_NAME, /* hasExtendedFixes */ false, + replicaTrackedRace.getTrack(replicaCompetitor))); + assertSame(competitorTrack, competitorSensorTrack.getGpsTrack()); + } + @Test public void testMarkFixReplication() throws InterruptedException { final GPSFixMovingImpl fix = new GPSFixMovingImpl(new DegreePosition(2, 3), new MillisecondsTimePoint(3456),