`GigaMap.update` / `apply` silently removes the entity on constraint violation

[fh-ms]

Summary

When a ConstraintViolationException is raised during GigaMap.update(current, logic) or GigaMap.apply(current, logic), the entity is removed from the map and the exception is rethrown. The caller's current reference is then no longer mapped — even though, from the caller's point of view, the operation just "failed validation".

The behavior is not symmetric with add / set / replace, which all leave the map's state intact on violation. In a typical persistent application this is a real data-loss footgun: a UI form save, a REST handler, or a batch update that hits a unique constraint will delete the record it was trying to validate, not just refuse the change.

The javadoc on update and apply now calls this out, so newly written code can at least see the warning. That is a stop-gap; it does not change the asymmetry, and existing call sites written before the doc fix remain at risk.

Reproduction

final GigaMap<Entity> map = GigaMap.New();
map.index().bitmap().ensure(Entity.wordIndex);
map.constraints().custom().addConstraint(new NoBadValues());

map.add(Entity.Random().setWord("a"));
final Entity b = Entity.Random().setWord("b");
map.add(b);
assertEquals(2, map.size());

try {
    map.update(b, entity -> entity.setWord("BAD"));
} catch (final ConstraintViolationException expected) {
    // expected
}

// Surprise: `b` is gone, even though the "update" was rejected.
assertEquals(1, map.size());
assertFalse(map.contains(b)); // passes

The same pattern shows up in production code that does
gigaMap.update(person, p -> p.setEmail(newEmail)) against a unique-email constraint — a duplicate email deletes the person.

Where the behavior lives

gigamap/gigamap/src/main/java/org/eclipse/store/gigamap/types/GigaMap.java, in Default.apply:

catch(final ConstraintViolationException e)
{
    this.ensureClearedAddingState();

    /*
     * Since a state change done by a custom logic cannot be rolled back, there is no other choice
     * but to remove the entity from the map and report the entity and entityId to the calling
     * context in the exception.
     */
    this.internalRemove(entityId);
    throw e;
}

Why it currently behaves this way

The mutation logic is a Consumer<E> / Function<E,R> that mutates the entity in place before the indices and constraints are re-evaluated. By the time a violation is detected the entity object already holds the new (rejected) state, and the original state is gone. Without a snapshot of the previous state, the implementation has three options:

1. Leave the corrupted entity in the map (silent corruption — worst).
2. Remove it (current behavior — visible, but data-loss-prone).
3. Snapshot/restore (correct, but requires deep-copy or some other rollback mechanism).

Behavior asymmetry across CRUD methods

Operation	On constraint violation
add / addAll	Map unchanged, no entity is added.
set / replace	Map unchanged, previous entity preserved (checked before mutate).
update / apply	Offending entity is removed from the map.

A user reasoning about "constraint violation = my change is rejected" is therefore correct for three out of four entry points and wrong for the fourth.

Recovery surface today

The thrown ConstraintViolationException carries entityId, replacedEntity, and violatingEntity, so a caller who knows about the removal can re-add or restore. This is fine as a contract — but it is undocumented and not at all obvious from the method signatures.

Discussion / possible mitigations

Not prescribing a fix, but options worth discussing:

1. Snapshot-based rollback — clone or shallow-copy the entity before invoking the logic, restore on violation. Costs allocations and assumes entities are cloneable; could be opt-in via an overload apply(current, logic, RollbackStrategy.SNAPSHOT).
2. Distinct exception type — e.g. EntityRemovedOnConstraintViolationException extending ConstraintViolationException, so callers can pattern-match on the post-state without relying on javadoc. Would communicate the data loss explicitly at the type level.
3. safeUpdate(current, logic) helper — applies logic to a clone, then routes through set(id, clone). Gets set-style semantics (no removal) at the cost of an extra allocation. Could live alongside the existing update rather than replacing it.
4. Constraint pre-check via dry-run — only viable for stateless constraints; not a general fix.

Asks

• Decide whether to introduce a non-destructive variant (a safeUpdate-style helper or an opt-in rollback strategy).
• Decide whether to introduce a distinct exception subclass for the "entity was removed" case so the data loss is visible at the type level rather than only in the javadoc.
• Add an explicit test that asserts the post-state of the map after an update violation, so the contract is locked in either way.