From b0f14a3d505c12073f85e538e6795096d9a8f2ad Mon Sep 17 00:00:00 2001 From: Rob Rudin Date: Thu, 18 Sep 2025 16:00:57 -0400 Subject: [PATCH] MLE-23425 Better error when ManageConfig is null Also deprecating the RestTemplate constructors and the setters for ManageConfig and RestTemplate so that ManageConfig can be made final in 7.0. --- .../java/com/marklogic/mgmt/ManageClient.java | 47 ++++++++++--------- .../com/marklogic/mgmt/ManageClientTest.java | 20 ++++++-- 2 files changed, 42 insertions(+), 25 deletions(-) diff --git a/ml-app-deployer/src/main/java/com/marklogic/mgmt/ManageClient.java b/ml-app-deployer/src/main/java/com/marklogic/mgmt/ManageClient.java index 6959ee00..3bbcefca 100644 --- a/ml-app-deployer/src/main/java/com/marklogic/mgmt/ManageClient.java +++ b/ml-app-deployer/src/main/java/com/marklogic/mgmt/ManageClient.java @@ -18,7 +18,6 @@ import org.springframework.web.client.RestTemplate; import java.io.IOException; -import java.io.StringReader; import java.io.StringWriter; import java.net.URI; import java.util.ArrayList; @@ -37,41 +36,39 @@ public class ManageClient extends LoggingObject { private PayloadParser payloadParser; public ManageClient(ManageConfig config) { - setManageConfig(config); + this.manageConfig = config; + if (logger.isInfoEnabled()) { + logger.info("Initializing ManageClient with manage config of: {}", config); + } } /** - * Uses the given ManageConfig instance to construct a Spring RestTemplate for communicating with the Manage API. - * In addition, if adminUsername on the ManageConfig instance differs from username, then a separate RestTemplate is - * constructed for making calls to the Manage API that need user with the manage-admin and security roles, which is - * often an admin user. + * Deprecated in 6.0.1 with the intention of removing in 7.0.0 so that the underlying ManageConfig can be declared + * as final. * - * @param config + * @deprecated */ + @Deprecated(since = "6.0.1", forRemoval = true) public void setManageConfig(ManageConfig config) { this.manageConfig = config; - if (logger.isInfoEnabled()) { - logger.info("Initializing ManageClient with manage config of: " + config); - } } /** - * Use this when you want to provide your own RestTemplate as opposed to using the one that's constructed via a - * ManageConfig instance. - * - * @param restTemplate + * Deprecated in 6.0.1 as it will not work without a ManageConfig instance being set, which is then unlikely to + * be consistent with the given RestTemplate. + * @deprecated */ + @Deprecated(since = "6.0.1", forRemoval = true) public ManageClient(RestTemplate restTemplate) { this(restTemplate, restTemplate); } /** - * Use this when you want to provide your own RestTemplate as opposed to using the one that's constructed via a - * ManageConfig instance. - * - * @param restTemplate - * @param adminRestTemplate + * Deprecated in 6.0.1 as it will not work without a ManageConfig instance being set, which is then unlikely to + * be consistent with the given RestTemplate. + * @deprecated */ + @Deprecated(since = "6.0.1", forRemoval = true) public ManageClient(RestTemplate restTemplate, RestTemplate adminRestTemplate) { this.restTemplate = restTemplate; this.securityUserRestTemplate = adminRestTemplate; @@ -257,8 +254,10 @@ public HttpEntity buildXmlEntity(String xml) { protected void logRequest(String path, String contentType, String method) { if (logger.isInfoEnabled()) { - String username = String.format("as user '%s' ", manageConfig.getUsername()); - logger.info("Sending {} {} request {}to path: {}", contentType, method, username, buildUri(path)); + String username = manageConfig != null && StringUtils.hasText(manageConfig.getUsername()) ? + String.format("as user '%s' ", manageConfig.getUsername()) : ""; + URI uri = buildUri(path); + logger.info("Sending {} {} request {}to path: {}", contentType, method, username, uri); } } @@ -268,7 +267,8 @@ protected void logSecurityUserRequest(String path, String contentType, String me if (!"".equals(username)) { username = String.format("as user '%s' (who should have the 'manage-admin' and 'security' roles) ", username); } - logger.info("Sending {} {} request {}to path: {}", contentType, method, username, buildUri(path)); + URI uri = buildUri(path); + logger.info("Sending {} {} request {}to path: {}", contentType, method, username, uri); } } @@ -307,6 +307,7 @@ private void initializeSecurityUserRestTemplate() { } public URI buildUri(String path) { + Objects.requireNonNull(manageConfig, "A ManageConfig instance must be provided"); return manageConfig.buildUri(path); } @@ -321,6 +322,7 @@ public ManageConfig getManageConfig() { return manageConfig; } + @Deprecated(since = "6.0.1", forRemoval = true) public void setRestTemplate(RestTemplate restTemplate) { this.restTemplate = restTemplate; } @@ -332,6 +334,7 @@ public RestTemplate getSecurityUserRestTemplate() { return securityUserRestTemplate; } + @Deprecated(since = "6.0.1", forRemoval = true) public void setSecurityUserRestTemplate(RestTemplate restTemplate) { this.securityUserRestTemplate = restTemplate; } diff --git a/ml-app-deployer/src/test/java/com/marklogic/mgmt/ManageClientTest.java b/ml-app-deployer/src/test/java/com/marklogic/mgmt/ManageClientTest.java index 4ede515d..02151b16 100644 --- a/ml-app-deployer/src/test/java/com/marklogic/mgmt/ManageClientTest.java +++ b/ml-app-deployer/src/test/java/com/marklogic/mgmt/ManageClientTest.java @@ -3,13 +3,16 @@ */ package com.marklogic.mgmt; -import static org.junit.jupiter.api.Assertions.*; +import com.marklogic.mgmt.resource.databases.DatabaseManager; import org.junit.jupiter.api.Test; -public class ManageClientTest { +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class ManageClientTest { @Test - public void determineUsernameForSecurityUserRequest() { + void determineUsernameForSecurityUserRequest() { ManageConfig config = new ManageConfig("localhost", 8002, "someone", "someword"); config.setSecurityUsername("admin"); config.setSecurityPassword("admin"); @@ -20,4 +23,15 @@ public void determineUsernameForSecurityUserRequest() { config.setSecurityUsername(null); assertEquals("someone", client.determineUsernameForSecurityUserRequest()); } + + @Test + void nullManageConfig() { + ManageClient client = new ManageClient(null); + NullPointerException npe = assertThrows(NullPointerException.class, () -> new DatabaseManager(client).getAsXml()); + assertEquals("A ManageConfig instance must be provided", npe.getMessage(), + "It's possible to pass in null as the ManageConfig since there's still a setManageConfig method, but that's been " + + "deprecated so that it can be removed in 7.0.0. The goal is to have ManageConfig be final once " + + "it's set, and ideally hidden as well so that the ManageClient is effectively immutable. " + + "In the meantime, we expect a nice error message if the ManageConfig is null."); + } }