From 404fbd2228ec6afbeafaf9942e837c394abcc338 Mon Sep 17 00:00:00 2001 From: Harshitha Sudhakar <111514477+harshithasudhakar@users.noreply.github.com> Date: Mon, 17 Jul 2023 14:04:38 +0530 Subject: [PATCH 01/15] Update ClusterDataImpl.java Checking if `serviceUrl`, `serviceUrlTls`, `brokerServiceUrl`, or `brokerServiceUrlTls` are not null. Then proceeds to throw `IllegalArgumentException` if either of `serviceUrl`, `serviceUrlTls` or `brokerServiceUrl`, `brokerServiceUrlTls` has not been set. --- .../common/policies/data/ClusterDataImpl.java | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java index 6a7110e65072d..7678016ef2701 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java @@ -412,7 +412,20 @@ public ClusterDataImpl build() { * * @throws IllegalArgumentException exist illegal property. */ - public void checkPropertiesIfPresent() throws IllegalArgumentException { + public void checkPropertiesIfPresent() throws IllegalArgumentException { + boolean hasServiceUrl = getServiceUrl() != null; + boolean hasServiceUrlTls = getServiceUrlTls() != null; + boolean hasBrokerServiceUrl = getBrokerServiceUrl() != null; + boolean hasBrokerServiceUrlTls = getBrokerServiceUrlTls() != null; + + if (!hasServiceUrl && !hasServiceUrlTls) { + throw new IllegalArgumentException("Atleast one of ServiceUrl or ServiceUrlTls must be set."); + } + + if (!hasBrokerServiceUrl && !hasBrokerServiceUrlTls) { + throw new IllegalArgumentException("Atleast one of BrokerServiceUrl or BrokerServiceUrlTls must be set."); + } + URIPreconditions.checkURIIfPresent(getServiceUrl(), uri -> Objects.equals(uri.getScheme(), "http"), "Illegal service url, example: http://pulsar.example.com:8080"); @@ -430,20 +443,5 @@ public void checkPropertiesIfPresent() throws IllegalArgumentException { || Objects.equals(uri.getScheme(), "pulsar+ssl"), "Illegal proxy service url, example: pulsar+ssl://ats-proxy.example.com:4443 " + "or pulsar://ats-proxy.example.com:4080"); - - warnIfUrlIsNotPresent(); - } - - private void warnIfUrlIsNotPresent() { - if (StringUtils.isEmpty(getServiceUrl()) && StringUtils.isEmpty(getServiceUrlTls())) { - log.warn("Service url not found, " - + "please provide either service url, example: http://pulsar.example.com:8080 " - + "or service tls url, example: https://pulsar.example.com:8443"); - } - if (StringUtils.isEmpty(getBrokerServiceUrl()) && StringUtils.isEmpty(getBrokerServiceUrlTls())) { - log.warn("Broker service url not found, " - + "please provide either broker service url, example: pulsar://pulsar.example.com:6650 " - + "or broker service tls url, example: pulsar+ssl://pulsar.example.com:6651."); - } } } From b9c8f7207caf480022189d8943e6005c9f30b8b7 Mon Sep 17 00:00:00 2001 From: Harshitha Sudhakar <111514477+harshithasudhakar@users.noreply.github.com> Date: Mon, 24 Jul 2023 13:37:35 +0530 Subject: [PATCH 02/15] Update ClusterDataImpl.java --- .../apache/pulsar/common/policies/data/ClusterDataImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java index 7678016ef2701..4f0c13f888b85 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java @@ -419,11 +419,11 @@ public void checkPropertiesIfPresent() throws IllegalArgumentException { boolean hasBrokerServiceUrlTls = getBrokerServiceUrlTls() != null; if (!hasServiceUrl && !hasServiceUrlTls) { - throw new IllegalArgumentException("Atleast one of ServiceUrl or ServiceUrlTls must be set."); + throw new IllegalArgumentException("At least one of ServiceUrl or ServiceUrlTls must be set."); } if (!hasBrokerServiceUrl && !hasBrokerServiceUrlTls) { - throw new IllegalArgumentException("Atleast one of BrokerServiceUrl or BrokerServiceUrlTls must be set."); + throw new IllegalArgumentException("At least one of BrokerServiceUrl or BrokerServiceUrlTls must be set."); } URIPreconditions.checkURIIfPresent(getServiceUrl(), From a105a478d5e7f598379a36c44093e28448a53da0 Mon Sep 17 00:00:00 2001 From: Harshitha Sudhakar <111514477+harshithasudhakar@users.noreply.github.com> Date: Mon, 24 Jul 2023 13:48:06 +0530 Subject: [PATCH 03/15] Update ClusterDataImpl.java Used StringUtils.isEmpty(getServiceUrl()) && StringUtils.isEmpty(getServiceUrlTls()) to reduce the codes --- .../pulsar/common/policies/data/ClusterDataImpl.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java index 4f0c13f888b85..632359a236adf 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java @@ -413,16 +413,12 @@ public ClusterDataImpl build() { * @throws IllegalArgumentException exist illegal property. */ public void checkPropertiesIfPresent() throws IllegalArgumentException { - boolean hasServiceUrl = getServiceUrl() != null; - boolean hasServiceUrlTls = getServiceUrlTls() != null; - boolean hasBrokerServiceUrl = getBrokerServiceUrl() != null; - boolean hasBrokerServiceUrlTls = getBrokerServiceUrlTls() != null; - if (!hasServiceUrl && !hasServiceUrlTls) { + if (StringUtils.isEmpty(getServiceUrl()) && StringUtils.isEmpty(getServiceUrlTls())) { throw new IllegalArgumentException("At least one of ServiceUrl or ServiceUrlTls must be set."); } - if (!hasBrokerServiceUrl && !hasBrokerServiceUrlTls) { + if (StringUtils.isEmpty(getBrokerServiceUrl()) && StringUtils.isEmpty(getBrokerServiceUrlTls())) { throw new IllegalArgumentException("At least one of BrokerServiceUrl or BrokerServiceUrlTls must be set."); } From e6dfcf899da13054fab1fa0214c068eb8a605f0a Mon Sep 17 00:00:00 2001 From: Harshitha Sudhakar <111514477+harshithasudhakar@users.noreply.github.com> Date: Thu, 27 Jul 2023 21:28:38 +0530 Subject: [PATCH 04/15] Update ClusterDataImpl.java --- .../apache/pulsar/common/policies/data/ClusterDataImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java index 632359a236adf..bfc44abf94392 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java @@ -415,11 +415,11 @@ public ClusterDataImpl build() { public void checkPropertiesIfPresent() throws IllegalArgumentException { if (StringUtils.isEmpty(getServiceUrl()) && StringUtils.isEmpty(getServiceUrlTls())) { - throw new IllegalArgumentException("At least one of ServiceUrl or ServiceUrlTls must be set."); + throw new IllegalArgumentException("At least one of ServiceUrl or ServiceUrlTls must be set."); } if (StringUtils.isEmpty(getBrokerServiceUrl()) && StringUtils.isEmpty(getBrokerServiceUrlTls())) { - throw new IllegalArgumentException("At least one of BrokerServiceUrl or BrokerServiceUrlTls must be set."); + throw new IllegalArgumentException("At least one of BrokerServiceUrl or BrokerServiceUrlTls must be set."); } URIPreconditions.checkURIIfPresent(getServiceUrl(), From 154d259662797030337410eea829373089f2969e Mon Sep 17 00:00:00 2001 From: Harshitha Sudhakar <111514477+harshithasudhakar@users.noreply.github.com> Date: Thu, 27 Jul 2023 23:11:14 +0530 Subject: [PATCH 05/15] Update ClusterDataImplTest.java --- .../policies/data/ClusterDataImplTest.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java index ca4cba2cf9749..515953e052fa1 100644 --- a/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java +++ b/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java @@ -62,4 +62,34 @@ public void verifyClone() { assertEquals(clone, originalData, "Clones should have object equality."); assertNotSame(clone, originalData, "Clones should not be the same reference."); } + + @Test + public void testBothServiceUrlsisEmpty(){ + ClusterDataImpl clusterData = new ClusterDataImpl(); + clusterData.setServiceUrl(""); + clusterData.setServiceUrlTls(""); + clusterData.setBrokerServiceUrl("pulsar://pulsar.example.com:6650"); + clusterData.setBrokerServiceUrlTls("pulsar+ssl://pulsar.example.com:6651"); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, clusterData::checkPropertiesIfPresent + ); + + assertEquals("At least one of ServiceUrl or ServiceUrlTls must be set.", exception.getMessage()); + } + + @Test + public void testBothBrokerServiceUrlsisEmpty(){ + ClusterDataImpl clusterData = new ClusterDataImpl(); + clusterData.setServiceUrl("http://pulsar.example.com:8080"); + clusterData.setServiceUrlTls("https://pulsar.example.com:8443"); + clusterData.setBrokerServiceUrl(""); + clusterData.setBrokerServiceUrlTls(""); + + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, clusterData::checkPropertiesIfPresent + ); + + assertEquals("At least one of BrokerServiceUrl or BrokerServiceUrlTls must be set.", exception.getMessage()); + } } From 41d5a4eb9cb3f5fe1b0c22f09a88d125cbe35742 Mon Sep 17 00:00:00 2001 From: Harshitha Sudhakar <111514477+harshithasudhakar@users.noreply.github.com> Date: Thu, 27 Jul 2023 23:16:07 +0530 Subject: [PATCH 06/15] Update ClusterDataImplTest.java --- .../pulsar/common/policies/data/ClusterDataImplTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java index 515953e052fa1..9023d38bdc5b5 100644 --- a/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java +++ b/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java @@ -20,6 +20,10 @@ import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotSame; +import static org.junit.jupiter.api.Assertions.*; + +import org.apache.commons.lang3.StringUtils; +import org.junit.jupiter.api.Test; import org.apache.pulsar.client.api.ProxyProtocol; import org.testng.annotations.Test; From 518b0d5cc64aed7815aca18d7d841872e6d20f2a Mon Sep 17 00:00:00 2001 From: Harshitha Sudhakar <111514477+harshithasudhakar@users.noreply.github.com> Date: Thu, 27 Jul 2023 23:31:32 +0530 Subject: [PATCH 07/15] Update ClusterDataImpl.java --- .../org/apache/pulsar/common/policies/data/ClusterDataImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java index bfc44abf94392..fbfcfdec11fa0 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java @@ -412,7 +412,7 @@ public ClusterDataImpl build() { * * @throws IllegalArgumentException exist illegal property. */ - public void checkPropertiesIfPresent() throws IllegalArgumentException { + public void checkPropertiesIfPresent() throws IllegalArgumentException { if (StringUtils.isEmpty(getServiceUrl()) && StringUtils.isEmpty(getServiceUrlTls())) { throw new IllegalArgumentException("At least one of ServiceUrl or ServiceUrlTls must be set."); From 749208b6a3c94a8db144c9eaff7b128f767f7cf5 Mon Sep 17 00:00:00 2001 From: Harshitha Sudhakar <111514477+harshithasudhakar@users.noreply.github.com> Date: Sat, 12 Aug 2023 00:19:59 +0530 Subject: [PATCH 08/15] Update AdminApi2Test.java Changes: 1. testUpdateClusterServiceUrl(): Tests API for updating cluster's Service URL, and checks if the retrieved serviceURL matches with it. 2. testUpdateClusterBrokerServiceUrl(): Tests API for updating cluster's Service Broker URL, and checks if the retrieved serviceBrokerURL matches with it. --- .../pulsar/broker/admin/AdminApi2Test.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java index d6176966d85d9..6644a922593e9 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java @@ -2095,6 +2095,32 @@ public void testUpdateClusterWithProxyUrl() throws Exception { Assert.assertEquals(admin.clusters().getCluster(clusterName), cluster); } + @Test + public void testUpdateClusterServiceUrl() throws Exception { + String clusterName = "test_cluster"; + clusterDataImpl initialCluster = new clusterDataImpl(); + initialCluster.setServiceUrl("http://example.com"); + admin.clusters().createCluster(clusterName, initialCluster); + clusterDataImpl updatedCluster = new clusterDataImpl(); + updatedCluster.setServiceUrl("http://new-example.com"); + admin.clusters().updateCluster(clusterName, updatedCluster); + clusterDataImpl retrievedCluster = (ClusterDataImpl) admin.clusters().getCluster(ClusterName); + Assert.assertEquals(retrievedCluster.getServiceUrl(), updatedCluster.getServiceUrl()); + } + + @Test + public void testUpdateClusterBrokerServiceUrl() throws Exception { + String clusterName = "test_cluster"; + clusterDataImpl initialCluster = new clusterDataImpl(); + initialCluster.setBrokerServiceUrl("pulsar://broker.example.com:6650"); + admin.clusters().createCluster(clusterName, initialCluster); + clusterDataImpl updatedCluster = new clusterDataImpl(); + updatedCluster.setBrokerServiceUrl("pulsar://new-broker.example.com:6650"); + admin.clusters().updateCluster(clusterName, updatedCluster); + clusterDataImpl retrievedCluster = (ClusterDataImpl) admin.clusters().getCluster(ClusterName); + Assert.assertEquals(retrievedCluster.getBrokerServiceUrl(), updatedCluster.getBrokerServiceUrl()); + } + @Test public void testMaxNamespacesPerTenant() throws Exception { restartClusterAfterTest(); From b97a9d577286b91502d80f34002a4f0c31f96012 Mon Sep 17 00:00:00 2001 From: Harshitha Sudhakar <111514477+harshithasudhakar@users.noreply.github.com> Date: Sat, 12 Aug 2023 00:25:57 +0530 Subject: [PATCH 09/15] Update AdminApi2Test.java --- .../test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java | 1 + 1 file changed, 1 insertion(+) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java index 6644a922593e9..981c21557c643 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java @@ -110,6 +110,7 @@ import org.apache.pulsar.common.policies.data.BrokerNamespaceIsolationDataImpl; import org.apache.pulsar.common.policies.data.BundlesData; import org.apache.pulsar.common.policies.data.ClusterData; +import org.apache.pulsar.common.policies.data.ClusterDataImpl; import org.apache.pulsar.common.policies.data.ConsumerStats; import org.apache.pulsar.common.policies.data.EntryFilters; import org.apache.pulsar.common.policies.data.FailureDomain; From b418367142dac7c4cd514a2b57bf86212e55f889 Mon Sep 17 00:00:00 2001 From: Harshitha Sudhakar <111514477+harshithasudhakar@users.noreply.github.com> Date: Thu, 21 Sep 2023 14:44:29 +0530 Subject: [PATCH 10/15] Update ClusterDataImpl.java --- .../common/policies/data/ClusterDataImpl.java | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java index fbfcfdec11fa0..e36bbf59338f4 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java @@ -412,32 +412,32 @@ public ClusterDataImpl build() { * * @throws IllegalArgumentException exist illegal property. */ - public void checkPropertiesIfPresent() throws IllegalArgumentException { - - if (StringUtils.isEmpty(getServiceUrl()) && StringUtils.isEmpty(getServiceUrlTls())) { - throw new IllegalArgumentException("At least one of ServiceUrl or ServiceUrlTls must be set."); - } - - if (StringUtils.isEmpty(getBrokerServiceUrl()) && StringUtils.isEmpty(getBrokerServiceUrlTls())) { - throw new IllegalArgumentException("At least one of BrokerServiceUrl or BrokerServiceUrlTls must be set."); - } - - URIPreconditions.checkURIIfPresent(getServiceUrl(), - uri -> Objects.equals(uri.getScheme(), "http"), - "Illegal service url, example: http://pulsar.example.com:8080"); - URIPreconditions.checkURIIfPresent(getServiceUrlTls(), - uri -> Objects.equals(uri.getScheme(), "https"), - "Illegal service tls url, example: https://pulsar.example.com:8443"); - URIPreconditions.checkURIIfPresent(getBrokerServiceUrl(), - uri -> Objects.equals(uri.getScheme(), "pulsar"), - "Illegal broker service url, example: pulsar://pulsar.example.com:6650"); - URIPreconditions.checkURIIfPresent(getBrokerServiceUrlTls(), - uri -> Objects.equals(uri.getScheme(), "pulsar+ssl"), - "Illegal broker service tls url, example: pulsar+ssl://pulsar.example.com:6651"); - URIPreconditions.checkURIIfPresent(getProxyServiceUrl(), - uri -> Objects.equals(uri.getScheme(), "pulsar") - || Objects.equals(uri.getScheme(), "pulsar+ssl"), - "Illegal proxy service url, example: pulsar+ssl://ats-proxy.example.com:4443 " - + "or pulsar://ats-proxy.example.com:4080"); - } + public void checkPropertiesIfPresent() throws IllegalArgumentException { + + if (StringUtils.isEmpty(getServiceUrl()) && StringUtils.isEmpty(getServiceUrlTls())) { + throw new IllegalArgumentException("At least one of ServiceUrl or ServiceUrlTls must be set."); + } + + if (StringUtils.isEmpty(getBrokerServiceUrl()) && StringUtils.isEmpty(getBrokerServiceUrlTls())) { + throw new IllegalArgumentException("At least one of BrokerServiceUrl or BrokerServiceUrlTls must be set."); + } + + URIPreconditions.checkURIIfPresent(getServiceUrl(), + uri -> Objects.equals(uri.getScheme(), "http"), + "Illegal service url, example: http://pulsar.example.com:8080"); + URIPreconditions.checkURIIfPresent(getServiceUrlTls(), + uri -> Objects.equals(uri.getScheme(), "https"), + "Illegal service tls url, example: https://pulsar.example.com:8443"); + URIPreconditions.checkURIIfPresent(getBrokerServiceUrl(), + uri -> Objects.equals(uri.getScheme(), "pulsar"), + "Illegal broker service url, example: pulsar://pulsar.example.com:6650"); + URIPreconditions.checkURIIfPresent(getBrokerServiceUrlTls(), + uri -> Objects.equals(uri.getScheme(), "pulsar+ssl"), + "Illegal broker service tls url, example: pulsar+ssl://pulsar.example.com:6651"); + URIPreconditions.checkURIIfPresent(getProxyServiceUrl(), + uri -> Objects.equals(uri.getScheme(), "pulsar") + || Objects.equals(uri.getScheme(), "pulsar+ssl"), + "Illegal proxy service url, example: pulsar+ssl://ats-proxy.example.com:4443 " + + "or pulsar://ats-proxy.example.com:4080"); + } } From 2a5e58d64453eb7033dd0bf8da243188a9c27ff9 Mon Sep 17 00:00:00 2001 From: Harshitha Sudhakar <111514477+harshithasudhakar@users.noreply.github.com> Date: Thu, 21 Sep 2023 14:47:20 +0530 Subject: [PATCH 11/15] Update ClusterDataImplTest.java --- .../pulsar/common/policies/data/ClusterDataImplTest.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java index 9023d38bdc5b5..515953e052fa1 100644 --- a/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java +++ b/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java @@ -20,10 +20,6 @@ import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotSame; -import static org.junit.jupiter.api.Assertions.*; - -import org.apache.commons.lang3.StringUtils; -import org.junit.jupiter.api.Test; import org.apache.pulsar.client.api.ProxyProtocol; import org.testng.annotations.Test; From db2ffdc99c0a22a66a6294b55ecd5ee359d4b3e8 Mon Sep 17 00:00:00 2001 From: Harshitha Sudhakar <111514477+harshithasudhakar@users.noreply.github.com> Date: Thu, 21 Sep 2023 15:16:36 +0530 Subject: [PATCH 12/15] Update ClusterDataImpl.java --- .../common/policies/data/ClusterDataImpl.java | 48 +++++++++---------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java index e36bbf59338f4..21892875e7336 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java @@ -413,31 +413,29 @@ public ClusterDataImpl build() { * @throws IllegalArgumentException exist illegal property. */ public void checkPropertiesIfPresent() throws IllegalArgumentException { + if (StringUtils.isEmpty(getServiceUrl()) && StringUtils.isEmpty(getServiceUrlTls())) { + throw new IllegalArgumentException("At least one of ServiceUrl or ServiceUrlTls must be set."); + } + if (StringUtils.isEmpty(getBrokerServiceUrl()) && StringUtils.isEmpty(getBrokerServiceUrlTls())) { + throw new IllegalArgumentException("At least one of BrokerServiceUrl or BrokerServiceUrlTls must be set."); + } - if (StringUtils.isEmpty(getServiceUrl()) && StringUtils.isEmpty(getServiceUrlTls())) { - throw new IllegalArgumentException("At least one of ServiceUrl or ServiceUrlTls must be set."); - } - - if (StringUtils.isEmpty(getBrokerServiceUrl()) && StringUtils.isEmpty(getBrokerServiceUrlTls())) { - throw new IllegalArgumentException("At least one of BrokerServiceUrl or BrokerServiceUrlTls must be set."); - } - - URIPreconditions.checkURIIfPresent(getServiceUrl(), - uri -> Objects.equals(uri.getScheme(), "http"), - "Illegal service url, example: http://pulsar.example.com:8080"); - URIPreconditions.checkURIIfPresent(getServiceUrlTls(), - uri -> Objects.equals(uri.getScheme(), "https"), - "Illegal service tls url, example: https://pulsar.example.com:8443"); - URIPreconditions.checkURIIfPresent(getBrokerServiceUrl(), - uri -> Objects.equals(uri.getScheme(), "pulsar"), - "Illegal broker service url, example: pulsar://pulsar.example.com:6650"); - URIPreconditions.checkURIIfPresent(getBrokerServiceUrlTls(), - uri -> Objects.equals(uri.getScheme(), "pulsar+ssl"), - "Illegal broker service tls url, example: pulsar+ssl://pulsar.example.com:6651"); - URIPreconditions.checkURIIfPresent(getProxyServiceUrl(), - uri -> Objects.equals(uri.getScheme(), "pulsar") - || Objects.equals(uri.getScheme(), "pulsar+ssl"), - "Illegal proxy service url, example: pulsar+ssl://ats-proxy.example.com:4443 " - + "or pulsar://ats-proxy.example.com:4080"); + URIPreconditions.checkURIIfPresent(getServiceUrl(), + uri -> Objects.equals(uri.getScheme(), "http"), + "Illegal service url, example: http://pulsar.example.com:8080"); + URIPreconditions.checkURIIfPresent(getServiceUrlTls(), + uri -> Objects.equals(uri.getScheme(), "https"), + "Illegal service tls url, example: https://pulsar.example.com:8443"); + URIPreconditions.checkURIIfPresent(getBrokerServiceUrl(), + uri -> Objects.equals(uri.getScheme(), "pulsar"), + "Illegal broker service url, example: pulsar://pulsar.example.com:6650"); + URIPreconditions.checkURIIfPresent(getBrokerServiceUrlTls(), + uri -> Objects.equals(uri.getScheme(), "pulsar+ssl"), + "Illegal broker service tls url, example: pulsar+ssl://pulsar.example.com:6651"); + URIPreconditions.checkURIIfPresent(getProxyServiceUrl(), + uri -> Objects.equals(uri.getScheme(), "pulsar") + || Objects.equals(uri.getScheme(), "pulsar+ssl"), + "Illegal proxy service url, example: pulsar+ssl://ats-proxy.example.com:4443 " + + "or pulsar://ats-proxy.example.com:4080"); } } From 4bd190d1fe03c6f188ccdf00ad7b8609d2086dbc Mon Sep 17 00:00:00 2001 From: Jiwe Guo Date: Wed, 11 Oct 2023 10:59:26 +0800 Subject: [PATCH 13/15] fix checkstyle issue --- .../pulsar/common/policies/data/ClusterDataImpl.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java index 21892875e7336..4977af8ceb184 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java @@ -415,11 +415,12 @@ public ClusterDataImpl build() { public void checkPropertiesIfPresent() throws IllegalArgumentException { if (StringUtils.isEmpty(getServiceUrl()) && StringUtils.isEmpty(getServiceUrlTls())) { throw new IllegalArgumentException("At least one of ServiceUrl or ServiceUrlTls must be set."); - } + } if (StringUtils.isEmpty(getBrokerServiceUrl()) && StringUtils.isEmpty(getBrokerServiceUrlTls())) { - throw new IllegalArgumentException("At least one of BrokerServiceUrl or BrokerServiceUrlTls must be set."); + throw new IllegalArgumentException("At least one of BrokerServiceUrl or BrokerServiceUrlTls" + + " must be set."); } - + URIPreconditions.checkURIIfPresent(getServiceUrl(), uri -> Objects.equals(uri.getScheme(), "http"), "Illegal service url, example: http://pulsar.example.com:8080"); From 190e8323f5ab7e683fdf36d346671add90cdc140 Mon Sep 17 00:00:00 2001 From: Jiwe Guo Date: Wed, 11 Oct 2023 11:17:16 +0800 Subject: [PATCH 14/15] fix test issue --- .../common/policies/data/ClusterDataImplTest.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java index 515953e052fa1..5285e73c96ac7 100644 --- a/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java +++ b/pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java @@ -20,6 +20,7 @@ import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotSame; +import static org.testng.Assert.assertThrows; import org.apache.pulsar.client.api.ProxyProtocol; import org.testng.annotations.Test; @@ -64,32 +65,28 @@ public void verifyClone() { } @Test - public void testBothServiceUrlsisEmpty(){ + public void testBothServiceUrlsIsEmpty(){ ClusterDataImpl clusterData = new ClusterDataImpl(); clusterData.setServiceUrl(""); clusterData.setServiceUrlTls(""); clusterData.setBrokerServiceUrl("pulsar://pulsar.example.com:6650"); clusterData.setBrokerServiceUrlTls("pulsar+ssl://pulsar.example.com:6651"); - IllegalArgumentException exception = assertThrows( + assertThrows("At least one of ServiceUrl or ServiceUrlTls must be set.", IllegalArgumentException.class, clusterData::checkPropertiesIfPresent ); - - assertEquals("At least one of ServiceUrl or ServiceUrlTls must be set.", exception.getMessage()); } @Test - public void testBothBrokerServiceUrlsisEmpty(){ + public void testBothBrokerServiceUrlsIsEmpty(){ ClusterDataImpl clusterData = new ClusterDataImpl(); clusterData.setServiceUrl("http://pulsar.example.com:8080"); clusterData.setServiceUrlTls("https://pulsar.example.com:8443"); clusterData.setBrokerServiceUrl(""); clusterData.setBrokerServiceUrlTls(""); - IllegalArgumentException exception = assertThrows( + assertThrows("At least one of BrokerServiceUrl or BrokerServiceUrlTls must be set.", IllegalArgumentException.class, clusterData::checkPropertiesIfPresent ); - - assertEquals("At least one of BrokerServiceUrl or BrokerServiceUrlTls must be set.", exception.getMessage()); } } From 5193fc42de08d585a42094e54b5257ffaed84e91 Mon Sep 17 00:00:00 2001 From: Jiwe Guo Date: Wed, 11 Oct 2023 11:33:54 +0800 Subject: [PATCH 15/15] fix test --- .../apache/pulsar/broker/admin/AdminApi2Test.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java index 981c21557c643..1f87465905da1 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java @@ -2099,26 +2099,26 @@ public void testUpdateClusterWithProxyUrl() throws Exception { @Test public void testUpdateClusterServiceUrl() throws Exception { String clusterName = "test_cluster"; - clusterDataImpl initialCluster = new clusterDataImpl(); + ClusterDataImpl initialCluster = new ClusterDataImpl(); initialCluster.setServiceUrl("http://example.com"); admin.clusters().createCluster(clusterName, initialCluster); - clusterDataImpl updatedCluster = new clusterDataImpl(); + ClusterDataImpl updatedCluster = new ClusterDataImpl(); updatedCluster.setServiceUrl("http://new-example.com"); admin.clusters().updateCluster(clusterName, updatedCluster); - clusterDataImpl retrievedCluster = (ClusterDataImpl) admin.clusters().getCluster(ClusterName); + ClusterDataImpl retrievedCluster = (ClusterDataImpl) admin.clusters().getCluster(clusterName); Assert.assertEquals(retrievedCluster.getServiceUrl(), updatedCluster.getServiceUrl()); } @Test public void testUpdateClusterBrokerServiceUrl() throws Exception { String clusterName = "test_cluster"; - clusterDataImpl initialCluster = new clusterDataImpl(); + ClusterDataImpl initialCluster = new ClusterDataImpl(); initialCluster.setBrokerServiceUrl("pulsar://broker.example.com:6650"); admin.clusters().createCluster(clusterName, initialCluster); - clusterDataImpl updatedCluster = new clusterDataImpl(); + ClusterDataImpl updatedCluster = new ClusterDataImpl(); updatedCluster.setBrokerServiceUrl("pulsar://new-broker.example.com:6650"); admin.clusters().updateCluster(clusterName, updatedCluster); - clusterDataImpl retrievedCluster = (ClusterDataImpl) admin.clusters().getCluster(ClusterName); + ClusterDataImpl retrievedCluster = (ClusterDataImpl) admin.clusters().getCluster(clusterName); Assert.assertEquals(retrievedCluster.getBrokerServiceUrl(), updatedCluster.getBrokerServiceUrl()); }