Provide option to skip a modification in the "twin" if the value "is equal"

[thjaeckle]

The default behaviour of Ditto is for each "ModifyCommand" to apply the modification and also the twin in the MongoDB.
This could be optimised:

• the existing behaviour should be the same (everything else would be an API break)
• there is an option (e.g. passed via header) to define "only update this value if it is not equal to the provided one"
• using "JSON equality" Ditto could check for the equality of the passed value (on all levels)
• if the value is equal (according to JSON equality)
  • the new value is not applied to the twin (persistence)
  • a DittoRuntimeException with status 304 (not modified) is returned
  • no event is emitted

Header name suggestion:

• if-equal: update/skip
  • similar to specified HTTP header "if-match"
  • to be read as: update resource even if new value is equal to existing value
  • current default: update

To consider:

• what about merge/PATCH requests?
  • I would assume that for each modification in the patch the equality check is applied.
  • so a patch with if-equal: skip would only patch provided values differing from existing ones.
  • only if no single entry was applied, a patch with that option would be responded to with 304 (not modified)
• order of json arrays
  • I would consider 2 json arrays with the same entries, but different order as different
  • as the order of a json array could also be used to declare eg a priority

[jufickel-b]

The idea to optimise is definitely welcome. The only thing that I worry about a bit is returning a DittoRuntimeException because from a semantic point of view the modification did not fail. Technically it would be possible to simply add 304 to the HTTP status codes of a regular modification response. The response does not imply that anything was changed.

TL;DR I suggest to favour a regular modification response (with status 304) over an error response.

[thjaeckle]

We already do that - do you suggest to change the existing exceptions as well?

ditto/things/model/src/main/java/org/eclipse/ditto/things/model/signals/commands/exceptions/ThingPreconditionNotModifiedException.java

Lines 52 to 58 in 229ff8c

	private ThingPreconditionNotModifiedException(final DittoHeaders dittoHeaders,
	@Nullable final String message,
	@Nullable final String description,
	@Nullable final Throwable cause,
	@Nullable final URI href) {
	super(ERROR_CODE, HttpStatus.NOT_MODIFIED, dittoHeaders, message, description, cause, href);
	}

ditto/policies/model/src/main/java/org/eclipse/ditto/policies/model/signals/commands/exceptions/PolicyPreconditionNotModifiedException.java

Lines 52 to 58 in 229ff8c

	private PolicyPreconditionNotModifiedException(final DittoHeaders dittoHeaders,
	@Nullable final String message,
	@Nullable final String description,
	@Nullable final Throwable cause,
	@Nullable final URI href) {
	super(ERROR_CODE, HttpStatus.NOT_MODIFIED, dittoHeaders, message, description, cause, href);
	}

[jufickel-b]

Then, I guess, we should stick to the established approach. (Although, I would be surprised to receive an exception for an idempotent action that actually produced the effect it was supposed to produce.)

[thjaeckle]

At the HTTP API, you will already get a HTTP 304 - with an "Error payload", but that can easily be ignored :)
At protocol level I agree, there it is semantically strange to receive an "error".

We can look into improving this when tackling the issue ..

[jufickel-b]

By the way, the HTTP API documentation does not state, that 304 is a possible result for modification (at least not for putting an attribute or a feature property) and thus also not the "error payload". So, formally, it would not be an API break if we changed it for HTTP as well, would it?

[thjaeckle]

For the HTTP API it probably would not be an API break - but for the DittoProtocol channels it would.

edit: Maybe we could add another header which would state that "not modified" shall not be responded as exception/error, but as "success". Or we could implicitly do that for the new "equality check" header... that would be a new API functionality and therefore not a break.