Skip to content

Commit

Permalink
Removed CommonUtilsException. Team decided IllegalArgumentExceptions …
Browse files Browse the repository at this point in the history 
…should be caught in the plugins themselves.

Signed-off-by: AWSHurneyt <hurneyt@amazon.com>
  • Loading branch information
@AWSHurneyt
AWSHurneyt committed Apr 13, 2024
1 parent 3afcf7f commit f460560
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 133 deletions.
76 changes: 33 additions & 43 deletions src/main/kotlin/org/opensearch/commons/alerting/model/ClusterMetricsInput.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
}
Expand Down
71 changes: 0 additions & 71 deletions src/main/kotlin/org/opensearch/commons/alerting/util/CommonUtilsException.kt
View file

This file was deleted.

37 changes: 18 additions & 19 deletions src/test/kotlin/org/opensearch/commons/alerting/model/ClusterMetricsInputTests.kt
View file
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -79,7 +78,7 @@ class ClusterMetricsInputTests {
path = "///"

// WHEN + THEN
assertFailsWith<CommonUtilsException>("Invalid URL.") {
assertFailsWith<IllegalArgumentException>("Invalid URL.") {
ClusterMetricsInput(path, pathParams, url)
}
}
Expand All @@ -90,7 +89,7 @@ class ClusterMetricsInputTests {
url = "///"

// WHEN + THEN
assertFailsWith<CommonUtilsException>("Invalid URL.") {
assertFailsWith<IllegalArgumentException>("Invalid URL.") {
ClusterMetricsInput(path, pathParams, url)
}
}
Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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)
}
}
Expand All @@ -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)
}
}
Expand All @@ -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)
}
}
Expand All @@ -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)
}
}
Expand All @@ -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)
}
}
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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()
}
}
Expand All @@ -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()
Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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)
}
}
Expand All @@ -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)
}
}
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down

0 comments on commit f460560

Please sign in to comment.