Using TempFiles breaks duplicateFiltering

[ChrisSchoe]

Hey guys, I love the idea of making it easier to use Camunda's rest API, especially for diagram deployment! However, this does not seem to work as intended at the moment:

TempFiles -> Random File Names -> Duplicate Check Broken

https://github.com/camunda-community-hub/camunda-platform-7-rest-client-java/blob/1670cf44d65aa37065554aae26e0bd55b192544f/camunda-engine-rest-client-openapi-springboot/src/main/java/org/camunda/community/rest/client/springboot/CamundaProcessAutodeployment.java#L75

Using TempFiles to temporarily store the (e.g.) .bpmn-files right before uploading them leads to the flags dulicateFiltering/deployChangedOnly not working properly. Explanation: Using TempFiles create files whose filename consists of the given prefix and suffix while also adding a (somewhat) unique random number at runtime. This number is then part of the filename and is different with every new startup of the application.

Unfortunately, this filename is then part of the resource being deployed via the rest api and then used to check whether or not the deployed diagram differs from the latest version. As these names will (virtually) always be different from the last deployment call (even if nothing has changed about the diagram itself), the API call will always create a new deployment despite deployChangedOnly being set (cf. ResourceManager::findLatestResourcesByDeploymentName within the Camunda RestApi code.)

A workaround

On possible way around this (while still using openapi generated clients) is to specify <library>resttemplate</library> as part of the ConfigOptions of the client generation, as these clients (also) accept ByteArrayInputStreams instead of files for multi part form parameters. Thus, files can be uploaded from within .jars without creating temp files.

A problem with the workaround and/or Camunda's open API spec

Using "resttemplate" as the client library carries its own problems, as the default serialization of the multipart form here leads to the boolean parameters deployChangedOnly and duplicateFiltering not being parsed correctly by the Rest API, thus necessitating additional steps to fix the serialization. (The OpenApi-Spec gives their type as 'boolean', which the opanepi generator interprets as Boolean. As this is a proper class, the values get serialized as application/json instead of text fields within the multipartform when using Spring Boot's Resttemplate with the default configuration. This is then not interpreted as intended by Camunda's Rest Api (cf. DeploymentRestServiceImpl::extractDuplicateFilteringForDeployment ), where only test values are read.) Currently, Camuna's Openapi spec does not specify that this value should be encoded as a string (cf. https://github.com/camunda/camunda-bpm-platform/blob/master/engine-rest/engine-rest-openapi/src/main/templates/models/org/camunda/bpm/engine/rest/dto/MultiFormDeploymentDto.ftl ). The API reference itself is quite clear about all values having to be encoded as plain text. Perhaps it might be a good idea to add the encoding to the Camunda's Openapi spec ( cf. https://swagger.io/docs/specification/describing-request-body/multipart-requests/ )? :) I am not sure, though, whether the generated client would pick up on that.

[berndruecker]

Howdy @ChrisSchoe - great issue (I mean in terms of details :-)).

We could double check if we can somehow access the proper name of the file (which I am not sure about, that's why there are UUID#s -https://github.com/camunda-community-hub/camunda-platform-7-rest-client-java/commit/d3ddc3d30eb2801dcdcdf251f9fd85f1aa27a3e5) - or probably we could do some kind of naming schema, that makes sure the names are the same every time it gets deployed? Like simply increasing a number (1.bpmn, 2.bpmn)? This is still brittle - but should normally work if there are no changes...

Otherwise, we might want to create an issue to improve the OpenAPI spec?

WDYT?

[turyanitsa]

can we use the checksum of a file to determine its name? In this case, if the file changes, there will be a new name, but if new bpmn are added, then this will not affect the name (unlike the serial number). For example

val md = MessageDigest.getInstance("MD5")
val r = DigestInputStream(resource.inputStream, md)
Base64.getEncoder().encodeToString(md.digest()) + ".bpmn"
//1B2M2Y8AsgTpgAmY7PhCfg==.bpmn

This is an alternative solution with its pros and cons.

[berndruecker]

I like the idea of a checksum of the file content - this should give us a stable name while not relying on any information from the resource. @turyanitsa - would you be motivated to adjust your PR for this change by any chance?

[turyanitsa]

@berndruecker , of course, I will do it with pleasure

[berndruecker]

@ChrisSchoe Do you agree that https://github.com/camunda-community-hub/camunda-platform-7-rest-client-java/pull/39 closes this issue as resolved ?

[ChrisSchoe]

Yeah, That looks great. I like the checksum idea to replace unavailable names, it looks much more resilient than random strings, and only changes if actual changes occurred. Good job implementing a fix this quickly, thanks!:)

Ps: once I'm back in the office next week, I might still create a corresponding issue on the open API spec as explained in my original post, but I'd consider this issue right here closed :)

[ChrisSchoe]

Fixed by #39

[berndruecker]

Great - thx @ChrisSchoe and @turyanitsa