davsclaus commented on code in PR #27802:
URL: https://github.com/apache/camel/pull/27802#discussion_r4266928518
##########
components/camel-bonita/src/main/java/org/apache/camel/component/bonita/api/util/BonitaAPIUtil.java:
##########
@@ -44,20 +43,31 @@
public class BonitaAPIUtil {
- private static BonitaAPIUtil instance;
private WebTarget webTarget;
+ /**
+ * Creates a helper that uploads to the Bonita server of the given
configuration, with the account of that
+ * configuration. Every call creates a new helper with a JAX-RS client of
its own; nothing is shared between
+ * configurations. {@code BonitaAPIBuilder} does not use this method: it
binds the helper to the client of the
+ * {@code BonitaAPI} that it builds, see {@link #forClient(Client,
BonitaAPIConfig)}.
+ */
public static BonitaAPIUtil getInstance(BonitaAPIConfig bonitaAPIConfig) {
Review Comment:
This method now creates a new JAX-RS client on every call, and nobody closes
that client. It is still called `getInstance`, which suggests a shared
instance. Inside Camel the only caller is the 2-arg `BonitaAPI(BonitaAPIConfig,
WebTarget)` constructor, and nothing calls that constructor anymore. Consider
marking both `@Deprecated` (pointing to `forClient`), so they can be removed
later.
##########
components/camel-bonita/src/main/java/org/apache/camel/component/bonita/api/util/BonitaAPIUtil.java:
##########
@@ -69,20 +79,21 @@ public UploadFileResponse uploadFile(
.resolveTemplate("processName", processDefinition.getName())
.resolveTemplate("processVersion",
processDefinition.getVersion());
+ // the temporary file only reserves a unique name for the part: the
content is sent from the byte array
File tempFile = Files.createTempFile("tempFile", ".tmp").toFile();
Review Comment:
Optional: the file is now only a unique name. A generated name such as
`"tempFile" + ThreadLocalRandom.current().nextLong(Long.MAX_VALUE) + ".tmp"`
sends the same disposition without creating and deleting an empty file in
`java.io.tmpdir` for each upload. It would also remove the ignored
`File.delete()` result, which SonarCloud reports (S899). The test pattern
`tempFile\d+\.tmp` still matches, and the temp-file assertion would have to go.
I am fine with keeping it as it is if you prefer the byte-identical request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
