Migration of the cf-java-client dependency by karrgov · Pull Request #1907 · cloudfoundry/multiapps-controller

karrgov · 2026-09-03T13:41:16Z

Migration of the cf-java-client dependency

LMCROSSITXSADEPLOY-3563

… from poms LMCROSSITXSADEPLOY-3054

LMCROSSITXSADEPLOY-3563

sonarqubecloud · 2026-09-15T09:16:40Z

Quality Gate passed

Issues
0 New issues
12 Accepted issues

Measures
0 Security Hotspots
80.6% Coverage on New Code
0.3% Duplication on New Code

See analysis details on SonarQube Cloud

Yavor16 · 2026-09-17T12:05:33Z

+    public String getValue() {
+        return name().toLowerCase();
+    }
+
    @Override
    public String toString() {
-        return this.name().toLowerCase();
+        return this.name()
+                   .toLowerCase();
    }


Why don't you use just of the two methods because they are the same?

you are right, fixed

Yavor16 · 2026-09-17T12:10:42Z

+        this.labels = labels == null ? Collections.emptyMap() : Collections.unmodifiableMap(new LinkedHashMap<>(labels));
+        this.annotations = annotations == null ? Collections.emptyMap() : Collections.unmodifiableMap(new LinkedHashMap<>(annotations));


Can you use Map.copyOf instead of Collections.unmodifiableMap(new LinkedHashMap<>(...)?

Have thought about this, but wanted to be a bit more sure, since copyOf() does not allow the key value pairs to have nulls, while the unmodifiableMap() allows it.

Yavor16 · 2026-09-17T12:11:41Z

+        public Builder labels(Map<String, String> labels) {
+            if (labels != null) {
+                this.labels.putAll(labels);
+            }
+
+            return this;
+        }
+
+        public Builder putAllLabels(Map<String, String> labels) {
+            return labels(labels);
+        }


Can you leave only one method because are the same but with differnet name

fixed, thanks

Yavor16 · 2026-09-17T12:11:51Z

+        public Builder annotations(Map<String, String> annotations) {
+            if (annotations != null) {
+                this.annotations.putAll(annotations);
+            }
+
+            return this;
+        }
+
+        public Builder putAllAnnotations(Map<String, String> annotations) {
+            return annotations(annotations);
+        }


Yavor16 · 2026-09-17T12:18:19Z

+public record V3Application(@JsonProperty("guid") String guid, @JsonProperty("name") String name, @JsonProperty("state") String state,
+                            @JsonProperty("created_at") String createdAt, @JsonProperty("updated_at") String updatedAt,
+                            @JsonProperty("lifecycle") V3Lifecycle lifecycle, @JsonProperty("metadata") V3Metadata metadata,
+                            @JsonProperty("relationships") V3Relationships relationships) {


Can we extract these string in a constant calss because they are repeated more in other classes

fixed, thanks

Yavor16 · 2026-09-18T08:45:24Z

+        }
+    }
+
+    @SuppressWarnings("unchecked")


Remove this

Yavor16 · 2026-09-18T08:45:34Z

+            return (Map<String, Object>) JsonUtil.convertJsonToMap(response.body())
+                                                 .get(Constants.ROOT_DOCUMENT_LINKS_LIST);


Use micsutil cast

Yavor16 · 2026-09-18T08:46:13Z

