Support @Version and optimistic lock

[belovaf]

Elide doesn't count @Version as a special field and just overrides it from patch request (RecordTerminalState#patch method). This behavior violates hibernate contract (only the persistence provider is permitted to set or update the value of the version attribute in the object) and brakes optimistic lock mechanism.

[DennisMcWherter]

Good catch. I think we missed this; it is in the spec JPA here. The spec says this:

The Version annotation specifies the version field or property of an entity class that serves as its optimistic lock value. The version is used to ensure integrity when performing the merge operation and for optimistic concurrency control. Only a single Version property or field should be used per class; applications that use more than one Version property or field will not be portable. The Version property should be mapped to the primary table for the entity class; applications that map the Version property to a table other than the primary table will not be portable. In general, fields or properties that are specified with the Version annotation should not be updated by the application.[123] The following types are supported for version properties: int, Integer, short, Short, long, Long, Timestamp.

My proposal is to simply have elide exclude any field marked with @Version unless you feel there is a reason that the API client-code may want to read this value. Even in this case, it would be trivial to configure a @ComputedAttribute that mirrors this value for read-only consumption by the client.

I can work up a quick PR soon to do this, but in the meantime this can be emulated by adding an @Exclude to that field.

Let me know if that doesn't fix your problem!

[belovaf]

I think it would be better if Elide will throw exception if client send patch request with stale resource version. With this approach clients can decide use/not use optimistic locking. If you just exclude @Version field, clients will lost optimistic locking.

[belovaf]

This snippet of code (very naive implementation with hardcoded version attribute name) solves my problem:

Optional<String> resourceVersion =
          Optional.ofNullable(resource.getAttributes())
                  .map((attributes) -> attributes.get("version"))
                  .map(Object::toString);

 if (resourceVersion.isPresent()) {
        
     Optional<String> recordVersion =
              Optional.ofNullable(record.getAttribute("version"))
                      .map(Object::toString);

      if (!resourceVersion.equals(recordVersion)) {
          throw new StaleStateException(
                  "Resource(type=" + resource.getType() + ", id=" + resource.getId() + ", version=" + resourceVersion + ", ...)" +
                          " is stale. Current version=" + recordVersion);
      }
  }

If you agree with this approach, I can make pull request with generic solution.

[DennisMcWherter]

Your approach appears mostly sane to me. The change I would really make is to find the @Version annotation properly on an entity. Logically, that code belongs in EntityBinding#bindAttr or a helper method around it (to store that metadata).

Then, you would just want to expose this through the EntityDictionary. Perhaps a method such as String getVersionField(Class<?> entityClass) or boolean isVersionField(Class<?> entityClass, String fieldName) may be sufficient.

Once these methods are exposed, you can actually take a dictionary instance and check:

if (requestedField.equals(dictionary.getVersionField(entityClass))) {
  // ...
}

[belovaf]

Looks very good! I'll be waiting for optimistic locking feature. :)

[clayreimann]

@belovaf Thanks for the input, as this is not a feature we use we aren't actively working on it. We're happy to look at any PRs you do put together though!

If you agree with this approach, I can make pull request with generic solution.