Identity/set update should work on existing server-set Identities

[quantranhong1999]

Why

Users should be able to customize server-set identities.

Requires

• Memory IdentityRepository and contract tests
• Implement Identity/set update (custom identities) #4429 (loosely coupled => we can separate this issue into 2 PR: 1 PR solved update existing at Repository layer and 1 PR for testing update them at JMAP level)

How

• Merge server-set identities and custom identities at Repository layer (list API). Here is a hint how to merge them:

  • Inject IdentityRepository to IdentityGetMethod.
  • Now we have two List<Identity> from IdentityFactory and IdentityRepository . We need to merge it.
    • Get List<Identity> from custom identities first, call it listCustomIdentities.
    • From listCustomIdentities we get it ids, call it idsCustomIdentities.
    • Get listServerIdentities, filter which one is not in idsCustomIdentities (this avoid having 2 duplicated record from server identities (maybe outdated one) and custom identities (maybe updated one), we just care about updated one) then we get listServerIdentitiesFiltered
    • Merge listCustomIdentities and listServerIdentitiesFiltered

Then filter element equals input Identity Id: If present call do updateX(Username username, IdentityId id, Identity newIdentity), if empty return notUpdated.
And this updateX API call DAO' update API which should just be an insert into query.

• Plug and test updating on server-set identities at JMAP layer.

DoD

• Test for Repository layer
• Test for JMAP contract

Note

• User is not able to update email, mayDelete

[quantranhong1999]

Should we move the merging stuff to IdentityRepository?

• Avoid duplicate code
• If we do merging stuff upon JMAP level, IdentityRepository update API has to be just an insert query without checking if present cause it may not know about some other server Identites. By injecting IdentityFactory to IdentityRepository, we can check if present at the Repository level.

[chibenwa]

Now we have two List from IdentityFactory and IdentityRepository . We need to merge it. See how to merge at Plug custom identities store to Identity/get.

This means that we have identities with Ids in both places (auto provisionned ones and the repo) correct?

-> This means that the repo need to store Ids being a sha256 in base64. So not just UUIDs. We thus need to adapt the Cassandra storage accordingly.
-> Also we need a method to save identities with a given ID (as the id of the existing identity is assigned by the JMAP layer)

[quantranhong1999]

This means that the repo need to store Ids being a sha256 in base64. So not just UUIDs. We thus need to adapt the Cassandra storage accordingly.

How about just change server-set Identities id from sha256 to UUID.fromString(address.asString) ?

[quantranhong1999]

Also we need a method to save identities with a given ID (as the id of the existing identity is assigned by the JMAP layer)

How about only 1 update API at IdentityRepository for all kinds of updates:

Publisher<Void> update(Username username, IdentityId id, Identity newIdentity);

[chibenwa]

How about just change server-set Identities id from sha256 to UUID.fromString(address.asString) ?

UUID.fromString(address.asString) -> this will not work as you expect :-) (as it only accepts string at the UUID structure). nameUUIDFromBytes sounds more like what you expess.

With hash keep in mind hash collision chances. UUID::nameUUIDFromBytes is backed by MD5, known to be prown to hash collisions.

Here the risk is low (as the set of address is pre-defined) so your proposal ikely make perfect sense but I would oppose it if the user could gain control on the hash generation...

[chibenwa]

How about only 1 update API at IdentityRepository for all kinds of updates ?

This leads to much more duplicated code, no?

Should we move the merging stuff to IdentityRepository?

Then when we have business logic, we should have a separate DAO layer.

Given that the dao layer blindly stores ids generated by the repository layer, this would make sense.

[quantranhong1999]

How about only 1 update API at IdentityRepository for all kinds of updates ?

This leads to much more duplicated code, no?

Where is the more duplicated code you mean?
When do update we always Filter Identity which its id equals input Identity Id. If present => create new Identity.withX(X newX) => update(Username username, Identity newIdentity).
And this update API should just be an insert into query.