+            requestTags.forEach(requestBuilder::header);
+            HttpResponse<String> response = getSimpleHttpClient().send(requestBuilder.build(), HttpResponse.BodyHandlers.ofString());
+
+            if (response.statusCode() / 100 != 2) {


I think if you add a range will be easier to understand :D

fixed, thanks :D

Yavor16 · 2026-09-18T08:47:00Z

+    }
+
+    @SuppressWarnings("unchecked")
+    private Map<String, Object> getCloudControllerRootDocumentLinks(URL controllerUrl, Map<String, String> requestTags) {


This method has a lot of nested structure and is long. Extract to new methods

Yavor16 · 2026-09-18T08:49:14Z

+import reactor.netty.http.client.HttpClient;

 @Value.Immutable
 public abstract class CloudControllerRestClientFactory {


I think this class does many things like creating a lot of client and extracting url-s. Can you move some of the methods to a new class

Yavor16 · 2026-09-18T10:03:27Z

+        V3ListResponse<R> firstPage = restClient.get()
+                                                .uri(firstPageRelativeUri)
+                                                .retrieve()
+                                                .body(pageType);


I would extract this to a new method because is it deplicated in a few places

Yavor16 · 2026-09-18T10:10:02Z

+    }
+
+    private static CloudOperationException jobFailed(V3Job job) {
+        String detail = "Job failed";


Extract this as a message and maybe add more details if possible

Yavor16 · 2026-09-18T10:10:24Z

+            detail = job.errors()
+                        .stream()
+                        .map(V3Job.V3Error::detail)
+                        .reduce((a, b) -> a + "\n" + b)


Find better names instead of a and b

Yavor16 · 2026-09-18T10:11:28Z

+    private static void sleep(Duration interval) {
+        try {
+            Thread.sleep(interval.toMillis());
+        } catch (InterruptedException e) {
+            Thread.currentThread()
+                  .interrupt();
+            throw new CloudException(MessageFormat.format(Messages.INTERRUPTED_WHILE_POLLING_ASYNC_JOB_0, e.getMessage()), e);
+        }
+    }


in MiscUtil we have method for sleep

Yavor16 · 2026-09-18T10:13:43Z

+            long elapsedNanos = System.nanoTime() - startTimeNanos;
+            if (elapsedNanos >= timeoutNanos) {
+                throw new CloudOperationException(HttpStatus.GATEWAY_TIMEOUT, Messages.JOB_TIMEOUT,
+                                                  MessageFormat.format(Messages.JOB_0_DID_NOT_COMPLETE_WITHIN_1, jobGuid, timeout));
+            }
+
+            long remainingNanos = timeoutNanos - elapsedNanos;


Can you move these to a new method

Yavor16 · 2026-09-18T11:46:38Z

+          .toBodilessEntity();
+    }
+
+    @SuppressWarnings("unchecked")


Yavor16 · 2026-09-18T11:47:07Z

+    }
+
+    private void assertSpaceProvided(String operation) {
+        Assert.notNull(target, "Unable to " + operation + " without specifying organization and space to use.");


Yavor16 · 2026-09-18T11:49:28Z

+        V3ServiceBinding createdServiceKey = getServiceKeyResourceByNameAndServiceInstanceGuid(keyModel.getName(),
+                                                                                               serviceInstance.getGuid());


Cant you get the id from the post request instead of making new request

Yavor16 · 2026-09-18T11:51:15Z

+        String query = CloudControllerV3Endpoints.SERVICE_CREDENTIAL_BINDINGS + CloudControllerV3Endpoints.QUERY_PER_PAGE
+            + CloudControllerV3Endpoints.DEFAULT_PAGE_SIZE + CloudControllerV3Endpoints.AMPERSAND_TYPE + "key"
+            + CloudControllerV3Endpoints.AMPERSAND_SERVICE_INSTANCE_GUIDS + serviceInstanceGuid + CloudControllerV3Endpoints.AMPERSAND_NAMES
+            + name;


Yavor16 · 2026-09-18T11:52:29Z

+@Target(ElementType.METHOD)
+@Retention(RetentionPolicy.CLASS)
+public @interface AllowNulls {


What the difference between this and nullable?

well if talk about collections, if a map is declared as Nullable, this only tells us that we allow the whole map to be null, but if we need to allow for the values of a pair inside the map to hold a null, (for example "someKey" : null) - in order to achieve this we need this custom annotation AllowNulls, since before the migration we used the already provided one from the cf-java-client dependency. Thats how it was directly in the cf-java-client:

karrgov added 6 commits September 3, 2026 16:45

CF Java Client PoC migration

e957b22

LMCROSSITXSADEPLOY-3563

Removing more leftovers, changing logging and removing cf-java-client…

6874258

… from poms LMCROSSITXSADEPLOY-3054

Adding reactor-netty back not completed

f273ed5

LMCROSSITXSADEPLOY-3563

Adding more fixes for the migration

2fd10a4

LMCROSSITXSADEPLOY-3563

Fix test

860fb56

LMCROSSITXSADEPLOY-3563

Bettering everything as a whole - simplifications and logic changes

698462b

LMCROSSITXSADEPLOY-3563

karrgov force-pushed the cf_java_client_poc branch from 2b06745 to 698462b Compare September 3, 2026 13:57

Adding unit tests for coverage and fixing Sonar

f3bf20d

LMCROSSITXSADEPLOY-3563

github-advanced-security Bot found potential problems Sep 10, 2026

View reviewed changes

Comment thread ...g/cloudfoundry/multiapps/controller/client/facade/rest/CloudControllerRestClientBuilder.java Fixed

Comment thread ...g/cloudfoundry/multiapps/controller/client/facade/rest/CloudControllerRestClientBuilder.java Fixed

karrgov added 2 commits September 11, 2026 19:09

Adding more unit tests and fixing existing ones

463ee28

LMCROSSITXSADEPLOY-3563

Fixing sonar issues and additional unit tests for coverage

245f56c

LMCROSSITXSADEPLOY-3563

Yavor16 requested changes Sep 18, 2026

View reviewed changes

		this.labels = labels == null ? Collections.emptyMap() : Collections.unmodifiableMap(new LinkedHashMap<>(labels));
		this.annotations = annotations == null ? Collections.emptyMap() : Collections.unmodifiableMap(new LinkedHashMap<>(annotations));

		return (Map<String, Object>) JsonUtil.convertJsonToMap(response.body())
		.get(Constants.ROOT_DOCUMENT_LINKS_LIST);

		V3ServiceBinding createdServiceKey = getServiceKeyResourceByNameAndServiceInstanceGuid(keyModel.getName(),
		serviceInstance.getGuid());

Conversation

karrgov commented Sep 3, 2026

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Sep 15, 2026

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants