allthingssecurity commented on code in PR #27802:
URL: https://github.com/apache/camel/pull/27802#discussion_r4267168035
##########
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:
Agreed. I checked the callers: inside Camel `getInstance` is only reached
through the 2-arg `BonitaAPI` constructor,
which nothing calls; `BonitaStartProducer` goes through
`BonitaAPIBuilder.build`, which hands its own client to
`forClient`, so the producer path creates no client beyond the one it
created on main. Done in 88736bee44eb:
`BonitaAPIUtil.getInstance` and `BonitaAPI(BonitaAPIConfig, WebTarget)` are
`@Deprecated(since = "4.24.0")`, and the
javadoc says that the client they create is never closed and points to
`forClient` and the 3-arg constructor.
`BonitaAPIUtilPrepareInputsTest` now uses `forClient` with a client it
closes. The client per exchange that
`BonitaStartProducer` builds and never closes is unchanged (as on main); I
will report it separately, as you suggest.
_Claude Code on behalf of allthingssecurity_
##########
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:
Done in 88736bee44eb: the part is named `"tempFile" +
ThreadLocalRandom.current().nextLong(Long.MAX_VALUE) + ".tmp"`
and no file is created, so the `File.delete()` is gone too. The request has
the same form as on main (one part,
`filename=tempFile<digits>.tmp`). The two tests that checked the deletion
are renamed (`uploadLeavesNoTemporaryFile`,
`...WhenTheUploadFails`); they still check that no file of that name exists
in `java.io.tmpdir` after the exchange.
camel-bonita: 22 run, 0 failures.
_Claude Code on behalf of allthingssecurity_
--
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]
