GrottoCenter / grottocenter-api / 41800033190 / 1

91%

develop: 91%

push

github

fix(caver): skip links created concurrently when merging a caver (#1922)

## 🤔 What

- `DELETE /api/v1/cavers/:id?entityId=:mergeId` no longer answers 500 `E_UNIQUE` when a link it merges into the target already exists.
- New `CaverService.mergeCollection(targetId, key, ids)` links records to a caver and skips the links that are already there, including links created by a concurrent request.
- The controller no longer loads the target with all seven collections populated; it only checks that the target exists.

Closes #1922

## 🤷‍♂️ Why

The controller read the target's links once, at the start of the request, then ran `TCaver.addToCollection` for the missing ids much later. When the same merge was submitted twice (the issue shows a 500 followed 0.85 s later by a 200 on the same path), the second request inserted links between the first one's read and its insert, and Waterline rejected the insert with `E_UNIQUE`. The merge should skip existing links, never fail with a 500.

## 🔍 How

- `mergeCollection` populates only the collection being merged, right before the insert, and deduplicates the source ids. This narrows the race window.
- If `addToCollection` still throws `E_UNIQUE`, the ids are added one by one, and the ids that are already linked are ignored. The missing links are still created. A warning is logged with the collection key and the ids, which answers the issue's "which key collided?" question next time.
- Any other error is rethrown.

## 🧪 Testing

- `test/integration/1_services/CaverService.test.js`, `mergeCollection()`:
  - only the missing records are linked;
  - nothing is inserted when every record is already linked;
  - a stubbed `addToCollection` simulates a concurrent request that links one entrance before the insert, then throws `E_UNIQUE`: the other entrance is still linked and no error escapes (#1922);
  - errors other than `E_UNIQUE` are rethrown.
- The existing caver route tests (delete and merge) still pass.
- `npm run lint`... (continued)

6505 of 7556 branches covered (86.09%)

Branch coverage included in aggregate %.

11975 of 13036 relevant lines covered (91.86%)

235186.97 hits per line