vaadin / collaboration-engine

The simplest way to build real-time collaboration into web apps
https://vaadin.com/collaboration
Other
3 stars 1 forks source link

Support general list operations in `CollaborationList` #45

Closed Legioth closed 2 years ago

Legioth commented 3 years ago

CollaborationList does currently only support appending values to the end of the list, but it doesn't support inserting values into other positions, replacing values or removing values.

All those missing operations need a way of referencing the list entry that is targeted. This cannot use the regular list index since any concurrent operation might cause the list to be reindexed. Instead, each list entry would need to have its own key that can be used to reference that entry in subsequent operations (there's also an option to use the value itself as a key, but that alternative doesn't allow having duplicates in the list). The entry key would be returned from insertion operations and also be available for iteration (based on a snapshot taken when iteration starts) as an alternative to iterating over only the values.

Technically, the key could be implemented e.g. as a randomly generated UUID or through fractional indexing. This is an opaque internal value that we should encapsulate in a value class to avoid leaking irrelevant implementation details. We could also consider a design where the value object is representing the whole entry so that e.g. update operations would be in the entry class instead of in the list class itself.

With the key as a value holder, the various operations would be like this:

CollaborationListKey key = ...;
list.set(key, "2");
list.replace(key, "2", "3"); // compare and swap
list.insertBefore(key, "4");
list.insertAfter(key, "5");
list.remove(key);
list.moveBefore(referenceKey, keyToMove);
list.moveAfter(referenceKey, keyToMove);
list.insertFirst("6");
list.insertLast("7"); // alternatively "append" which is currently defined but inconsistent with "insertFirst"

list.getKeys().forEach(key -> list.set(key, list.get(key, String.class) + " updated"));

All data manipulation operations should give a CompletableFuture that is used to signal when the operation is completed. All operations except inserting at the beginning or the end can fail in case the referenced key is no longer present in the list. The insertion operations should also return the key that is associated with the new entry, which leads to multiple bad API alternatives:

As code, the various alternatives would thus look like this:

list.insertAfter(locationKey, value).thenAccept(insertedKey -> {
  if (insertedKey == null) { handleError(); }
  else { handleSuccess(insertedKey); } 
});

list.insetAfter(locationKey, value).thenAccept(insertedKey -> {
  if (insertedKey.isInvalid()) { handleError(); }
  else { handleSuccess(insertedKey); }
});

list.insertAfter(locationKey, value)
  .thenAccept(insertedKey -> handleSuccess(insertedKey))
  .exceptionally(exception -> handleError());

InsertResult result = list.insertAfter(locationKey, value);
handleKey(result.getKey());
result.getCompletableFuture().thenAccept(success -> {
  if (success) { handleSuccess(result.getKey()); }
  else { handleError(); }
});

CollaborationListKey key = list.insertAfter(locationKey, value, result -> {
  if (result.isSuccess()) { handleSuccess(result.getKey()); }
  else { handleError(); }
});
handleKey(key);

With using the key as an entry with further operations, usage would instead be like this:

CollaborationListEntry entry = ...;
entry.set("2");
entry.replace("2", "3"); // compare and swap
entry.insertBefore("4");
entry.insertAfter("5");
entry.moveBefore(referenceKey);
entry.moveAfter(referenceKey);
entry.remove();
list.insertFirst("6");
list.insertLast("7");

list.getEntries().forEach(entry -> entry.set(entry.get(String.class) + " updated"));
Legioth commented 2 years ago

These are the potential outcomes for the different operations that we might want to support.

One potential way out of the problem with return values is to observe that only the various insert* operations may need to return multiple values (key and asynchronous status), whereas all the other operations only need to return an asynchronous status. We could thus let all the other operations return a CompletableFuture<T> where T is either a Boolean or an enum that helps clarify which of the different failure conditions were encountered. The insert operations would instead return a wrapper class (could be a record if we would use Java 16) with getters for the generated key and an appropriate CompletableFuture.

When it comes to CollaborationListKey versus CollaborationListEntry, it would seem that it's better to just use a key. With an entry, it would leave the room for ambiguity about whether the value in the entry is the latest live value or the value that was present when the entry was created. Using a key instead would help make that aspect clearer.

Legioth commented 2 years ago

The event delivered to subscribers would also need to be enhanced to be able to distinguish all the cases.