Removed CommonUtilsException assets and references. (opensearch-proje…

…ct#639) * Added validation for the clusters field. Refactored ClusterMetricsInput validiation to throw 4xx-level CommonUtilsExceptions instead of 5xx-level IllegalArgumentException. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Moved some regex from alerting plugin to common utils. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Moved cluster-based regex to separate file. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Fixed ktlint error. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Fixed regex. Moved cluster-related regexes. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> * Removed CommonUtilsException. Team decided IllegalArgumentExceptions should be caught in the plugins themselves. Signed-off-by: AWSHurneyt <hurneyt@amazon.com> --------- Signed-off-by: AWSHurneyt <hurneyt@amazon.com>
AWSHurneyt · Apr 13, 2024 · c19a10a · c19a10a
1 parent abc69cd
commit c19a10a
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 133 deletions.
diff --git a/src/main/kotlin/org/opensearch/commons/alerting/model/ClusterMetricsInput.kt b/src/main/kotlin/org/opensearch/commons/alerting/model/ClusterMetricsInput.kt
@@ -3,7 +3,6 @@ package org.opensearch.commons.alerting.model
 import org.apache.commons.validator.routines.UrlValidator
 import org.apache.hc.core5.net.URIBuilder
 import org.opensearch.common.CheckedFunction
-import org.opensearch.commons.alerting.util.CommonUtilsException
 import org.opensearch.commons.utils.CLUSTER_NAME_REGEX
 import org.opensearch.core.ParseField
 import org.opensearch.core.common.io.stream.StreamInput
@@ -33,46 +32,41 @@ data class ClusterMetricsInput(
 
     // Verify parameters are valid during creation
     init {
-        // Wrap any validation exceptions in CommonUtilsException.
-        try {
-            require(validateFields()) {
-                "The uri.api_type field, uri.path field, or uri.uri field must be defined."
-            }
+        require(validateFields()) {
+            "The uri.api_type field, uri.path field, or uri.uri field must be defined."
+        }
 
-            // Create an UrlValidator that only accepts "http" and "https" as valid scheme and allows local URLs.
-            val urlValidator = UrlValidator(arrayOf("http", "https"), UrlValidator.ALLOW_LOCAL_URLS)
+        // Create an UrlValidator that only accepts "http" and "https" as valid scheme and allows local URLs.
+        val urlValidator = UrlValidator(arrayOf("http", "https"), UrlValidator.ALLOW_LOCAL_URLS)
 
-            // Build url field by field if not provided as whole.
-            constructedUri = toConstructedUri()
+        // Build url field by field if not provided as whole.
+        constructedUri = toConstructedUri()
 
-            require(urlValidator.isValid(constructedUri.toString())) {
-                "Invalid URI constructed from the path and path_params inputs, or the url input."
-            }
+        require(urlValidator.isValid(constructedUri.toString())) {
+            "Invalid URI constructed from the path and path_params inputs, or the url input."
+        }
 
-            if (url.isNotEmpty() && validateFieldsNotEmpty()) {
-                require(constructedUri == constructUrlFromInputs()) {
-                    "The provided URL and URI fields form different URLs."
-                }
+        if (url.isNotEmpty() && validateFieldsNotEmpty()) {
+            require(constructedUri == constructUrlFromInputs()) {
+                "The provided URL and URI fields form different URLs."
             }
+        }
 
-            require(constructedUri.host.lowercase() == SUPPORTED_HOST) {
-                "Only host '$SUPPORTED_HOST' is supported."
-            }
-            require(constructedUri.port == SUPPORTED_PORT) {
-                "Only port '$SUPPORTED_PORT' is supported."
-            }
+        require(constructedUri.host.lowercase() == SUPPORTED_HOST) {
+            "Only host '$SUPPORTED_HOST' is supported."
+        }
+        require(constructedUri.port == SUPPORTED_PORT) {
+            "Only port '$SUPPORTED_PORT' is supported."
+        }
 
-            if (clusters.isNotEmpty()) {
-                require(clusters.all { CLUSTER_NAME_REGEX.matches(it) }) {
-                    "Cluster names are not valid."
-                }
+        if (clusters.isNotEmpty()) {
+            require(clusters.all { CLUSTER_NAME_REGEX.matches(it) }) {
+                "Cluster names are not valid."
             }
-
-            clusterMetricType = findApiType(constructedUri.path)
-            this.parseEmptyFields()
-        } catch (exception: Exception) {
-            throw CommonUtilsException.wrap(exception)
         }
+
+        clusterMetricType = findApiType(constructedUri.path)
+        this.parseEmptyFields()
     }
 
     @Throws(IOException::class)
@@ -171,7 +165,7 @@ data class ClusterMetricsInput(
     /**
      * Isolates just the path parameters from the [ClusterMetricsInput] URI.
      * @return The path parameters portion of the [ClusterMetricsInput] URI.
-     * @throws CommonUtilsException if the [ClusterMetricType] requires path parameters, but none are supplied;
+     * @throws [IllegalArgumentException] if the [ClusterMetricType] requires path parameters, but none are supplied;
      * or when path parameters are provided for an [ClusterMetricType] that does not use path parameters.
      */
     fun parsePathParams(): String {
@@ -191,22 +185,18 @@ data class ClusterMetricsInput(
             pathParams = pathParams.trim('/')
             ILLEGAL_PATH_PARAMETER_CHARACTERS.forEach { character ->
                 if (pathParams.contains(character)) {
-                    throw CommonUtilsException.wrap(
-                        IllegalArgumentException(
-                            "The provided path parameters contain invalid characters or spaces. Please omit: " + ILLEGAL_PATH_PARAMETER_CHARACTERS.joinToString(
-                                " "
-                            )
-                        )
+                    throw IllegalArgumentException(
+                        "The provided path parameters contain invalid characters or spaces. Please omit: " + ILLEGAL_PATH_PARAMETER_CHARACTERS.joinToString(" ")
                     )
                 }
             }
         }
 
         if (apiType.requiresPathParams && pathParams.isEmpty()) {
-            throw CommonUtilsException.wrap(IllegalArgumentException("The API requires path parameters."))
+            throw IllegalArgumentException("The API requires path parameters.")
         }
         if (!apiType.supportsPathParams && pathParams.isNotEmpty()) {
-            throw CommonUtilsException.wrap(IllegalArgumentException("The API does not use path parameters."))
+            throw IllegalArgumentException("The API does not use path parameters.")
         }
 
         return pathParams
@@ -216,7 +206,7 @@ data class ClusterMetricsInput(
      * Examines the path of a [ClusterMetricsInput] to determine which API is being called.
      * @param uriPath The path to examine.
      * @return The [ClusterMetricType] associated with the [ClusterMetricsInput] monitor.
-     * @throws CommonUtilsException when the API to call cannot be determined from the URI.
+     * @throws [IllegalArgumentException] when the API to call cannot be determined from the URI.
      */
     private fun findApiType(uriPath: String): ClusterMetricType {
         var apiType = ClusterMetricType.BLANK
@@ -228,7 +218,7 @@ data class ClusterMetricsInput(
                 }
             }
         if (apiType.isBlank()) {
-            throw CommonUtilsException.wrap(IllegalArgumentException("The API could not be determined from the provided URI."))
+            throw IllegalArgumentException("The API could not be determined from the provided URI.")
         }
         return apiType
     }

diff --git a/src/main/kotlin/org/opensearch/commons/alerting/util/CommonUtilsException.kt b/src/main/kotlin/org/opensearch/commons/alerting/util/CommonUtilsException.kt
diff --git a/src/test/kotlin/org/opensearch/commons/alerting/model/ClusterMetricsInputTests.kt b/src/test/kotlin/org/opensearch/commons/alerting/model/ClusterMetricsInputTests.kt
@@ -1,7 +1,6 @@
 package org.opensearch.commons.alerting.model
 
 import org.junit.jupiter.api.Test
-import org.opensearch.commons.alerting.util.CommonUtilsException
 import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
 
@@ -79,7 +78,7 @@ class ClusterMetricsInputTests {
         path = "///"
 
         // WHEN + THEN
-        assertFailsWith<CommonUtilsException>("Invalid URL.") {
+        assertFailsWith<IllegalArgumentException>("Invalid URL.") {
             ClusterMetricsInput(path, pathParams, url)
         }
     }
@@ -90,7 +89,7 @@ class ClusterMetricsInputTests {
         url = "///"
 
         // WHEN + THEN
-        assertFailsWith<CommonUtilsException>("Invalid URL.") {
+        assertFailsWith<IllegalArgumentException>("Invalid URL.") {
             ClusterMetricsInput(path, pathParams, url)
         }
     }
@@ -135,7 +134,7 @@ class ClusterMetricsInputTests {
         url = "http://localhost:9200/_cluster/stats"
 
         // WHEN + THEN
-        assertFailsWith<CommonUtilsException>("The provided URL and URI fields form different URLs.") {
+        assertFailsWith<IllegalArgumentException>("The provided URL and URI fields form different URLs.") {
             ClusterMetricsInput(path, pathParams, url)
         }
     }
@@ -147,7 +146,7 @@ class ClusterMetricsInputTests {
         url = "http://localhost:9200/_cluster/stats/index1,index2,index3,index4,index5"
 
         // WHEN + THEN
-        assertFailsWith<CommonUtilsException>("The provided URL and URI fields form different URLs.") {
+        assertFailsWith<IllegalArgumentException>("The provided URL and URI fields form different URLs.") {
             ClusterMetricsInput(path, pathParams, url)
         }
     }
@@ -160,7 +159,7 @@ class ClusterMetricsInputTests {
         url = ""
 
         // WHEN + THEN
-        assertFailsWith<CommonUtilsException>("The uri.api_type field, uri.path field, or uri.uri field must be defined.") {
+        assertFailsWith<IllegalArgumentException>("The uri.api_type field, uri.path field, or uri.uri field must be defined.") {
             ClusterMetricsInput(path, pathParams, url)
         }
     }
@@ -173,7 +172,7 @@ class ClusterMetricsInputTests {
         url = ""
 
         // WHEN + THEN
-        assertFailsWith<CommonUtilsException>("The uri.api_type field, uri.path field, or uri.uri field must be defined.") {
+        assertFailsWith<IllegalArgumentException>("The uri.api_type field, uri.path field, or uri.uri field must be defined.") {
             ClusterMetricsInput(path, pathParams, url)
         }
     }
@@ -185,7 +184,7 @@ class ClusterMetricsInputTests {
         url = "invalidScheme://localhost:9200/_cluster/health"
 
         // WHEN + THEN
-        assertFailsWith<CommonUtilsException>("Invalid URL.") {
+        assertFailsWith<IllegalArgumentException>("Invalid URL.") {
             ClusterMetricsInput(path, pathParams, url)
         }
     }
@@ -197,7 +196,7 @@ class ClusterMetricsInputTests {
         url = "http://127.0.0.1:9200/_cluster/health"
 
         // WHEN + THEN
-        assertFailsWith<CommonUtilsException>("Only host '${ClusterMetricsInput.SUPPORTED_HOST}' is supported.") {
+        assertFailsWith<IllegalArgumentException>("Only host '${ClusterMetricsInput.SUPPORTED_HOST}' is supported.") {
             ClusterMetricsInput(path, pathParams, url)
         }
     }
@@ -209,7 +208,7 @@ class ClusterMetricsInputTests {
         url = "http://localhost:${ClusterMetricsInput.SUPPORTED_PORT + 1}/_cluster/health"
 
         // WHEN + THEN
-        assertFailsWith<CommonUtilsException>("Only port '${ClusterMetricsInput.SUPPORTED_PORT}' is supported.") {
+        assertFailsWith<IllegalArgumentException>("Only port '${ClusterMetricsInput.SUPPORTED_PORT}' is supported.") {
             ClusterMetricsInput(path, pathParams, url)
         }
     }
@@ -269,7 +268,7 @@ class ClusterMetricsInputTests {
         path = "/_cat/snapshots"
 
         // WHEN + THEN
-        assertFailsWith<CommonUtilsException>("The API requires path parameters.") {
+        assertFailsWith<IllegalArgumentException>("The API requires path parameters.") {
             ClusterMetricsInput(path, pathParams, url)
         }
     }
@@ -282,7 +281,7 @@ class ClusterMetricsInputTests {
         val clusterMetricsInput = ClusterMetricsInput(path, pathParams, url)
 
         // WHEN + THEN
-        assertFailsWith<CommonUtilsException>("The API does not use path parameters.") {
+        assertFailsWith<IllegalArgumentException>("The API does not use path parameters.") {
             clusterMetricsInput.parsePathParams()
         }
     }
@@ -296,7 +295,7 @@ class ClusterMetricsInputTests {
             val clusterMetricsInput = ClusterMetricsInput(path, pathParams, url)
 
             // WHEN + THEN
-            assertFailsWith<CommonUtilsException>(
+            assertFailsWith<IllegalArgumentException>(
                 "The provided path parameters contain invalid characters or spaces. Please omit: " + ILLEGAL_PATH_PARAMETER_CHARACTERS.joinToString(" ")
             ) {
                 clusterMetricsInput.parsePathParams()
@@ -394,7 +393,7 @@ class ClusterMetricsInputTests {
         path = "/_cat/paws"
 
         // WHEN + THEN
-        assertFailsWith<CommonUtilsException>("The API could not be determined from the provided URI.") {
+        assertFailsWith<IllegalArgumentException>("The API could not be determined from the provided URI.") {
             ClusterMetricsInput(path, pathParams, url)
         }
     }
@@ -406,7 +405,7 @@ class ClusterMetricsInputTests {
         pathParams = "index1,index2,index3,index4,index5"
 
         // WHEN + THEN
-        assertFailsWith<CommonUtilsException>("The API could not be determined from the provided URI.") {
+        assertFailsWith<IllegalArgumentException>("The API could not be determined from the provided URI.") {
             ClusterMetricsInput(path, pathParams, url)
         }
     }
@@ -418,7 +417,7 @@ class ClusterMetricsInputTests {
         url = "http://localhost:9200/_cat/paws"
 
         // WHEN + THEN
-        assertFailsWith<CommonUtilsException>("The API could not be determined from the provided URI.") {
+        assertFailsWith<IllegalArgumentException>("The API could not be determined from the provided URI.") {
             ClusterMetricsInput(path, pathParams, url)
         }
     }
@@ -430,7 +429,7 @@ class ClusterMetricsInputTests {
         url = "http://localhost:9200/_cat/paws/index1,index2,index3,index4,index5"
 
         // WHEN + THEN
-        assertFailsWith<CommonUtilsException>("The API could not be determined from the provided URI.") {
+        assertFailsWith<IllegalArgumentException>("The API could not be determined from the provided URI.") {
             ClusterMetricsInput(path, pathParams, url)
         }
     }
@@ -527,7 +526,7 @@ class ClusterMetricsInputTests {
             val clusters = listOf(it)
 
             // WHEN + THEN
-            assertFailsWith<CommonUtilsException>("The API could not be determined from the provided URI.") {
+            assertFailsWith<IllegalArgumentException>("The API could not be determined from the provided URI.") {
                 ClusterMetricsInput(
                     path = path,
                     pathParams = pathParams,
@@ -547,7 +546,7 @@ class ClusterMetricsInputTests {
         val clusters = invalidClusters
 
         // WHEN + THEN
-        assertFailsWith<CommonUtilsException>("The API could not be determined from the provided URI.") {
+        assertFailsWith<IllegalArgumentException>("The API could not be determined from the provided URI.") {
             ClusterMetricsInput(
                 path = path,
                 pathParams = pathParams,