fd7d2c171e04b0dd670d2d5155bd15e9f49b4707
SELECTION_REFACTORING_ANALYSIS.md
| ... | ... | @@ -1,400 +0,0 @@ |
| 1 | -# LeaderboardPanel Selection State Refactoring Analysis |
|
| 2 | - |
|
| 3 | -## Current Architecture |
|
| 4 | - |
|
| 5 | -### Three Selection Models in Play |
|
| 6 | - |
|
| 7 | -1. **`leaderboardSelectionModel`** (LeaderboardPanel:239) |
|
| 8 | - - Type: `MultiSelectionModel<LeaderboardRowDTO>` |
|
| 9 | - - Created at line 527 |
|
| 10 | - - Attached to the CellTable at line 547 |
|
| 11 | - - Has a `selectionChangeHandler` that syncs TO CompetitorSelectionProvider (lines 530-544) |
|
| 12 | - |
|
| 13 | -2. **`SelectionCheckboxColumn.selectionModel`** (SelectionCheckboxColumn:51) |
|
| 14 | - - Type: `RefreshableMultiSelectionModel<T>` |
|
| 15 | - - Created by SelectionCheckboxColumn constructor |
|
| 16 | - - **Currently NOT used by the LeaderboardPanel's CellTable** |
|
| 17 | - - Has display.flush() callbacks for UI updates |
|
| 18 | - |
|
| 19 | -3. **`CompetitorSelectionProvider`** |
|
| 20 | - - Application-level selection state |
|
| 21 | - - Both LeaderboardPanel and SelectionCheckboxColumn listen to it |
|
| 22 | - - Notifies listeners via `addedToSelection()` / `removedFromSelection()` |
|
| 23 | - |
|
| 24 | -### Current Selection Flow Problems |
|
| 25 | - |
|
| 26 | -#### Flow 1: User clicks checkbox in UI |
|
| 27 | -``` |
|
| 28 | -User clicks checkbox |
|
| 29 | - ↓ |
|
| 30 | -SelectionCheckboxColumn handles click event |
|
| 31 | - ↓ |
|
| 32 | -leaderboardSelectionModel.setSelected() called ← Uses WRONG selection model! |
|
| 33 | - ↓ |
|
| 34 | -selectionChangeHandler fires (lines 530-544) |
|
| 35 | - ↓ |
|
| 36 | -competitorSelectionProvider.setSelection() called |
|
| 37 | - ↓ |
|
| 38 | -CompetitorSelectionProvider fires addedToSelection/removedFromSelection |
|
| 39 | - ↓ |
|
| 40 | -BOTH LeaderboardPanel AND SelectionCheckboxColumn receive callback: |
|
| 41 | - - LeaderboardPanel.addedToSelection() (line 3393) |
|
| 42 | - → calls leaderboardSelectionModel.setSelected() |
|
| 43 | - - SelectionCheckboxColumn.addedToSelection() (line 2160) |
|
| 44 | - → calls getSelectionModel().setSelected() ← Different model! |
|
| 45 | -``` |
|
| 46 | - |
|
| 47 | -**Problem**: Two selection models exist but only one (leaderboardSelectionModel) is attached to the table. The SelectionCheckboxColumn's RefreshableMultiSelectionModel is created but orphaned. |
|
| 48 | - |
|
| 49 | -#### Flow 2: CompetitorSelectionProvider changes externally |
|
| 50 | -``` |
|
| 51 | -External change to CompetitorSelectionProvider |
|
| 52 | - ↓ |
|
| 53 | -Fires addedToSelection/removedFromSelection |
|
| 54 | - ↓ |
|
| 55 | -BOTH listeners receive callback: |
|
| 56 | - - LeaderboardPanel.addedToSelection() (line 3393) |
|
| 57 | - → leaderboardSelectionModel.setSelected(row, true) |
|
| 58 | - → Could trigger selectionChangeHandler → infinite loop potential! |
|
| 59 | - - SelectionCheckboxColumn.addedToSelection() (line 2160) |
|
| 60 | - → getSelectionModel().setSelected(row, true) ← Orphaned model! |
|
| 61 | -``` |
|
| 62 | - |
|
| 63 | -**Problem**: LeaderboardPanel guards against recursion by temporarily removing the handler (line 2712), but this is complex. SelectionCheckboxColumn updates an unused selection model. |
|
| 64 | - |
|
| 65 | -### Inconsistencies Found |
|
| 66 | - |
|
| 67 | -1. **Line 527**: LeaderboardPanel creates its own `MultiSelectionModel` |
|
| 68 | -2. **Line 547**: Attaches that model to the table, ignoring SelectionCheckboxColumn's model |
|
| 69 | -3. **Line 2163/2174**: SelectionCheckboxColumn callbacks update its own orphaned model |
|
| 70 | -4. **Line 3396/3404**: LeaderboardPanel callbacks update the table's model |
|
| 71 | -5. **SelectionCheckboxColumn.getValue()** (line 2140): Checks CompetitorSelectionProvider directly, NOT its own selection model! |
|
| 72 | - |
|
| 73 | -## Proposed Refactoring |
|
| 74 | - |
|
| 75 | -### Goal |
|
| 76 | -Use **ONLY** the `RefreshableMultiSelectionModel` from SelectionCheckboxColumn as the single source of truth, synchronized bidirectionally with CompetitorSelectionProvider. |
|
| 77 | - |
|
| 78 | -### Step-by-Step Plan |
|
| 79 | - |
|
| 80 | -#### Step 1: Use SelectionCheckboxColumn's Model as Table's Model |
|
| 81 | - |
|
| 82 | -**File**: `LeaderboardPanel.java` |
|
| 83 | - |
|
| 84 | -**Current code** (lines 525-547): |
|
| 85 | -```java |
|
| 86 | -selectionCheckboxColumn = new LeaderboardSelectionCheckboxColumn(competitorSelectionProvider); |
|
| 87 | -leaderboardTable.setWidth("100%"); |
|
| 88 | -leaderboardSelectionModel = new MultiSelectionModel<LeaderboardRowDTO>(); // ← REMOVE THIS |
|
| 89 | -selectionChangeHandler = new Handler() { |
|
| 90 | - @Override |
|
| 91 | - public void onSelectionChange(SelectionChangeEvent event) { |
|
| 92 | - List<CompetitorDTO> selection = new ArrayList<>(); |
|
| 93 | - for (LeaderboardRowDTO row : getSelectedRows()) { |
|
| 94 | - selection.add(row.competitor); |
|
| 95 | - } |
|
| 96 | - LeaderboardPanel.this.competitorSelectionProvider.setSelection(selection, |
|
| 97 | - /* listenersNotToNotify */LeaderboardPanel.this); |
|
| 98 | - if (blurInOnSelectionChanged > 0) { |
|
| 99 | - blurInOnSelectionChanged--; |
|
| 100 | - blurFocusedElementAfterSelectionChange(); |
|
| 101 | - } |
|
| 102 | - } |
|
| 103 | -}; |
|
| 104 | -leaderboardAsTableSelectionModelRegistration = leaderboardSelectionModel |
|
| 105 | - .addSelectionChangeHandler(selectionChangeHandler); |
|
| 106 | -leaderboardTable.setSelectionModel(leaderboardSelectionModel, selectionCheckboxColumn.getSelectionManager()); |
|
| 107 | -``` |
|
| 108 | - |
|
| 109 | -**Refactored**: |
|
| 110 | -```java |
|
| 111 | -selectionCheckboxColumn = new LeaderboardSelectionCheckboxColumn(competitorSelectionProvider); |
|
| 112 | -leaderboardTable.setWidth("100%"); |
|
| 113 | - |
|
| 114 | -// Use the SelectionCheckboxColumn's model as THE selection model |
|
| 115 | -leaderboardSelectionModel = selectionCheckboxColumn.getSelectionModel(); |
|
| 116 | - |
|
| 117 | -selectionChangeHandler = new Handler() { |
|
| 118 | - @Override |
|
| 119 | - public void onSelectionChange(SelectionChangeEvent event) { |
|
| 120 | - if (updatingSelectionFromProvider) { |
|
| 121 | - // Guard: don't sync back to provider if we're currently syncing FROM provider |
|
| 122 | - return; |
|
| 123 | - } |
|
| 124 | - List<CompetitorDTO> selection = new ArrayList<>(); |
|
| 125 | - for (LeaderboardRowDTO row : getSelectedRows()) { |
|
| 126 | - selection.add(row.competitor); |
|
| 127 | - } |
|
| 128 | - LeaderboardPanel.this.competitorSelectionProvider.setSelection(selection, |
|
| 129 | - /* listenersNotToNotify */LeaderboardPanel.this); |
|
| 130 | - if (blurInOnSelectionChanged > 0) { |
|
| 131 | - blurInOnSelectionChanged--; |
|
| 132 | - blurFocusedElementAfterSelectionChange(); |
|
| 133 | - } |
|
| 134 | - } |
|
| 135 | -}; |
|
| 136 | -leaderboardAsTableSelectionModelRegistration = leaderboardSelectionModel |
|
| 137 | - .addSelectionChangeHandler(selectionChangeHandler); |
|
| 138 | -leaderboardTable.setSelectionModel(leaderboardSelectionModel, selectionCheckboxColumn.getSelectionManager()); |
|
| 139 | -``` |
|
| 140 | - |
|
| 141 | -**Changes**: |
|
| 142 | -- Line 527: Change from `new MultiSelectionModel<>()` to `selectionCheckboxColumn.getSelectionModel()` |
|
| 143 | -- Add guard flag `updatingSelectionFromProvider` to prevent recursion |
|
| 144 | -- Declare field: `private boolean updatingSelectionFromProvider = false;` |
|
| 145 | - |
|
| 146 | -#### Step 2: Update Field Declaration |
|
| 147 | - |
|
| 148 | -**Current** (line 239): |
|
| 149 | -```java |
|
| 150 | -private final MultiSelectionModel<LeaderboardRowDTO> leaderboardSelectionModel; |
|
| 151 | -``` |
|
| 152 | - |
|
| 153 | -**Refactored**: |
|
| 154 | -```java |
|
| 155 | -private final RefreshableMultiSelectionModel<LeaderboardRowDTO> leaderboardSelectionModel; |
|
| 156 | -``` |
|
| 157 | - |
|
| 158 | -Add import: |
|
| 159 | -```java |
|
| 160 | -import com.sap.sse.gwt.client.celltable.RefreshableMultiSelectionModel; |
|
| 161 | -``` |
|
| 162 | - |
|
| 163 | -#### Step 3: Fix LeaderboardPanel's Selection Callbacks |
|
| 164 | - |
|
| 165 | -**Current** (lines 3393-3406): |
|
| 166 | -```java |
|
| 167 | -@Override |
|
| 168 | -public void addedToSelection(CompetitorDTO competitor) { |
|
| 169 | - LeaderboardRowDTO row = getRow(competitor.getIdAsString()); |
|
| 170 | - if (row != null) { |
|
| 171 | - leaderboardSelectionModel.setSelected(row, true); |
|
| 172 | - } |
|
| 173 | -} |
|
| 174 | - |
|
| 175 | -@Override |
|
| 176 | -public void removedFromSelection(CompetitorDTO competitor) { |
|
| 177 | - LeaderboardRowDTO row = getRow(competitor.getIdAsString()); |
|
| 178 | - if (row != null) { |
|
| 179 | - leaderboardSelectionModel.setSelected(row, false); |
|
| 180 | - } |
|
| 181 | -} |
|
| 182 | -``` |
|
| 183 | - |
|
| 184 | -**Refactored**: |
|
| 185 | -```java |
|
| 186 | -@Override |
|
| 187 | -public void addedToSelection(CompetitorDTO competitor) { |
|
| 188 | - LeaderboardRowDTO row = getRow(competitor.getIdAsString()); |
|
| 189 | - if (row != null) { |
|
| 190 | - updatingSelectionFromProvider = true; |
|
| 191 | - try { |
|
| 192 | - leaderboardSelectionModel.setSelected(row, true); |
|
| 193 | - } finally { |
|
| 194 | - updatingSelectionFromProvider = false; |
|
| 195 | - } |
|
| 196 | - } |
|
| 197 | -} |
|
| 198 | - |
|
| 199 | -@Override |
|
| 200 | -public void removedFromSelection(CompetitorDTO competitor) { |
|
| 201 | - LeaderboardRowDTO row = getRow(competitor.getIdAsString()); |
|
| 202 | - if (row != null) { |
|
| 203 | - updatingSelectionFromProvider = true; |
|
| 204 | - try { |
|
| 205 | - leaderboardSelectionModel.setSelected(row, false); |
|
| 206 | - } finally { |
|
| 207 | - updatingSelectionFromProvider = false; |
|
| 208 | - } |
|
| 209 | - } |
|
| 210 | -} |
|
| 211 | -``` |
|
| 212 | - |
|
| 213 | -**Changes**: |
|
| 214 | -- Set guard flag before updating selection model |
|
| 215 | -- Ensures selectionChangeHandler won't sync back to CompetitorSelectionProvider |
|
| 216 | - |
|
| 217 | -#### Step 4: Remove SelectionCheckboxColumn's Redundant Listener |
|
| 218 | - |
|
| 219 | -**File**: `LeaderboardPanel.java`, inner class `LeaderboardSelectionCheckboxColumn` |
|
| 220 | - |
|
| 221 | -**Current** (lines 2120-2136): |
|
| 222 | -```java |
|
| 223 | -protected LeaderboardSelectionCheckboxColumn(final CompetitorSelectionProvider competitorSelectionProvider) { |
|
| 224 | - super(style.getTableresources().cellTableStyle().cellTableCheckboxSelected(), |
|
| 225 | - style.getTableresources().cellTableStyle().cellTableCheckboxDeselected(), |
|
| 226 | - style.getTableresources().cellTableStyle().cellTableCheckboxColumnCell(), |
|
| 227 | - new EntityIdentityComparator<LeaderboardRowDTO>() { |
|
| 228 | - @Override |
|
| 229 | - public boolean representSameEntity(LeaderboardRowDTO dto1, LeaderboardRowDTO dto2) { |
|
| 230 | - return dto1.competitor.getIdAsString().equals(dto2.competitor.getIdAsString()); |
|
| 231 | - } |
|
| 232 | - |
|
| 233 | - @Override |
|
| 234 | - public int hashCode(LeaderboardRowDTO t) { |
|
| 235 | - return t.competitor.getIdAsString().hashCode(); |
|
| 236 | - } |
|
| 237 | - }, getData(), leaderboardTable); |
|
| 238 | - competitorSelectionProvider.addCompetitorSelectionChangeListener(this); // ← REMOVE THIS |
|
| 239 | -} |
|
| 240 | -``` |
|
| 241 | - |
|
| 242 | -**Refactored**: |
|
| 243 | -```java |
|
| 244 | -protected LeaderboardSelectionCheckboxColumn(final CompetitorSelectionProvider competitorSelectionProvider) { |
|
| 245 | - super(style.getTableresources().cellTableStyle().cellTableCheckboxSelected(), |
|
| 246 | - style.getTableresources().cellTableStyle().cellTableCheckboxDeselected(), |
|
| 247 | - style.getTableresources().cellTableStyle().cellTableCheckboxColumnCell(), |
|
| 248 | - new EntityIdentityComparator<LeaderboardRowDTO>() { |
|
| 249 | - @Override |
|
| 250 | - public boolean representSameEntity(LeaderboardRowDTO dto1, LeaderboardRowDTO dto2) { |
|
| 251 | - return dto1.competitor.getIdAsString().equals(dto2.competitor.getIdAsString()); |
|
| 252 | - } |
|
| 253 | - |
|
| 254 | - @Override |
|
| 255 | - public int hashCode(LeaderboardRowDTO t) { |
|
| 256 | - return t.competitor.getIdAsString().hashCode(); |
|
| 257 | - } |
|
| 258 | - }, getData(), leaderboardTable); |
|
| 259 | - // REMOVED: competitorSelectionProvider.addCompetitorSelectionChangeListener(this); |
|
| 260 | - // LeaderboardPanel is now the ONLY listener that syncs selection state |
|
| 261 | -} |
|
| 262 | -``` |
|
| 263 | - |
|
| 264 | -**Current** (lines 2160-2176, now unnecessary): |
|
| 265 | -```java |
|
| 266 | -@Override |
|
| 267 | -public void addedToSelection(CompetitorDTO competitor) { |
|
| 268 | - final LeaderboardRowDTO row = getRow(competitor.getIdAsString()); |
|
| 269 | - if (row != null) { |
|
| 270 | - getSelectionModel().setSelected(row, true); |
|
| 271 | - } |
|
| 272 | -} |
|
| 273 | - |
|
| 274 | -@Override |
|
| 275 | -public void removedFromSelection(CompetitorDTO competitor) { |
|
| 276 | - final LeaderboardRowDTO row = getRow(competitor.getIdAsString()); |
|
| 277 | - if (row != null) { |
|
| 278 | - getSelectionModel().setSelected(row, false); |
|
| 279 | - } |
|
| 280 | -} |
|
| 281 | -``` |
|
| 282 | - |
|
| 283 | -**Refactored**: Remove these methods entirely. The SelectionCheckboxColumn no longer needs to implement `CompetitorSelectionChangeListener`. |
|
| 284 | - |
|
| 285 | -**Also remove** (line 2119): |
|
| 286 | -```java |
|
| 287 | -// FROM: |
|
| 288 | -private class LeaderboardSelectionCheckboxColumn |
|
| 289 | - extends com.sap.sse.gwt.client.celltable.SelectionCheckboxColumn<LeaderboardRowDTO> |
|
| 290 | - implements CompetitorSelectionChangeListener { |
|
| 291 | - |
|
| 292 | -// TO: |
|
| 293 | -private class LeaderboardSelectionCheckboxColumn |
|
| 294 | - extends com.sap.sse.gwt.client.celltable.SelectionCheckboxColumn<LeaderboardRowDTO> { |
|
| 295 | -``` |
|
| 296 | - |
|
| 297 | -Remove the empty listener method implementations at lines 2144-2154. |
|
| 298 | - |
|
| 299 | -#### Step 5: Simplify updateSelection() |
|
| 300 | - |
|
| 301 | -**Current** (lines 2707-2721): |
|
| 302 | -```java |
|
| 303 | -private void updateSelection(LeaderboardRowDTO row) { |
|
| 304 | - final boolean shallBeSelected = competitorSelectionProvider.isSelected(row.competitor); |
|
| 305 | - if (leaderboardAsTableSelectionModelRegistration != null) { |
|
| 306 | - // suspend selection events while actively adjusting the leaderboardSelectionModel to match the |
|
| 307 | - // competitorSelectionProvider |
|
| 308 | - leaderboardAsTableSelectionModelRegistration.removeHandler(); |
|
| 309 | - leaderboardAsTableSelectionModelRegistration = null; |
|
| 310 | - } |
|
| 311 | - if (leaderboardSelectionModel.isSelected(row) != shallBeSelected) { |
|
| 312 | - leaderboardSelectionModel.setSelected(row, shallBeSelected); |
|
| 313 | - } |
|
| 314 | - // register the selection change handler again |
|
| 315 | - leaderboardAsTableSelectionModelRegistration = leaderboardTable.getSelectionModel() |
|
| 316 | - .addSelectionChangeHandler(selectionChangeHandler); |
|
| 317 | -} |
|
| 318 | -``` |
|
| 319 | - |
|
| 320 | -**Refactored**: |
|
| 321 | -```java |
|
| 322 | -private void updateSelection(LeaderboardRowDTO row) { |
|
| 323 | - final boolean shallBeSelected = competitorSelectionProvider.isSelected(row.competitor); |
|
| 324 | - updatingSelectionFromProvider = true; |
|
| 325 | - try { |
|
| 326 | - if (leaderboardSelectionModel.isSelected(row) != shallBeSelected) { |
|
| 327 | - leaderboardSelectionModel.setSelected(row, shallBeSelected); |
|
| 328 | - } |
|
| 329 | - } finally { |
|
| 330 | - updatingSelectionFromProvider = false; |
|
| 331 | - } |
|
| 332 | -} |
|
| 333 | -``` |
|
| 334 | - |
|
| 335 | -**Changes**: |
|
| 336 | -- Removed handler removal/re-registration complexity |
|
| 337 | -- Use guard flag instead |
|
| 338 | -- Much simpler and clearer |
|
| 339 | - |
|
| 340 | -#### Step 6: Fix getValue() in LeaderboardSelectionCheckboxColumn |
|
| 341 | - |
|
| 342 | -**Current** (lines 2139-2141): |
|
| 343 | -```java |
|
| 344 | -@Override |
|
| 345 | -public Boolean getValue(LeaderboardRowDTO row) { |
|
| 346 | - return competitorSelectionProvider.isSelected(row.competitor); |
|
| 347 | -} |
|
| 348 | -``` |
|
| 349 | - |
|
| 350 | -**Refactored**: |
|
| 351 | -```java |
|
| 352 | -@Override |
|
| 353 | -public Boolean getValue(LeaderboardRowDTO row) { |
|
| 354 | - // Use the selection model as the source of truth, not CompetitorSelectionProvider |
|
| 355 | - return getSelectionModel().isSelected(row); |
|
| 356 | -} |
|
| 357 | -``` |
|
| 358 | - |
|
| 359 | -**Rationale**: The selection model should be the single source of truth for rendering. It stays synchronized with CompetitorSelectionProvider via the LeaderboardPanel's listeners. |
|
| 360 | - |
|
| 361 | -### Summary of Changes |
|
| 362 | - |
|
| 363 | -1. **LeaderboardPanel.java line 527**: Change to use `selectionCheckboxColumn.getSelectionModel()` |
|
| 364 | -2. **LeaderboardPanel.java line 239**: Change type to `RefreshableMultiSelectionModel` |
|
| 365 | -3. **LeaderboardPanel.java**: Add field `private boolean updatingSelectionFromProvider = false;` |
|
| 366 | -4. **LeaderboardPanel.java lines 530-544**: Add guard check in selectionChangeHandler |
|
| 367 | -5. **LeaderboardPanel.java lines 3393-3406**: Add guard flag around setSelected calls |
|
| 368 | -6. **LeaderboardPanel.java lines 2707-2721**: Simplify updateSelection() using guard flag |
|
| 369 | -7. **LeaderboardPanel.LeaderboardSelectionCheckboxColumn line 2135**: Remove listener registration |
|
| 370 | -8. **LeaderboardPanel.LeaderboardSelectionCheckboxColumn line 2119**: Remove `implements CompetitorSelectionChangeListener` |
|
| 371 | -9. **LeaderboardPanel.LeaderboardSelectionCheckboxColumn lines 2144-2176**: Remove listener methods |
|
| 372 | -10. **LeaderboardPanel.LeaderboardSelectionCheckboxColumn line 2140**: Use selection model instead of provider |
|
| 373 | - |
|
| 374 | -## Benefits |
|
| 375 | - |
|
| 376 | -1. **Single Selection Model**: RefreshableMultiSelectionModel is the only selection model |
|
| 377 | -2. **No Orphaned State**: Eliminates the unused selection model in SelectionCheckboxColumn |
|
| 378 | -3. **Simpler Synchronization**: Single bidirectional sync point between model and provider |
|
| 379 | -4. **No Handler Removal**: Guard flag is simpler than removing/re-adding handlers |
|
| 380 | -5. **Clearer Ownership**: LeaderboardPanel owns synchronization, SelectionCheckboxColumn just renders |
|
| 381 | - |
|
| 382 | -## Testing Strategy |
|
| 383 | - |
|
| 384 | -1. Test user clicking checkboxes - selection should sync to CompetitorSelectionProvider |
|
| 385 | -2. Test external CompetitorSelectionProvider changes - should update checkboxes |
|
| 386 | -3. Test rapid clicking - no infinite loops or race conditions |
|
| 387 | -4. Test with filtering - filtered rows should maintain correct selection state |
|
| 388 | -5. Test leaderboard updates - selection should persist across data refreshes |
|
| 389 | - |
|
| 390 | -## Potential Risks |
|
| 391 | - |
|
| 392 | -1. **GWT Selection Model Flush Timing**: GWT selection models flush changes at end of event loop. The guard flag approach should handle this correctly since the flag is set synchronously. |
|
| 393 | - |
|
| 394 | -2. **Multiple LeaderboardPanels**: If multiple LeaderboardPanels share the same CompetitorSelectionProvider, each will receive callbacks. This should work fine as each has its own guard flag. |
|
| 395 | - |
|
| 396 | -3. **RefreshableMultiSelectionModel.refreshSelectionModel()**: This method has its own `dontCheckSelectionState` guard (RefreshableMultiSelectionModel.java line 131). Verify this doesn't interfere with our guard flag. |
|
| 397 | - |
|
| 398 | -## Future Improvements |
|
| 399 | - |
|
| 400 | -Consider enhancing RefreshableMultiSelectionModel to accept a callback for selection changes, eliminating the need for the external SelectionChangeHandler and making the synchronization more encapsulated within the model itself. |
java/com.sap.sailing.domain.persistence/src/com/sap/sailing/domain/persistence/impl/MongoObjectFactoryImpl.java
| ... | ... | @@ -2107,7 +2107,7 @@ public class MongoObjectFactoryImpl implements MongoObjectFactory { |
| 2107 | 2107 | final Document fingerprintDoc = Document.parse(fingerprintjson.toString()); |
| 2108 | 2108 | result.put(FieldNames.MANEUVER_FINGERPRINT.name(), fingerprintDoc); |
| 2109 | 2109 | storeRaceIdentifier(result, raceIdentifier); |
| 2110 | - final List<Document> maneuverDoc = storeManeuvers( maneuvers , raceIdentifier, course); |
|
| 2110 | + final List<Document> maneuverDoc = storeManeuvers(maneuvers , raceIdentifier, course); |
|
| 2111 | 2111 | result.put(FieldNames.MANEUVERS.name(), maneuverDoc); |
| 2112 | 2112 | maneuverCollection.replaceOne(query, result, new ReplaceOptions().upsert(true)); |
| 2113 | 2113 | } |
java/com.sap.sailing.domain.racelogtrackingadapter.test/src/com/sap/sailing/domain/racelogtracking/test/impl/GPSFixStoreListenerTest.java
| ... | ... | @@ -39,10 +39,10 @@ public class GPSFixStoreListenerTest extends AbstractGPSFixStoreTest { |
| 39 | 39 | Thread thread = new Thread() { |
| 40 | 40 | public void run() { |
| 41 | 41 | try { |
| 42 | - barrier.await(100, TimeUnit.MILLISECONDS); |
|
| 42 | + barrier.await(1000, TimeUnit.MILLISECONDS); |
|
| 43 | 43 | // During iteration in the main thread this causes a modification that makes the iterator throw a |
| 44 | 44 | // ConcurrentModificationException on next() |
| 45 | - store.addListener((DeviceIdentifier device, GPSFixMoving fix, boolean returnManeuverChanges, boolean returnLiveDelay) -> { |
|
| 45 | + store.addListener((DeviceIdentifier device, Iterable<GPSFixMoving> fixes, boolean returnManeuverChanges, boolean returnLiveDelay) -> { |
|
| 46 | 46 | return null; |
| 47 | 47 | }, device); |
| 48 | 48 | barrier.await(100, TimeUnit.MILLISECONDS); |
| ... | ... | @@ -71,9 +71,9 @@ public class GPSFixStoreListenerTest extends AbstractGPSFixStoreTest { |
| 71 | 71 | } |
| 72 | 72 | |
| 73 | 73 | @Override |
| 74 | - public Iterable<Triple<RegattaAndRaceIdentifier, Boolean, Duration>> fixReceived(DeviceIdentifier device, GPSFixMoving fix, boolean returnManeuverChanges, boolean returnLiveDelay) { |
|
| 74 | + public Iterable<Triple<RegattaAndRaceIdentifier, Boolean, Duration>> fixesReceived(DeviceIdentifier device, Iterable<GPSFixMoving> fixes, boolean returnManeuverChanges, boolean returnLiveDelay) { |
|
| 75 | 75 | try { |
| 76 | - barrier.await(100, TimeUnit.MILLISECONDS); |
|
| 76 | + barrier.await(1000, TimeUnit.MILLISECONDS); |
|
| 77 | 77 | } catch (TimeoutException e) { |
| 78 | 78 | throw new TimoutRuntimeException(e); |
| 79 | 79 | } catch (Exception e) { |
java/com.sap.sailing.domain.racelogtrackingadapter.test/src/com/sap/sailing/domain/racelogtracking/test/impl/SensorFixStoreTest.java
| ... | ... | @@ -139,7 +139,6 @@ public class SensorFixStoreTest { |
| 139 | 139 | for (int i = 0; i < fixes; i++) { |
| 140 | 140 | addBravoFix(device, FIX_TIMESTAMP + 1, FIX_RIDE_HEIGHT); |
| 141 | 141 | } |
| 142 | - |
|
| 143 | 142 | List<Double> progressData = new ArrayList<>(); |
| 144 | 143 | store.loadFixes((fix) -> { |
| 145 | 144 | }, device, new MillisecondsTimePoint(FIX_TIMESTAMP - 1), new MillisecondsTimePoint(FIX_TIMESTAMP + fixes + 1), |
java/com.sap.sailing.domain.racelogtrackingadapter/src/com/sap/sailing/domain/racelogtracking/impl/fixtracker/FixLoaderAndTracker.java
| ... | ... | @@ -206,161 +206,163 @@ public class FixLoaderAndTracker implements TrackingDataLoader { |
| 206 | 206 | |
| 207 | 207 | private final FixReceivedListener<Timed> listener = new FixReceivedListener<Timed>() { |
| 208 | 208 | @Override |
| 209 | - public Iterable<Triple<RegattaAndRaceIdentifier, Boolean, Duration>> fixReceived(DeviceIdentifier device, |
|
| 210 | - Timed fix, boolean returnManeuverChanges, boolean returnLiveDelay) { |
|
| 209 | + public Iterable<Triple<RegattaAndRaceIdentifier, Boolean, Duration>> fixesReceived(DeviceIdentifier device, |
|
| 210 | + Iterable<Timed> fixes, boolean returnManeuverChanges, boolean returnLiveDelay) { |
|
| 211 | 211 | final Set<RegattaAndRaceIdentifier> maneuverChanged = new HashSet<>(); |
| 212 | 212 | final Map<RegattaAndRaceIdentifier, Duration> delayToLive = new HashMap<>(); |
| 213 | 213 | if (!preemptiveStopRequested.get() && trackedRace.getStartOfTracking() != null) { |
| 214 | - final TimePoint timePoint = fix.getTimePoint(); |
|
| 215 | - deviceMappings.forEachMappingOfDeviceIncludingTimePoint(device, fix.getTimePoint(), |
|
| 216 | - new Consumer<DeviceMappingWithRegattaLogEvent<WithID>>() { |
|
| 217 | - @Override |
|
| 218 | - public void accept(DeviceMappingWithRegattaLogEvent<WithID> mapping) { |
|
| 219 | - mapping.getRegattaLogEvent().accept(new MappingEventVisitor() { |
|
| 220 | - @Override |
|
| 221 | - public void visit(RegattaLogDeviceCompetitorSensorDataMappingEvent event) { |
|
| 222 | - recordSensorFixForCompetitor(event.getMappedTo(), event); |
|
| 223 | - } |
|
| 224 | - |
|
| 225 | - @Override |
|
| 226 | - public void visit(RegattaLogDeviceBoatSensorDataMappingEvent event) { |
|
| 227 | - final Boat boat = event.getMappedTo(); |
|
| 228 | - final Competitor competitor = trackedRace.getCompetitorOfBoat(boat); |
|
| 229 | - if (competitor != null) { |
|
| 230 | - recordSensorFixForCompetitor(competitor, event); |
|
| 231 | - } else { |
|
| 232 | - logger.log(Level.FINE, ()->"Could not record fix for boat because no competitor could be determined. Boat: " + boat); |
|
| 214 | + for (final Timed fix : fixes) { |
|
| 215 | + final TimePoint timePoint = fix.getTimePoint(); |
|
| 216 | + deviceMappings.forEachMappingOfDeviceIncludingTimePoint(device, fix.getTimePoint(), |
|
| 217 | + new Consumer<DeviceMappingWithRegattaLogEvent<WithID>>() { |
|
| 218 | + @Override |
|
| 219 | + public void accept(DeviceMappingWithRegattaLogEvent<WithID> mapping) { |
|
| 220 | + mapping.getRegattaLogEvent().accept(new MappingEventVisitor() { |
|
| 221 | + @Override |
|
| 222 | + public void visit(RegattaLogDeviceCompetitorSensorDataMappingEvent event) { |
|
| 223 | + recordSensorFixForCompetitor(event.getMappedTo(), event); |
|
| 233 | 224 | } |
| 234 | - } |
|
| 235 | - |
|
| 236 | - private void recordSensorFixForCompetitor(Competitor competitor, RegattaLogDeviceMappingEvent<?> event) { |
|
| 237 | - if (!preemptiveStopRequested.get()) { |
|
| 238 | - @SuppressWarnings("unchecked") |
|
| 239 | - SensorFixMapper<SensorFix, DynamicSensorFixTrack<Competitor, SensorFix>, Competitor> mapper = sensorFixMapperFactory |
|
| 240 | - .createCompetitorMapper((Class<? extends RegattaLogDeviceMappingEvent<?>>) event.getClass()); |
|
| 241 | - DynamicSensorFixTrack<Competitor, SensorFix> track = mapper.getTrack(trackedRace, competitor); |
|
| 242 | - if (track != null && trackedRace.isWithinStartAndEndOfTracking(fix.getTimePoint())) { |
|
| 243 | - mapper.addFix(track, (DoubleVectorFix) fix); |
|
| 244 | - if (returnLiveDelay) { |
|
| 245 | - delayToLive.put(trackedRace.getRaceIdentifier(), new MillisecondsDurationImpl(trackedRace.getDelayToLiveInMillis())); |
|
| 246 | - } |
|
| 225 | + |
|
| 226 | + @Override |
|
| 227 | + public void visit(RegattaLogDeviceBoatSensorDataMappingEvent event) { |
|
| 228 | + final Boat boat = event.getMappedTo(); |
|
| 229 | + final Competitor competitor = trackedRace.getCompetitorOfBoat(boat); |
|
| 230 | + if (competitor != null) { |
|
| 231 | + recordSensorFixForCompetitor(competitor, event); |
|
| 232 | + } else { |
|
| 233 | + logger.log(Level.FINE, ()->"Could not record fix for boat because no competitor could be determined. Boat: " + boat); |
|
| 247 | 234 | } |
| 248 | 235 | } |
| 249 | - } |
|
| 250 | - |
|
| 251 | - @Override |
|
| 252 | - public void visit(RegattaLogDeviceCompetitorMappingEvent event) { |
|
| 253 | - recordForCompetitor(event.getMappedTo()); |
|
| 254 | - } |
|
| 255 | - |
|
| 256 | - @Override |
|
| 257 | - public void visit(RegattaLogDeviceBoatMappingEvent event) { |
|
| 258 | - final Boat boat = event.getMappedTo(); |
|
| 259 | - final Competitor comp = trackedRace.getCompetitorOfBoat(boat); |
|
| 260 | - if (comp != null) { |
|
| 261 | - recordForCompetitor(comp); |
|
| 262 | - } else { |
|
| 263 | - // this is not necessarily something to warn of; while a boat tracker may continuously track |
|
| 264 | - logger.log(Level.FINE, |
|
| 265 | - ()->"Could not record fix for boat because no competitor could be determined. Boat: " + boat); |
|
| 266 | - } |
|
| 267 | - } |
|
| 268 | - |
|
| 269 | - private void recordForCompetitor(Competitor comp) { |
|
| 270 | - if (!preemptiveStopRequested.get()) { |
|
| 271 | - if (fix instanceof GPSFixMoving) { |
|
| 272 | - // try to record the fix, and only if it was really to the track, |
|
| 273 | - // check for maneuvers; otherwise, the fix may not have been accepted |
|
| 274 | - // by the race or the track, e.g., because the race's end-of-tracking |
|
| 275 | - // comes before the fix's time point |
|
| 276 | - if (trackedRace.recordFix(comp, (GPSFixMoving) fix)) { // TOOD bug6229: this checks the TrackedRace's tracking interval, but for an MDI we'd also want to intersect with Event/Regatta end date if set |
|
| 277 | - if (returnManeuverChanges) { |
|
| 278 | - RegattaAndRaceIdentifier maneuverChangedAnswer = detectIfManeuverChanged(comp); |
|
| 279 | - if (maneuverChangedAnswer != null) { |
|
| 280 | - maneuverChanged.add(maneuverChangedAnswer); |
|
| 281 | - } |
|
| 282 | - } |
|
| 236 | + |
|
| 237 | + private void recordSensorFixForCompetitor(Competitor competitor, RegattaLogDeviceMappingEvent<?> event) { |
|
| 238 | + if (!preemptiveStopRequested.get()) { |
|
| 239 | + @SuppressWarnings("unchecked") |
|
| 240 | + SensorFixMapper<SensorFix, DynamicSensorFixTrack<Competitor, SensorFix>, Competitor> mapper = sensorFixMapperFactory |
|
| 241 | + .createCompetitorMapper((Class<? extends RegattaLogDeviceMappingEvent<?>>) event.getClass()); |
|
| 242 | + DynamicSensorFixTrack<Competitor, SensorFix> track = mapper.getTrack(trackedRace, competitor); |
|
| 243 | + if (track != null && trackedRace.isWithinStartAndEndOfTracking(fix.getTimePoint())) { |
|
| 244 | + mapper.addFix(track, (DoubleVectorFix) fix); |
|
| 283 | 245 | if (returnLiveDelay) { |
| 284 | 246 | delayToLive.put(trackedRace.getRaceIdentifier(), new MillisecondsDurationImpl(trackedRace.getDelayToLiveInMillis())); |
| 285 | 247 | } |
| 286 | 248 | } |
| 287 | - } else { |
|
| 288 | - logger.log(Level.WARNING, |
|
| 289 | - String.format( |
|
| 290 | - "Could not add fix for competitor (%s) in race (%s), as it" |
|
| 291 | - + " is no GPSFixMoving, meaning it is missing COG/SOG values", |
|
| 292 | - comp, trackedRace.getRace().getName())); |
|
| 293 | 249 | } |
| 294 | 250 | } |
| 295 | - } |
|
| 296 | - |
|
| 297 | - @Override |
|
| 298 | - public void visit(RegattaLogDeviceMarkMappingEvent event) { |
|
| 299 | - if (!preemptiveStopRequested.get()) { |
|
| 300 | - Mark mark = event.getMappedTo(); |
|
| 301 | - final DynamicGPSFixTrack<Mark, GPSFix> markTrack = trackedRace.getOrCreateTrack(mark); |
|
| 302 | - final GPSFix firstFixAtOrAfter; |
|
| 303 | - final boolean forceFix; |
|
| 304 | - if (trackedRace.isWithinStartAndEndOfTracking(fix.getTimePoint())) { |
|
| 305 | - forceFix = false; |
|
| 251 | + |
|
| 252 | + @Override |
|
| 253 | + public void visit(RegattaLogDeviceCompetitorMappingEvent event) { |
|
| 254 | + recordForCompetitor(event.getMappedTo()); |
|
| 255 | + } |
|
| 256 | + |
|
| 257 | + @Override |
|
| 258 | + public void visit(RegattaLogDeviceBoatMappingEvent event) { |
|
| 259 | + final Boat boat = event.getMappedTo(); |
|
| 260 | + final Competitor comp = trackedRace.getCompetitorOfBoat(boat); |
|
| 261 | + if (comp != null) { |
|
| 262 | + recordForCompetitor(comp); |
|
| 306 | 263 | } else { |
| 307 | - markTrack.lockForRead(); |
|
| 308 | - try { |
|
| 309 | - if (Util.isEmpty(markTrack.getRawFixes()) |
|
| 310 | - || (firstFixAtOrAfter = markTrack.getFirstFixAtOrAfter(timePoint)) != null |
|
| 311 | - && firstFixAtOrAfter.getTimePoint().equals(timePoint)) { |
|
| 312 | - // either the first fix or overwriting an existing one |
|
| 313 | - forceFix = true; |
|
| 314 | - } else { |
|
| 315 | - // checking if the given fix is "better" than an existing one |
|
| 316 | - TimePoint startOfTracking = trackedRace.getStartOfTracking(); |
|
| 317 | - TimePoint endOfTracking = trackedRace.getEndOfTracking(); |
|
| 318 | - if (startOfTracking != null) { |
|
| 319 | - GPSFix fixAfterStartOfTracking = markTrack |
|
| 320 | - .getFirstFixAtOrAfter(startOfTracking); |
|
| 321 | - if (fixAfterStartOfTracking == null |
|
| 322 | - || !trackedRace.isWithinStartAndEndOfTracking( |
|
| 323 | - fixAfterStartOfTracking.getTimePoint())) { |
|
| 324 | - // There is no fix in the tracking interval, so this fix could be "better" |
|
| 325 | - // than ones already available in the track |
|
| 326 | - // Better means closer before/after the beginning/end of the tracking |
|
| 327 | - // interval |
|
| 328 | - if (timePoint.before(startOfTracking)) { |
|
| 329 | - // check if it is closer to the beginning of the tracking interval |
|
| 330 | - GPSFix fixBeforeStartOfTracking = markTrack |
|
| 331 | - .getLastFixAtOrBefore(startOfTracking); |
|
| 332 | - forceFix = (fixBeforeStartOfTracking == null |
|
| 333 | - || fixBeforeStartOfTracking.getTimePoint().before(timePoint)); |
|
| 334 | - } else if (endOfTracking != null && timePoint.after(endOfTracking)) { |
|
| 335 | - // check if it is closer to the end of the tracking interval |
|
| 336 | - GPSFix fixAfterEndOfTracking = markTrack |
|
| 337 | - .getFirstFixAtOrAfter(endOfTracking); |
|
| 338 | - forceFix = (fixAfterEndOfTracking == null |
|
| 339 | - || fixAfterEndOfTracking.getTimePoint().after(timePoint)); |
|
| 264 | + // this is not necessarily something to warn of; while a boat tracker may continuously track |
|
| 265 | + logger.log(Level.FINE, |
|
| 266 | + ()->"Could not record fix for boat because no competitor could be determined. Boat: " + boat); |
|
| 267 | + } |
|
| 268 | + } |
|
| 269 | + |
|
| 270 | + private void recordForCompetitor(Competitor comp) { |
|
| 271 | + if (!preemptiveStopRequested.get()) { |
|
| 272 | + if (fix instanceof GPSFixMoving) { |
|
| 273 | + // try to record the fix, and only if it was really to the track, |
|
| 274 | + // check for maneuvers; otherwise, the fix may not have been accepted |
|
| 275 | + // by the race or the track, e.g., because the race's end-of-tracking |
|
| 276 | + // comes before the fix's time point |
|
| 277 | + if (trackedRace.recordFix(comp, (GPSFixMoving) fix)) { // TOOD bug6229: this checks the TrackedRace's tracking interval, but for an MDI we'd also want to intersect with Event/Regatta end date if set |
|
| 278 | + if (returnManeuverChanges) { |
|
| 279 | + RegattaAndRaceIdentifier maneuverChangedAnswer = detectIfManeuverChanged(comp); |
|
| 280 | + if (maneuverChangedAnswer != null) { |
|
| 281 | + maneuverChanged.add(maneuverChangedAnswer); |
|
| 282 | + } |
|
| 283 | + } |
|
| 284 | + if (returnLiveDelay) { |
|
| 285 | + delayToLive.put(trackedRace.getRaceIdentifier(), new MillisecondsDurationImpl(trackedRace.getDelayToLiveInMillis())); |
|
| 286 | + } |
|
| 287 | + } |
|
| 288 | + } else { |
|
| 289 | + logger.log(Level.WARNING, |
|
| 290 | + String.format( |
|
| 291 | + "Could not add fix for competitor (%s) in race (%s), as it" |
|
| 292 | + + " is no GPSFixMoving, meaning it is missing COG/SOG values", |
|
| 293 | + comp, trackedRace.getRace().getName())); |
|
| 294 | + } |
|
| 295 | + } |
|
| 296 | + } |
|
| 297 | + |
|
| 298 | + @Override |
|
| 299 | + public void visit(RegattaLogDeviceMarkMappingEvent event) { |
|
| 300 | + if (!preemptiveStopRequested.get()) { |
|
| 301 | + Mark mark = event.getMappedTo(); |
|
| 302 | + final DynamicGPSFixTrack<Mark, GPSFix> markTrack = trackedRace.getOrCreateTrack(mark); |
|
| 303 | + final GPSFix firstFixAtOrAfter; |
|
| 304 | + final boolean forceFix; |
|
| 305 | + if (trackedRace.isWithinStartAndEndOfTracking(fix.getTimePoint())) { |
|
| 306 | + forceFix = false; |
|
| 307 | + } else { |
|
| 308 | + markTrack.lockForRead(); |
|
| 309 | + try { |
|
| 310 | + if (Util.isEmpty(markTrack.getRawFixes()) |
|
| 311 | + || (firstFixAtOrAfter = markTrack.getFirstFixAtOrAfter(timePoint)) != null |
|
| 312 | + && firstFixAtOrAfter.getTimePoint().equals(timePoint)) { |
|
| 313 | + // either the first fix or overwriting an existing one |
|
| 314 | + forceFix = true; |
|
| 315 | + } else { |
|
| 316 | + // checking if the given fix is "better" than an existing one |
|
| 317 | + TimePoint startOfTracking = trackedRace.getStartOfTracking(); |
|
| 318 | + TimePoint endOfTracking = trackedRace.getEndOfTracking(); |
|
| 319 | + if (startOfTracking != null) { |
|
| 320 | + GPSFix fixAfterStartOfTracking = markTrack |
|
| 321 | + .getFirstFixAtOrAfter(startOfTracking); |
|
| 322 | + if (fixAfterStartOfTracking == null |
|
| 323 | + || !trackedRace.isWithinStartAndEndOfTracking( |
|
| 324 | + fixAfterStartOfTracking.getTimePoint())) { |
|
| 325 | + // There is no fix in the tracking interval, so this fix could be "better" |
|
| 326 | + // than ones already available in the track |
|
| 327 | + // Better means closer before/after the beginning/end of the tracking |
|
| 328 | + // interval |
|
| 329 | + if (timePoint.before(startOfTracking)) { |
|
| 330 | + // check if it is closer to the beginning of the tracking interval |
|
| 331 | + GPSFix fixBeforeStartOfTracking = markTrack |
|
| 332 | + .getLastFixAtOrBefore(startOfTracking); |
|
| 333 | + forceFix = (fixBeforeStartOfTracking == null |
|
| 334 | + || fixBeforeStartOfTracking.getTimePoint().before(timePoint)); |
|
| 335 | + } else if (endOfTracking != null && timePoint.after(endOfTracking)) { |
|
| 336 | + // check if it is closer to the end of the tracking interval |
|
| 337 | + GPSFix fixAfterEndOfTracking = markTrack |
|
| 338 | + .getFirstFixAtOrAfter(endOfTracking); |
|
| 339 | + forceFix = (fixAfterEndOfTracking == null |
|
| 340 | + || fixAfterEndOfTracking.getTimePoint().after(timePoint)); |
|
| 341 | + } else { |
|
| 342 | + forceFix = false; |
|
| 343 | + } |
|
| 340 | 344 | } else { |
| 345 | + // there is already a fix in the tracking interval |
|
| 341 | 346 | forceFix = false; |
| 342 | 347 | } |
| 343 | 348 | } else { |
| 344 | - // there is already a fix in the tracking interval |
|
| 345 | 349 | forceFix = false; |
| 346 | 350 | } |
| 347 | - } else { |
|
| 348 | - forceFix = false; |
|
| 349 | 351 | } |
| 352 | + } finally { |
|
| 353 | + markTrack.unlockAfterRead(); |
|
| 350 | 354 | } |
| 351 | - } finally { |
|
| 352 | - markTrack.unlockAfterRead(); |
|
| 353 | 355 | } |
| 354 | - } |
|
| 355 | - trackedRace.recordFix(mark, (GPSFix) fix, /* only when in tracking interval */ !forceFix); |
|
| 356 | - if (returnLiveDelay) { |
|
| 357 | - delayToLive.put(trackedRace.getRaceIdentifier(), new MillisecondsDurationImpl(trackedRace.getDelayToLiveInMillis())); |
|
| 356 | + trackedRace.recordFix(mark, (GPSFix) fix, /* only when in tracking interval */ !forceFix); |
|
| 357 | + if (returnLiveDelay) { |
|
| 358 | + delayToLive.put(trackedRace.getRaceIdentifier(), new MillisecondsDurationImpl(trackedRace.getDelayToLiveInMillis())); |
|
| 359 | + } |
|
| 358 | 360 | } |
| 359 | 361 | } |
| 360 | - } |
|
| 361 | - }); |
|
| 362 | - } |
|
| 363 | - }); |
|
| 362 | + }); |
|
| 363 | + } |
|
| 364 | + }); |
|
| 365 | + } |
|
| 364 | 366 | } |
| 365 | 367 | return mergeManeuverChangedAndLiveDelayResult(maneuverChanged, delayToLive); |
| 366 | 368 | } |
| ... | ... | @@ -878,6 +880,11 @@ public class FixLoaderAndTracker implements TrackingDataLoader { |
| 878 | 880 | addLoadingJob(new LoadFixesForNewlyCoveredTimeRangesJob(item, newlyCoveredTimeRanges)); |
| 879 | 881 | } |
| 880 | 882 | } |
| 883 | + |
|
| 884 | + @Override |
|
| 885 | + public String toString() { |
|
| 886 | + return "FixLoaderDeviceMappings for race "+trackedRace.getRaceIdentifier(); |
|
| 887 | + } |
|
| 881 | 888 | } |
| 882 | 889 | |
| 883 | 890 | /** |
java/com.sap.sailing.domain.racelogtrackingadapter/src/com/sap/sailing/domain/racelogtracking/impl/fixtracker/RegattaLogDeviceMappings.java
| ... | ... | @@ -4,6 +4,7 @@ import java.util.ArrayList; |
| 4 | 4 | import java.util.Collections; |
| 5 | 5 | import java.util.HashMap; |
| 6 | 6 | import java.util.HashSet; |
| 7 | +import java.util.LinkedList; |
|
| 7 | 8 | import java.util.List; |
| 8 | 9 | import java.util.Map; |
| 9 | 10 | import java.util.Set; |
| ... | ... | @@ -69,6 +70,39 @@ public abstract class RegattaLogDeviceMappings<ItemT extends WithID> { |
| 69 | 70 | private final Map<ItemT, List<DeviceMappingWithRegattaLogEvent<ItemT>>> mappings = new HashMap<>(); |
| 70 | 71 | private final Map<DeviceIdentifier, List<DeviceMappingWithRegattaLogEvent<ItemT>>> mappingsByDevice = new HashMap<>(); |
| 71 | 72 | |
| 73 | + /** |
|
| 74 | + * A cache that holds the device mappings as the {@link Pair#getB() second} component of the values in this map, |
|
| 75 | + * such that exactly these device mappings apply for any time point {@link TimeRange#includes(TimePoint) included} |
|
| 76 | + * by the {@link TimeRange} that is the {@link Pair#getA() first} component of a value in this map. This map's keys |
|
| 77 | + * match with the {@link DeviceMapping#getDevice() device identifiers} of the {@link Pair#getB() second} components |
|
| 78 | + * of their corresponding values. |
|
| 79 | + * <p> |
|
| 80 | + * |
|
| 81 | + * This cache is designed to work well for cases where mappings change at a frequency orders of magnitude less than |
|
| 82 | + * the frequency with which fixes arrive and are to be mapped to items. Furthermore, the cache hit rates benefit |
|
| 83 | + * from mappings covering large time ranges. |
|
| 84 | + * <p> |
|
| 85 | + * |
|
| 86 | + * Any change to the mappings for a device will remove the mapping for the device's {@link DeviceIdentifier |
|
| 87 | + * identifier} from this map. |
|
| 88 | + * <p> |
|
| 89 | + * |
|
| 90 | + * Access to this map has to undergo the same locking drill as any access to {@link #mappings}, using the |
|
| 91 | + * {@link #mappingsLock}. |
|
| 92 | + */ |
|
| 93 | + private final Map<DeviceIdentifier, Pair<TimeRange, List<DeviceMappingWithRegattaLogEvent<ItemT>>>> cachedMappings = new HashMap<>(); |
|
| 94 | + |
|
| 95 | + /** |
|
| 96 | + * When the {@link #cachedMappings} are to be updated, the corresponding update job is stored in this field. It must be executed |
|
| 97 | + * under the {@link #mappingsLock}'s write lock. However, when other updates to {@link #mappings} or {@link #mappingsByDevice} |
|
| 98 | + * are performed (usually by the {@link #updateMappingsInternal()} method), this field will be cleared because the cache update |
|
| 99 | + * will most likely be obsolete. |
|
| 100 | + */ |
|
| 101 | + private Runnable cacheUpdateJob; |
|
| 102 | + |
|
| 103 | + private int cacheHits; |
|
| 104 | + private int cacheMisses; |
|
| 105 | + |
|
| 72 | 106 | private final RegattaLogEventVisitor regattaLogEventVisitor = new BaseRegattaLogEventVisitor() { |
| 73 | 107 | @Override |
| 74 | 108 | public void visit(RegattaLogDeviceCompetitorSensorDataMappingEvent event) { |
| ... | ... | @@ -156,7 +190,13 @@ public abstract class RegattaLogDeviceMappings<ItemT extends WithID> { |
| 156 | 190 | |
| 157 | 191 | /** |
| 158 | 192 | * Calls the given callback for every DeviceMapping that is known for the given {@link DeviceIdentifier} that |
| 159 | - * includes the given {@link TimePoint}. |
|
| 193 | + * includes the given {@link TimePoint}.<p> |
|
| 194 | + * |
|
| 195 | + * Searches the {@link #cachedMappings} for a match for the {@code device}; if found, checks whether {@code timePoint} |
|
| 196 | + * is within the time range for which device mappings were cached, and if so, uses those device mappings. Otherwise, |
|
| 197 | + * the device mappings are calculated and a cache update is carried out after releasing the read-lock of |
|
| 198 | + * {@link #mappingsLock} and after obtaining its write-lock. Should an update have been squeezed in between releasing |
|
| 199 | + * the read-lock and obtaining the write-lock, the cache update is not carried out. |
|
| 160 | 200 | * |
| 161 | 201 | * @param device |
| 162 | 202 | * the device to get the mappings for |
| ... | ... | @@ -169,15 +209,51 @@ public abstract class RegattaLogDeviceMappings<ItemT extends WithID> { |
| 169 | 209 | public void forEachMappingOfDeviceIncludingTimePoint(DeviceIdentifier device, TimePoint timePoint, |
| 170 | 210 | Consumer<DeviceMappingWithRegattaLogEvent<ItemT>> callback) { |
| 171 | 211 | LockUtil.executeWithReadLock(mappingsLock, () -> { |
| 172 | - List<DeviceMappingWithRegattaLogEvent<ItemT>> mappingsForDevice = mappingsByDevice.get(device); |
|
| 173 | - if (mappingsForDevice != null) { |
|
| 174 | - for (DeviceMappingWithRegattaLogEvent<ItemT> mapping : mappingsForDevice) { |
|
| 175 | - if (mapping.getTimeRange().includes(timePoint)) { |
|
| 176 | - callback.accept(mapping); |
|
| 212 | + final Pair<TimeRange, List<DeviceMappingWithRegattaLogEvent<ItemT>>> cachedTimeRangeForDevice = cachedMappings.get(device); |
|
| 213 | + if (cachedTimeRangeForDevice != null && cachedTimeRangeForDevice.getA().includes(timePoint)) { |
|
| 214 | + cacheHits++; |
|
| 215 | + logger.fine(() -> "Device mapping cache hit for mapper " + this + " for device " + device |
|
| 216 | + + " and time point " + timePoint + ", included in cached range " |
|
| 217 | + + cachedTimeRangeForDevice.getA() + "; " + cacheHits + " hits, " + cacheMisses + " misses"); |
|
| 218 | + cachedTimeRangeForDevice.getB().forEach(mapping->callback.accept(mapping)); |
|
| 219 | + } else { |
|
| 220 | + final List<DeviceMappingWithRegattaLogEvent<ItemT>> mappingsForDevice = mappingsByDevice.get(device); |
|
| 221 | + TimeRange timeRangeForCache = null; |
|
| 222 | + final List<DeviceMappingWithRegattaLogEvent<ItemT>> deviceMappingsForCache = new LinkedList<>(); |
|
| 223 | + cacheMisses++; |
|
| 224 | + if (mappingsForDevice != null) { |
|
| 225 | + for (final DeviceMappingWithRegattaLogEvent<ItemT> mapping : mappingsForDevice) { |
|
| 226 | + if (mapping.getTimeRange().includes(timePoint)) { |
|
| 227 | + if (timeRangeForCache == null) { |
|
| 228 | + timeRangeForCache = mapping.getTimeRange(); |
|
| 229 | + } else { |
|
| 230 | + timeRangeForCache = timeRangeForCache.intersection(mapping.getTimeRange()); |
|
| 231 | + } |
|
| 232 | + deviceMappingsForCache.add(mapping); |
|
| 233 | + callback.accept(mapping); |
|
| 234 | + } |
|
| 177 | 235 | } |
| 178 | 236 | } |
| 237 | + final TimeRange finalTimeRangeForCache = timeRangeForCache; |
|
| 238 | + logger.fine(() -> "Device mapping cache miss for mapper " + this + " for device " + device |
|
| 239 | + + " and time point " + timePoint + ", determined cachable range " |
|
| 240 | + + finalTimeRangeForCache + "; " + cacheHits + " hits, " + cacheMisses + " misses"); |
|
| 241 | + if (timeRangeForCache != null) { |
|
| 242 | + cacheUpdateJob = ()->cachedMappings.put(device, new Pair<>(finalTimeRangeForCache, deviceMappingsForCache)); |
|
| 243 | + } |
|
| 179 | 244 | } |
| 180 | 245 | }); |
| 246 | + if (cacheUpdateJob != null) { |
|
| 247 | + LockUtil.executeWithWriteLock(mappingsLock, ()->{ |
|
| 248 | + if (cacheUpdateJob != null) { |
|
| 249 | + logger.fine(()-> "Device mapping cache miss for mapper " + this + " performs cache update."); |
|
| 250 | + cacheUpdateJob.run(); |
|
| 251 | + } else { |
|
| 252 | + logger.fine(() -> "Device mapping cache miss for mapper " + this |
|
| 253 | + + " does not update the cache because the mappings were updated in between"); |
|
| 254 | + } |
|
| 255 | + }); |
|
| 256 | + } |
|
| 181 | 257 | } |
| 182 | 258 | |
| 183 | 259 | public void forEachItemAndCoveredTimeRanges(final BiConsumer<ItemT, Map<RegattaLogDeviceMappingEvent<ItemT>, MultiTimeRange>> consumer) { |
| ... | ... | @@ -245,8 +321,10 @@ public abstract class RegattaLogDeviceMappings<ItemT extends WithID> { |
| 245 | 321 | oldMappings.putAll(mappings); |
| 246 | 322 | oldDeviceIds.addAll(mappingsByDevice.keySet()); |
| 247 | 323 | mappings.clear(); |
| 324 | + cachedMappings.clear(); |
|
| 248 | 325 | mappings.putAll(newMappings); |
| 249 | 326 | mappingsByDevice.clear(); |
| 327 | + cacheUpdateJob = null; |
|
| 250 | 328 | for (ItemT item : newMappings.keySet()) { |
| 251 | 329 | for (DeviceMappingWithRegattaLogEvent<ItemT> mapping : newMappings.get(item)) { |
| 252 | 330 | List<DeviceMappingWithRegattaLogEvent<ItemT>> list = mappingsByDevice.get(mapping.getDevice()); |
java/com.sap.sailing.domain/src/com/sap/sailing/domain/racelog/tracking/FixReceivedListener.java
| ... | ... | @@ -1,5 +1,7 @@ |
| 1 | 1 | package com.sap.sailing.domain.racelog.tracking; |
| 2 | 2 | |
| 3 | +import java.util.Collections; |
|
| 4 | + |
|
| 3 | 5 | import com.sap.sailing.domain.common.DeviceIdentifier; |
| 4 | 6 | import com.sap.sailing.domain.common.RegattaAndRaceIdentifier; |
| 5 | 7 | import com.sap.sse.common.Duration; |
| ... | ... | @@ -12,6 +14,7 @@ import com.sap.sse.common.Util.Triple; |
| 12 | 14 | * @param <FixT> |
| 13 | 15 | * the type of fixes this listener can consume. |
| 14 | 16 | */ |
| 17 | +@FunctionalInterface |
|
| 15 | 18 | public interface FixReceivedListener<FixT extends Timed> { |
| 16 | 19 | /** |
| 17 | 20 | * |
| ... | ... | @@ -34,6 +37,32 @@ public interface FixReceivedListener<FixT extends Timed> { |
| 34 | 37 | * returned can be empty but is never {@code null}. It can also contain multiple identifiers if the device |
| 35 | 38 | * mapping is currently ambiguous. |
| 36 | 39 | */ |
| 37 | - Iterable<Triple<RegattaAndRaceIdentifier, Boolean, Duration>> fixReceived(DeviceIdentifier device, FixT fix, |
|
| 40 | + default Iterable<Triple<RegattaAndRaceIdentifier, Boolean, Duration>> fixReceived(DeviceIdentifier device, FixT fix, |
|
| 41 | + boolean returnManeuverChanges, boolean returnLiveDelay) { |
|
| 42 | + return fixesReceived(device, Collections.singleton(fix), returnManeuverChanges, returnLiveDelay); |
|
| 43 | + } |
|
| 44 | + |
|
| 45 | + /** |
|
| 46 | + * |
|
| 47 | + * @param device |
|
| 48 | + * the device that recorded the fix. Cannot be <code>null</code>. |
|
| 49 | + * @param fixes |
|
| 50 | + * The fixes that were stored. Must not be <code>null</code> but may be empty |
|
| 51 | + * @param returnLiveDelay |
|
| 52 | + * if {@code true} then all listeners to which the fixes are forwarded shall check to which races the fix |
|
| 53 | + * maps and report the live delay for the newest of those {@code fixes} for all those races as the third |
|
| 54 | + * component of the resulting {@link Triple}s. |
|
| 55 | + * @param returnManeuverUpdate |
|
| 56 | + * if {@code true}, all listeners to which this fixes are forwarded shall check whether the fixes feed |
|
| 57 | + * into a competitor's track in the scope of a race where for that competitor the maneuver list has |
|
| 58 | + * changed since the last call of this type; if so, the race identifier will be part of the result, with |
|
| 59 | + * the {@link Boolean} component being {@code true} for that race. Otherwise, the {@link Boolean} |
|
| 60 | + * component is {@code false} or the race is not listed in the result. |
|
| 61 | + * @return An {@link Iterable} with {@link RegattaAndRaceIdentifier}s is returned that will contain races with new |
|
| 62 | + * maneuvers which were not available at the last time the given device stored a fix. The {@link Iterable} |
|
| 63 | + * returned can be empty but is never {@code null}. It can also contain multiple identifiers if the device |
|
| 64 | + * mapping is currently ambiguous. |
|
| 65 | + */ |
|
| 66 | + Iterable<Triple<RegattaAndRaceIdentifier, Boolean, Duration>> fixesReceived(DeviceIdentifier device, Iterable<FixT> fixes, |
|
| 38 | 67 | boolean returnManeuverChanges, boolean returnLiveDelay); |
| 39 | 68 | } |
java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/client/StringMessages.properties
| ... | ... | @@ -580,7 +580,7 @@ factor=Factor |
| 580 | 580 | errorUpdatingIsMedalRace=Error updating the medal race setting: {0} |
| 581 | 581 | maneuverLoss=Maneuver loss |
| 582 | 582 | averageManeuverLossInMeters=\u2205 Maneuver Loss |
| 583 | -averageManeuverLossInMetersTooltip=The average distance loss during any mneuver. |
|
| 583 | +averageManeuverLossInMetersTooltip=The average distance loss during any maneuver. |
|
| 584 | 584 | averageTackLossInMeters=\u2205 Tack Loss |
| 585 | 585 | averageTackLossInMetersTooltip=The average distance loss during tacks. |
| 586 | 586 | averageJibeLossInMeters=\u2205 Jibe Loss |
| ... | ... | @@ -637,7 +637,7 @@ polarSheetRemoveOutliersTooltip=Outliers are removed by a distance based approac |
| 637 | 637 | polarSheetOutlierDetectionRadius=Outlier detection neighborhood radius |
| 638 | 638 | polarSheetOutlierDetectionRadiusTooltip=The radius used in the distance based outlier\ndetection algorithm. |
| 639 | 639 | polarSheetOutlierDetectionMinimumPerc=Minimum outlier detection neighborhood percentage |
| 640 | -polarSheetOutlierDetectionMinimumPercTooltip=The percentage of neightborhood datapoints / overall datapoints\nneeds to be at less than the given value for the point to be considered an outlier. |
|
| 640 | +polarSheetOutlierDetectionMinimumPercTooltip=The percentage of neighborhood datapoints / overall datapoints\nneeds to be at less than the given value for the point to be considered an outlier. |
|
| 641 | 641 | polarSheetNumberOfHistogramColumns=Number of histogram columns per wind range |
| 642 | 642 | polarSheetWindSteppingInKnots=Wind stepping in knots |
| 643 | 643 | polarSheetWindSteppingMaxDistance=Max. distance to wind level |
| ... | ... | @@ -1021,7 +1021,7 @@ addIgtimiUser=Add Igtimi User |
| 1021 | 1021 | errorTryingToRemoveIgtimiDevice=Error trying to remove Igtimi device {0}: {1} |
| 1022 | 1022 | successfullyRemoveIgtimiDevice=Igtimi device {0} removed successfully |
| 1023 | 1023 | errorCreatingDataAccessWindow=Error creating data access window for Igtimi device {0}: {1} |
| 1024 | -successfullyCreatedIgtimiDataAccessWindow=Data access window for Igtimi devic {0} created successfully |
|
| 1024 | +successfullyCreatedIgtimiDataAccessWindow=Data access window for Igtimi device {0} created successfully |
|
| 1025 | 1025 | errorFetchingIgtimiDataAccessWindows=Error fetching Igtimi data access windows: {0} |
| 1026 | 1026 | doYouReallyWantToRemoveTheSelectedIgtimiDataAccessWindows=Do you really want to remove the selected Igtimi data access windows? |
| 1027 | 1027 | igtimiDataAccessWindows=Igtimi Data Access Windows |
| ... | ... | @@ -1525,7 +1525,7 @@ showOnlyCompetitorsOfLog=Show only competitors registered in Log |
| 1525 | 1525 | showOnlyBoatsOfLog=Show only boats registered in Log |
| 1526 | 1526 | confirmLosingCompetitorEditsWhenTogglingLogBasedView=This will reset your current, unsaved changes to the competitors.\nDo you really want to continue? |
| 1527 | 1527 | confirmLosingBoatEditsWhenTogglingLogBasedView=This will reset your current, unsaved changes to the boats.\nDo you really want to continue? |
| 1528 | -removalOfMarkDisabledMayBeUsedInRaces=Removal deavticated, marks are used in races {0} |
|
| 1528 | +removalOfMarkDisabledMayBeUsedInRaces=Removal deactivated, marks are used in races {0} |
|
| 1529 | 1529 | createDefaultLeaderboardGroup=Would you like to create a default LeaderboardGroup for your event? |
| 1530 | 1530 | pleaseCreateAtLeastOneMappingBy=Please create at least one mapping by first selecting a track on the left hand side and then selecting a mark, boat or competitor on the right hand side |
| 1531 | 1531 | matcherType=Matcher type |
| ... | ... | @@ -2464,8 +2464,8 @@ errorLoadingPolarDataForBoatClass=Error loading polar data for boat class {0}: { |
| 2464 | 2464 | videoGuide=Video guide |
| 2465 | 2465 | orcExplanation=ORC explanation |
| 2466 | 2466 | confirmNonRenewingSubscriptionTitle=Stop auto-renewal |
| 2467 | -confirmNonRenewingSubscriptionText=Do you really want to stop the auto-renewal of your subscription?\n You will loose your aquired premium privileges at the end of the current term. |
|
| 2468 | -subscriptionOneTimePlanLockedText=This plan may only be aquired once. |
|
| 2467 | +confirmNonRenewingSubscriptionText=Do you really want to stop the auto-renewal of your subscription?\n You will loose your acquired premium privileges at the end of the current term. |
|
| 2468 | +subscriptionOneTimePlanLockedText=This plan may only be acquired once. |
|
| 2469 | 2469 | errorSettingRaceToDefineItsOwnCompetitors=Error setting race for fleet {2} in column {1} of leaderboard {0} to define its own competitors: {3} |
| 2470 | 2470 | twdInDegrees=TWD (from) in degrees |
| 2471 | 2471 | legDirectionInDegrees=Leg direction in degrees |
| ... | ... | @@ -2578,7 +2578,7 @@ selectRaceColumnsWhosePairingsToCopy=Race columns whose pairings to copy |
| 2578 | 2578 | errorCopyingPairings=Error copying pairings: {0} |
| 2579 | 2579 | successfullyCopiedPairings=Successfully copied pairings |
| 2580 | 2580 | selectFromRaceColumn=Select race column from where to start copying pairings |
| 2581 | -selectToRaceColumn=Select race colunm to where to copy pairings |
|
| 2581 | +selectToRaceColumn=Select race column to where to copy pairings |
|
| 2582 | 2582 | exportTWAHistogramToCsv=Export True Wind Angle histogram to CSV |
| 2583 | 2583 | exportWindSpeedHistogramToCsv=Export Wind Speed histogram to CSV |
| 2584 | 2584 | optionalBearerTokenForWindImport=Optional bearer token for wind import |
java/com.sap.sse.datamining.ui/src/main/java/com/sap/sse/datamining/ui/client/StringMessages.properties
| ... | ... | @@ -2,7 +2,7 @@ dataMiningSettings=Data Mining Settings |
| 2 | 2 | runPredefinedQuery=Run Predefined Query |
| 3 | 3 | viewQueryDefinition=View Query Definition |
| 4 | 4 | selectPredefinedQuery=Select Predefined Query |
| 5 | -errorRunningDataMiningQuery=An error occured running the query |
|
| 5 | +errorRunningDataMiningQuery=An error occurred running the query |
|
| 6 | 6 | predefinedQueryRunner=Predefined Query Runner |
| 7 | 7 | useClassGetNameTooltip=More robust against changes in the code base, but the code snippet can only be used in the scope where the classes are available. |
| 8 | 8 | useClassGetName=Use Class.getName() for type names |
java/com.sap.sse.filestorage/resources/stringmessages/FileStorageStringMessages.properties
| ... | ... | @@ -2,6 +2,6 @@ s3Desc=Store files on Amazon S3. The resulting file name is a random UUID plus t |
| 2 | 2 | s3AccessIdDesc=Access ID (leave blank to use ~/.aws/credentials instead) |
| 3 | 3 | s3AccessKeyDesc=Secret Access Key (leave blank to use ~/.aws/credentials instead) |
| 4 | 4 | s3BucketNameDesc=Name of Bucket to use (has to already exist, and user needs sufficient permissions) |
| 5 | -localDesc=Service for storing files in the local file system. Files get stored in localPath+fileName and can be accessed at baseUrl+fileName Note that the content lying in baseUrl must therefore be accessible remotely. This can for examplebe achieved by mounting a remote file system for example from static.sapsailing.com on the replicas to localPath. |
|
| 5 | +localDesc=Service for storing files in the local file system. Files get stored in localPath+fileName and can be accessed at baseUrl+fileName Note that the content lying in baseUrl must therefore be accessible remotely. This can for example be achieved by mounting a remote file system for example from static.sapsailing.com on the replicas to localPath. |
|
| 6 | 6 | localBaseUrlDesc=Base URL + fileName = URL where uploaded file will be accessible |
| 7 | 7 | localLocalPathDesc=Local Path to use for file storage |
java/com.sap.sse.gwt.adminconsole/src/com/sap/sse/gwt/adminconsole/StringMessages.properties
| ... | ... | @@ -44,9 +44,9 @@ explainReplicationServletPort=Servlet port number that is used to connect to reg |
| 44 | 44 | setUpStorageService=Please set up a file storage service first |
| 45 | 45 | usernameAndPasswordMustBothBeSet=Username and password must both be filled or empty |
| 46 | 46 | username=User name |
| 47 | -explainUserName=Username to use for authentification on the master |
|
| 47 | +explainUserName=Username to use for authentication on the master |
|
| 48 | 48 | password=Password |
| 49 | -explainPassword=Password to use for authentification on the master |
|
| 49 | +explainPassword=Password to use for authentication on the master |
|
| 50 | 50 | additionalInformation=Info |
| 51 | 51 | serverInformation=Server Information |
| 52 | 52 | serverName=Server Name: {0} |
java/target/configuration/logging_debug.properties
| ... | ... | @@ -68,4 +68,7 @@ com.sap.sse.landscape.impl.GithubReleasesRepository.level = FINE |
| 68 | 68 | |
| 69 | 69 | # Produce wind-from-maneuver estimation graph: |
| 70 | 70 | #com.sap.sailing.windestimation.integration.IncrementalMstHmmWindEstimationForTrackedRaceTest.level = FINE |
| 71 | -#com.sap.sailing.windestimation.integration.IncrementalMstHmmWindEstimationForTrackedRace.level = FINE |
|
| ... | ... | \ No newline at end of file |
| 0 | +#com.sap.sailing.windestimation.integration.IncrementalMstHmmWindEstimationForTrackedRace.level = FINE |
|
| 1 | + |
|
| 2 | +# Log device mapping caching |
|
| 3 | +com.sap.sailing.domain.racelogtracking.impl.fixtracker.RegattaLogDeviceMappings.level = FINE |