From 0fe25e18429d7cae494a47b5e79bc3bff28729f8 Mon Sep 17 00:00:00 2001 From: Axel Uhl Date: Wed, 11 Jan 2017 18:10:01 +0100 Subject: [PATCH] found a fix for bug 4025 that also fixes the failing test case; compute old results by using an extension to the results finder that can exclude a particular result event; then remove score corrections matching old results that are not overwritten by new results Change-Id: I953f85a5dd04bffc5a82941406492aa7d8a5641d --- .../ConfirmedFinishPositioningListFinder.java | 20 +++++- .../race/state/RaceStateChangedListener.java | 1 - .../impl/DynamicTrackedRaceLogListener.java | 3 + .../test/ApplyScoresFromRaceLogTest.java | 20 +++--- .../server/impl/RaceLogScoringReplicator.java | 71 ++++++++++++++++--- 5 files changed, 93 insertions(+), 22 deletions(-) diff --git a/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/abstractlog/race/analyzing/impl/ConfirmedFinishPositioningListFinder.java b/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/abstractlog/race/analyzing/impl/ConfirmedFinishPositioningListFinder.java index 381df5b1c36..47c18c85d70 100644 --- a/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/abstractlog/race/analyzing/impl/ConfirmedFinishPositioningListFinder.java +++ b/java/com.sap.sailing.domain.shared.android/src/com/sap/sailing/domain/abstractlog/race/analyzing/impl/ConfirmedFinishPositioningListFinder.java @@ -13,12 +13,30 @@ public class ConfirmedFinishPositioningListFinder extends RaceLogAnalyzer raceLogs = new ConcurrentHashMap<>(); private DynamicTrackedRace trackedRace; diff --git a/java/com.sap.sailing.server.test/src/com/sap/sailing/server/test/ApplyScoresFromRaceLogTest.java b/java/com.sap.sailing.server.test/src/com/sap/sailing/server/test/ApplyScoresFromRaceLogTest.java index 9c64d6d6ff5..1949cdd0517 100644 --- a/java/com.sap.sailing.server.test/src/com/sap/sailing/server/test/ApplyScoresFromRaceLogTest.java +++ b/java/com.sap.sailing.server.test/src/com/sap/sailing/server/test/ApplyScoresFromRaceLogTest.java @@ -179,12 +179,12 @@ public class ApplyScoresFromRaceLogTest extends LeaderboardScoringAndRankingTest public void testApplyingOCSThenClearingOCS() { TimePoint now = MillisecondsTimePoint.now(); TimePoint later = new MillisecondsTimePoint(now.asMillis()+1000); - setUp(3, now, ScoringSchemeType.LOW_POINT); - int oneBasedRank = 1; + final int numberOfCompetitors = 2; + setUp(numberOfCompetitors, now, ScoringSchemeType.LOW_POINT); final CompetitorResults results = new CompetitorResultsImpl(); // set OCS - setResultForCompetitor(competitors.get(0), oneBasedRank++, results, MaxPointsReason.OCS, /* explicit score */ null); - setResultForCompetitor(competitors.get(1), oneBasedRank++, results, MaxPointsReason.OCS, /* explicit score */ null); + setResultForCompetitor(competitors.get(0), /* one-based rank */ 0, results, MaxPointsReason.OCS, /* explicit score */ null); + setResultForCompetitor(competitors.get(1), /* one-based rank */ 0, results, MaxPointsReason.OCS, /* explicit score */ null); final RaceLog f1RaceLog = f1Column.getRaceLog(f1Column.getFleets().iterator().next()); final LogEventAuthorImpl author = new LogEventAuthorImpl("Axel", 0); final RaceState f1RaceState = new RaceStateImpl(service, f1RaceLog, author, @@ -194,17 +194,17 @@ public class ApplyScoresFromRaceLogTest extends LeaderboardScoringAndRankingTest assertEquals(competitors, rankedCompetitorsBeforeApplying); // no effects of preliminary results list yet f1RaceState.setFinishPositioningConfirmed(now); // validate that it arrived in leaderboard - assertScoreCorrections(leaderboard, f1Column, competitors.get(0), MaxPointsReason.OCS, 4, /* score is corrected */ false, later); - assertScoreCorrections(leaderboard, f1Column, competitors.get(1), MaxPointsReason.OCS, 4, /* score is corrected */ false, later); + assertScoreCorrections(leaderboard, f1Column, competitors.get(0), MaxPointsReason.OCS, numberOfCompetitors+1, /* score is corrected */ false, later); + assertScoreCorrections(leaderboard, f1Column, competitors.get(1), MaxPointsReason.OCS, numberOfCompetitors+1, /* score is corrected */ false, later); // now clear OCS for first competitor again and leave second competitor OCS: final CompetitorResults resultsWithOCSCleared = new CompetitorResultsImpl(); - setResultForCompetitor(competitors.get(1), oneBasedRank++, resultsWithOCSCleared, MaxPointsReason.OCS, /* explicit score */ null); + setResultForCompetitor(competitors.get(1), /* one-based rank */ 0, resultsWithOCSCleared, MaxPointsReason.OCS, /* explicit score */ null); f1RaceState.setFinishPositioningListChanged(later, resultsWithOCSCleared); f1RaceState.setFinishPositioningConfirmed(later); // validate that it got cleared in leaderboard - assertScoreCorrections(leaderboard, f1Column, competitors.get(1), MaxPointsReason.OCS, 4, /* score is corrected */ false, later); - assertScoreCorrections(leaderboard, f1Column, competitors.get(0), MaxPointsReason.NONE, 0, /* score is corrected */ false, later); + assertScoreCorrections(leaderboard, f1Column, competitors.get(1), MaxPointsReason.OCS, numberOfCompetitors+1, /* score is corrected */ false, later); + assertScoreCorrections(leaderboard, f1Column, competitors.get(0), MaxPointsReason.NONE, 1, /* score is corrected */ false, later); // ranking first, one point in low-point scheme } private void assertScoreCorrections(Leaderboard leaderboard, RaceColumn raceColumn, Competitor competitor, @@ -217,7 +217,7 @@ public class ApplyScoresFromRaceLogTest extends LeaderboardScoringAndRankingTest private void setResultForCompetitor(final Competitor competitor, int oneBasedRank, final CompetitorResults results, MaxPointsReason maxPointsReason, Double explicitScore) { - results.add(new CompetitorResultImpl(competitor.getId(), competitor.getName(), oneBasedRank++, maxPointsReason, explicitScore, /* finishingTime */ null, /* comment */ null)); + results.add(new CompetitorResultImpl(competitor.getId(), competitor.getName(), oneBasedRank, maxPointsReason, explicitScore, /* finishingTime */ null, /* comment */ null)); } diff --git a/java/com.sap.sailing.server/src/com/sap/sailing/server/impl/RaceLogScoringReplicator.java b/java/com.sap.sailing.server/src/com/sap/sailing/server/impl/RaceLogScoringReplicator.java index 56e3f0ef579..8d5e943fecf 100644 --- a/java/com.sap.sailing.server/src/com/sap/sailing/server/impl/RaceLogScoringReplicator.java +++ b/java/com.sap.sailing.server/src/com/sap/sailing/server/impl/RaceLogScoringReplicator.java @@ -1,5 +1,9 @@ package com.sap.sailing.server.impl; +import java.io.Serializable; +import java.util.HashMap; +import java.util.Map; + import com.sap.sailing.domain.abstractlog.race.CompetitorResult; import com.sap.sailing.domain.abstractlog.race.CompetitorResults; import com.sap.sailing.domain.abstractlog.race.RaceLog; @@ -16,6 +20,7 @@ import com.sap.sailing.domain.base.RaceColumnListener; import com.sap.sailing.domain.common.MaxPointsReason; import com.sap.sailing.domain.leaderboard.Leaderboard; import com.sap.sailing.domain.leaderboard.ResultDiscardingRule; +import com.sap.sailing.domain.leaderboard.ScoringScheme; import com.sap.sailing.domain.racelog.RaceLogIdentifier; import com.sap.sailing.domain.tracking.TrackedRace; import com.sap.sailing.server.RacingEventService; @@ -94,7 +99,7 @@ public class RaceLogScoringReplicator implements RaceColumnListener { event.accept(new BaseRaceLogEventVisitor() { @Override public void visit(RaceLogFinishPositioningConfirmedEvent event) { - handleFinishPositioningList(raceColumn, raceLogIdentifier, (RaceLogFinishPositioningConfirmedEvent) event); + handleFinishPositioningList(raceColumn, raceLogIdentifier, event); } }); } @@ -104,7 +109,7 @@ public class RaceLogScoringReplicator implements RaceColumnListener { if (leaderboard != null) { Fleet fleet = raceColumn.getFleetByName(raceLogIdentifier.getFleetName()); RaceLog raceLog = raceColumn.getRaceLog(fleet); - checkNeedForScoreCorrectionByResultsOfRaceCommittee(leaderboard, raceColumn, fleet, raceLog, event.getCreatedAt()); + checkNeedForScoreCorrectionByResultsOfRaceCommittee(leaderboard, raceColumn, fleet, raceLog, event.getCreatedAt(), event); } } @@ -129,13 +134,19 @@ public class RaceLogScoringReplicator implements RaceColumnListener { * * @param timePoint * the TimePoint at which the race committee confirmed their last rank list entered in the app. + * @param event + * the event that has announced the latest results; used with a finder to determine the effective changes to + * be applied to the leaderboard; if before {@code event} the race log had a result for a competitor that + * is still in the {@code leaderboard}'s score corrections and after applying {@code event} there is no + * result for that competitor anymore, remove those score corrections. */ - private void checkNeedForScoreCorrectionByResultsOfRaceCommittee(Leaderboard leaderboard, RaceColumn raceColumn, Fleet fleet, RaceLog raceLog, TimePoint timePoint) { + private void checkNeedForScoreCorrectionByResultsOfRaceCommittee(Leaderboard leaderboard, RaceColumn raceColumn, + Fleet fleet, RaceLog raceLog, TimePoint timePoint, RaceLogFinishPositioningConfirmedEvent event) { int numberOfCompetitorsInLeaderboard = Util.size(leaderboard.getCompetitors()); int numberOfCompetitorsInRace; CompetitorResults positioningList; numberOfCompetitorsInRace = getNumberOfCompetitorsInRace(raceColumn, fleet, numberOfCompetitorsInLeaderboard); - ConfirmedFinishPositioningListFinder confirmedPositioningListFinder = new ConfirmedFinishPositioningListFinder(raceLog); + final ConfirmedFinishPositioningListFinder confirmedPositioningListFinder = new ConfirmedFinishPositioningListFinder(raceLog); positioningList = confirmedPositioningListFinder.analyze(); if (positioningList == null) { // we expect this case for old sailing events such as ESS Singapore, Quingdao, where the confirmation event did not contain the finish @@ -145,27 +156,52 @@ public class RaceLogScoringReplicator implements RaceColumnListener { // RaceLogFinishPositioningConfirmedEvent event was found in the race log } if (positioningList != null) { + final Map newResultsByCompetitorId = new HashMap<>(); for (CompetitorResult positionedCompetitor : positioningList) { - Competitor competitor = service.getBaseDomainFactory().getExistingCompetitorById(positionedCompetitor.getCompetitorId()); + newResultsByCompetitorId.put(positionedCompetitor.getCompetitorId(), positionedCompetitor); + final Competitor competitor = service.getBaseDomainFactory().getExistingCompetitorById(positionedCompetitor.getCompetitorId()); // The score is updated when explicitly provided or when no penalty was set; // in turn, this means that when a penalty is set and no score is explicitly provided, // it is up to the scoring scheme to infer a penalty score for the MaxPointsReason. // See also bug 3955. - if (positionedCompetitor.getScore() != null - || positionedCompetitor.getMaxPointsReason() == null - || positionedCompetitor.getMaxPointsReason().equals(MaxPointsReason.NONE)) { + if (isNeedToCorrectScore(positionedCompetitor)) { int rankByRaceCommittee = getRankInPositioningListByRaceCommittee(positionedCompetitor); correctScoreInLeaderboard(leaderboard, raceColumn, timePoint, numberOfCompetitorsInRace, competitor, rankByRaceCommittee, positionedCompetitor.getScore()); } setMaxPointsReasonInLeaderboardIfNecessary(leaderboard, raceColumn, timePoint, positionedCompetitor.getMaxPointsReason(), competitor); } + final CompetitorResults oldResults = confirmedPositioningListFinder.analyzeIgnoring(event); + if (oldResults != null) { + // check if any of the old results need to be canceled from the leaderboard's score corrections, such + // as a cleared OCS; see also bug 4025 + for (final CompetitorResult oldResult : oldResults) { + if (!newResultsByCompetitorId.containsKey(oldResult.getCompetitorId())) { + final Competitor competitor = service.getBaseDomainFactory().getExistingCompetitorById(oldResult.getCompetitorId()); + // check what the leaderboard score correction looks like; if it matches the old result, remove the correction: + if (Util.equalsWithNull(leaderboard.getScoreCorrection().getMaxPointsReason(competitor, raceColumn, TimePoint.EndOfTime), oldResult.getMaxPointsReason())) { + service.apply(new UpdateLeaderboardMaxPointsReason(leaderboard.getName(), raceColumn.getName(), competitor.getId().toString(), /* reason */ null, timePoint)); + } + if (isNeedToCorrectScore(oldResult) && + Util.equalsWithNull(leaderboard.getScoreCorrection().getExplicitScoreCorrection(competitor, raceColumn), + getScoreFromRaceCommittee(leaderboard, raceColumn, timePoint, numberOfCompetitorsInRace, competitor, oldResult.getOneBasedRank(), oldResult.getScore()))) { + service.apply(new UpdateLeaderboardScoreCorrection(leaderboard.getName(), raceColumn.getName(), competitor.getId().toString(), /* correctedScore */ null, timePoint)); + } + } + } + } // Since the metadata update is used by the Sailing suite to determine the final state of a race, it has to // be triggered, even though no score correction may have been performed applyMetadataUpdate(leaderboard, timePoint, COMMENT_TEXT_ON_SCORE_CORRECTION); } } + private boolean isNeedToCorrectScore(CompetitorResult positionedCompetitor) { + return positionedCompetitor.getScore() != null + || positionedCompetitor.getMaxPointsReason() == null + || positionedCompetitor.getMaxPointsReason().equals(MaxPointsReason.NONE); + } + private boolean setMaxPointsReasonInLeaderboardIfNecessary(Leaderboard leaderboard, RaceColumn raceColumn, TimePoint timePoint, MaxPointsReason maxPointsReason, Competitor competitor) { boolean scoreHasBeenCorrected = false; @@ -181,6 +217,22 @@ public class RaceLogScoringReplicator implements RaceColumnListener { private void correctScoreInLeaderboard(Leaderboard leaderboard, RaceColumn raceColumn, TimePoint timePoint, final int numberOfCompetitorsInRace, Competitor competitor, int rankByRaceCommittee, Double optionalExplicitScore) { + final Double scoreByRaceCommittee = getScoreFromRaceCommittee(leaderboard, raceColumn, timePoint, + numberOfCompetitorsInRace, competitor, rankByRaceCommittee, optionalExplicitScore); + // Do ALWAYS apply score corrections from race committee + applyScoreCorrectionOperation(leaderboard, raceColumn, competitor, scoreByRaceCommittee, timePoint); + } + + /** + * If a non-{@code null} {@code optionalExplicitScore} is provided, it it returned. Otherwise, an implicit score is + * determined from the {@code rankByRaceCommittee} using the leaderboard scoring scheme's + * {@link ScoringScheme#getScoreForRank(Leaderboard, RaceColumn, Competitor, int, java.util.concurrent.Callable, com.sap.sailing.domain.leaderboard.NumberOfCompetitorsInLeaderboardFetcher, TimePoint) + * getScoreForRank(...)} method. Usually, scoring schemes will return {@code null} as score for {@code 0} as + * (one-based) rank. + */ + private Double getScoreFromRaceCommittee(Leaderboard leaderboard, RaceColumn raceColumn, TimePoint timePoint, + final int numberOfCompetitorsInRace, Competitor competitor, int rankByRaceCommittee, + Double optionalExplicitScore) { final Double scoreByRaceCommittee; if (optionalExplicitScore == null) { scoreByRaceCommittee = leaderboard.getScoringScheme().getScoreForRank(leaderboard, raceColumn, competitor, @@ -188,8 +240,7 @@ public class RaceLogScoringReplicator implements RaceColumnListener { } else { scoreByRaceCommittee = optionalExplicitScore; } - // Do ALWAYS apply score corrections from race committee - applyScoreCorrectionOperation(leaderboard, raceColumn, competitor, scoreByRaceCommittee, timePoint); + return scoreByRaceCommittee; } private void applyScoreCorrectionOperation(Leaderboard leaderboard, RaceColumn raceColumn, Competitor competitor, Double correctedScore, TimePoint timePoint) {