Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed CommonUtilsException assets and references. #639

Merged
merged 7 commits into from
Apr 13, 2024
+51 −133
Merged

Removed CommonUtilsException assets and references. #639

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
04ddb3d
Added validation for the clusters field. Refactored ClusterMetricsInp…
AWSHurneyt Apr 12, 2024
4636569
Moved some regex from alerting plugin to common utils.
AWSHurneyt Apr 12, 2024
c913185
Moved cluster-based regex to separate file.
AWSHurneyt Apr 12, 2024
15ba46d
Fixed ktlint error.
AWSHurneyt Apr 12, 2024
89aef1c
Fixed regex. Moved cluster-related regexes.
AWSHurneyt Apr 12, 2024
3afcf7f
Merge branch 'opensearch-project:main' into 3.0-cross-cluster-dev
AWSHurneyt Apr 13, 2024
f460560
Removed CommonUtilsException. Team decided IllegalArgumentExceptions …
AWSHurneyt Apr 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 33 additions & 43 deletions src/main/kotlin/org/opensearch/commons/alerting/model/ClusterMetricsInput.kt
View file
Open in desktop
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
Open in desktop

This file was deleted.

37 changes: 18 additions & 19 deletions src/test/kotlin/org/opensearch/commons/alerting/model/ClusterMetricsInputTests.kt
View file
Open in desktop
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
Loading