eTag support with If-Match

[bdemers]

Along with this we may want to tweak the text in the Repository.update methods, some of the nuance that goes along with this? This value could potentially be a comma separated list, or prefixed with W/. Maybe this is something that should be abstracted out?

As is it's still probably an improvement! But if we need to think about changing the API for this, we can (until we cut the official 1.0)

[symphony-enrico]

Maybe this is something that should be abstracted out?

In my repository implementation I process version in this way:

  /**
   * Version field may contain: null (If-Match header is not required), * (all version allowed,
   * same as null), a specific version or a comma separated version list (not very
   * useful, but it is possible according If-Match specifications)
   */
  private List<String> processVersion(String version) {
    if (StringUtils.isBlank(version) || "*".equals(version.trim())) {
      return null;
    }
    // Weak entity tag will be not processed correctly,
    // as they are prefixed W/. But this is not a problem
    // because they should not match for If-Match
    return Stream.of(version.split(","))
        .filter(StringUtils::isNotBlank)
        .map(String::trim)
        .map(etag -> etag.replaceAll("^\"|\"$", ""))
        .collect(Collectors.toList());
  }

then, if it is not null, I compare it with version of scim entity.

But if we want change APIs for repository inside framework, I can suggest, instead of:

  T update(String id, String version, T resource, Set<AttributeReference> includedAttributes, Set<AttributeReference> excludedAttributes) throws ResourceException;

We could pass:

  T update(String id, @Nullable List<ETag> ifMatch, T resource, Set<AttributeReference> includedAttributes, Set<AttributeReference> excludedAttributes) throws ResourceException;

where ETag POJO could be simply:

public class ETag {
  String value;
  boolean weak;
}

WDYT @bdemers ?

[bdemers]

@symphony-enrico I like it! I think the ETag object makes it clear what it's intent is!

Should the List be a Set? It probably doesn't matter much in this context, as uniqueness is less of an issue, but it would align with the other collection parameters. (though again, i'm not sure this needs to signal uniqueness). Thoughts?

[symphony-enrico]

@symphony-enrico I like it! I think the ETag object makes it clear what it's intent is!

Should the List be a Set? It probably doesn't matter much in this context, as uniqueness is less of an issue, but it would align with the other collection parameters. (though again, i'm not sure this needs to signal uniqueness). Thoughts?

Hi @bdemers I modified to a set, added a parser for header and modified repository apis according to this...

[bdemers]

This looks great @symphony-enrico! One last quick comment about the weak etag

[bdemers]

@symphony-enrico SCIMple's ETag support is an area that I think could be improved. (and this Pull Request is very. appreciated in moving that forward!!)

SCIMple basically has 2 types of resources:

• In Memory (Schema, Configuration, these are created when the application starts, and likely will not change)
• Repository controlled (e.g. the persistence of ScimUser/ScimGroup)

For the in memory items, the ETagGenerator should be used. For Repository controlled objects, it's up to the implementation, they can use the ETagGenerator, but they could also use something else (e.g. functionality tied to a database)

[!NOTE] This was relatively recent change (before the last release). For implementations that don't return all fields based on optimized lookups based on the attributes/excludedAttributes params using the ETagGenerator would have resulted in an incorrect version/etag. For example, if a ScimUser Repository that did NOT return group memberships, that data would be missing for hashing in the ETagGenerator.

All that being said, if you have thoughts around this, ways to make this easier, or think my above assumption is wrong. Please let us know!

[symphony-enrico]

Maybe this is something that should be abstracted out?

thank you, I understand. No, for now I dont have suggestions but I am just discovering the framework...

[symphony-enrico]

any chances to merge it (and release) if there are no blocking point? Thanks a lot @bdemers

[bdemers]

I should have some time later this week to get this merged and a release kicked off. (Well the merge part is easy, but I want to play around with it a little and write up something for the vote and release notes)

[symphony-enrico]

I should have some time later this week to get this merged and a release kicked off. (Well the merge part is easy, but I want to play around with it a little and write up something for the vote and release notes)

Hello @bdemers any news? Thanks!

[bdemers]

@symphony-enrico Sorry again for the delay, I hadn't forgot about this, it's a great addition to SCIMple!