Skip to content

Commit

Permalink
Remove allowedConfigFeatureList from GetPluginFeaturesResponse (opens…
Browse files Browse the repository at this point in the history 
…earch-project#144)

Signed-off-by: Mohammad Qureshi <[email protected]>
  • Loading branch information
@qreshi
qreshi authored Mar 28, 2022
1 parent a8658e7 commit a6f4a0e
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 39 deletions.
1 change: 0 additions & 1 deletion src/main/kotlin/org/opensearch/commons/notifications/NotificationConstants.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ object NotificationConstants {
const val QUERY_TAG = "query"
const val COMPACT_TAG = "compact"
const val ALLOWED_CONFIG_TYPE_LIST_TAG = "allowed_config_type_list"
const val ALLOWED_CONFIG_FEATURE_LIST_TAG = "allowed_config_feature_list"
const val PLUGIN_FEATURES_TAG = "plugin_features"

const val DEFAULT_MAX_ITEMS = 1000
Expand Down
13 changes: 1 addition & 12 deletions src/main/kotlin/org/opensearch/commons/notifications/action/GetPluginFeaturesResponse.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import org.opensearch.common.xcontent.ToXContent
import org.opensearch.common.xcontent.XContentBuilder
import org.opensearch.common.xcontent.XContentParser
import org.opensearch.common.xcontent.XContentParserUtils
import org.opensearch.commons.notifications.NotificationConstants.ALLOWED_CONFIG_FEATURE_LIST_TAG
import org.opensearch.commons.notifications.NotificationConstants.ALLOWED_CONFIG_TYPE_LIST_TAG
import org.opensearch.commons.notifications.NotificationConstants.PLUGIN_FEATURES_TAG
import org.opensearch.commons.utils.STRING_READER
Expand All @@ -25,7 +24,6 @@ import java.io.IOException
*/
class GetPluginFeaturesResponse : BaseResponse {
val allowedConfigTypeList: List<String>
val allowedConfigFeatureList: List<String>
val pluginFeatures: Map<String, String>

companion object {
Expand All @@ -44,7 +42,6 @@ class GetPluginFeaturesResponse : BaseResponse {
@Throws(IOException::class)
fun parse(parser: XContentParser): GetPluginFeaturesResponse {
var allowedConfigTypeList: List<String>? = null
var allowedConfigFeatureList: List<String>? = null
var pluginFeatures: Map<String, String>? = null

XContentParserUtils.ensureExpectedToken(
Expand All @@ -57,7 +54,6 @@ class GetPluginFeaturesResponse : BaseResponse {
parser.nextToken()
when (fieldName) {
ALLOWED_CONFIG_TYPE_LIST_TAG -> allowedConfigTypeList = parser.stringList()
ALLOWED_CONFIG_FEATURE_LIST_TAG -> allowedConfigFeatureList = parser.stringList()
PLUGIN_FEATURES_TAG -> pluginFeatures = parser.mapStrings()
else -> {
parser.skipChildren()
Expand All @@ -66,9 +62,8 @@ class GetPluginFeaturesResponse : BaseResponse {
}
}
allowedConfigTypeList ?: throw IllegalArgumentException("$ALLOWED_CONFIG_TYPE_LIST_TAG field absent")
allowedConfigFeatureList ?: throw IllegalArgumentException("$ALLOWED_CONFIG_TYPE_LIST_TAG field absent")
pluginFeatures ?: throw IllegalArgumentException("$PLUGIN_FEATURES_TAG field absent")
return GetPluginFeaturesResponse(allowedConfigTypeList, allowedConfigFeatureList, pluginFeatures)
return GetPluginFeaturesResponse(allowedConfigTypeList, pluginFeatures)
}
}

Expand All @@ -78,24 +73,20 @@ class GetPluginFeaturesResponse : BaseResponse {
override fun toXContent(builder: XContentBuilder?, params: ToXContent.Params?): XContentBuilder {
return builder!!.startObject()
.field(ALLOWED_CONFIG_TYPE_LIST_TAG, allowedConfigTypeList)
.field(ALLOWED_CONFIG_FEATURE_LIST_TAG, allowedConfigFeatureList)
.field(PLUGIN_FEATURES_TAG, pluginFeatures)
.endObject()
}

/**
* constructor for creating the class
* @param allowedConfigTypeList the list of config types supported by plugin
* @param allowedConfigFeatureList the list of config features supported by plugin
* @param pluginFeatures the map of plugin features supported to its value
*/
constructor(
allowedConfigTypeList: List<String>,
allowedConfigFeatureList: List<String>,
pluginFeatures: Map<String, String>
) {
this.allowedConfigTypeList = allowedConfigTypeList
this.allowedConfigFeatureList = allowedConfigFeatureList
this.pluginFeatures = pluginFeatures
}

Expand All @@ -105,7 +96,6 @@ class GetPluginFeaturesResponse : BaseResponse {
@Throws(IOException::class)
constructor(input: StreamInput) : super(input) {
allowedConfigTypeList = input.readStringList()
allowedConfigFeatureList = input.readStringList()
pluginFeatures = input.readMap(STRING_READER, STRING_READER)
}

Expand All @@ -115,7 +105,6 @@ class GetPluginFeaturesResponse : BaseResponse {
@Throws(IOException::class)
override fun writeTo(output: StreamOutput) {
output.writeStringCollection(allowedConfigTypeList)
output.writeStringCollection(allowedConfigFeatureList)
output.writeMap(pluginFeatures, STRING_WRITER, STRING_WRITER)
}
}
1 change: 0 additions & 1 deletion src/test/kotlin/org/opensearch/commons/notifications/NotificationsPluginInterfaceTests.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ internal class NotificationsPluginInterfaceTests {
val request = mock(GetPluginFeaturesRequest::class.java)
val response = GetPluginFeaturesResponse(
listOf("config_type_1", "config_type_2", "config_type_3"),
listOf("config_feature_1", "config_feature_2", "config_feature_3"),
mapOf(
Pair("FeatureKey1", "FeatureValue1"),
Pair("FeatureKey2", "FeatureValue2"),
Expand Down
26 changes: 1 addition & 25 deletions ...test/kotlin/org/opensearch/commons/notifications/action/GetPluginFeaturesResponseTests.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,13 @@ internal class GetPluginFeaturesResponseTests {
actual: GetPluginFeaturesResponse
) {
assertEquals(expected.allowedConfigTypeList, actual.allowedConfigTypeList)
assertEquals(expected.allowedConfigFeatureList, actual.allowedConfigFeatureList)
assertEquals(expected.pluginFeatures, actual.pluginFeatures)
}

@Test
fun `Get Response serialize and deserialize transport object should be equal`() {
val response = GetPluginFeaturesResponse(
listOf("config_type_1", "config_type_2", "config_type_3"),
listOf("config_feature_1", "config_feature_2", "config_feature_3"),
mapOf(
Pair("FeatureKey1", "FeatureValue1"),
Pair("FeatureKey2", "FeatureValue2"),
Expand All @@ -41,7 +39,6 @@ internal class GetPluginFeaturesResponseTests {
fun `Get Response serialize and deserialize using json config object should be equal`() {
val response = GetPluginFeaturesResponse(
listOf("config_type_1", "config_type_2", "config_type_3"),
listOf("config_feature_1", "config_feature_2", "config_feature_3"),
mapOf(
Pair("FeatureKey1", "FeatureValue1"),
Pair("FeatureKey2", "FeatureValue2"),
Expand All @@ -57,7 +54,6 @@ internal class GetPluginFeaturesResponseTests {
fun `Get Response should safely ignore extra field in json object`() {
val response = GetPluginFeaturesResponse(
listOf("config_type_1", "config_type_2", "config_type_3"),
listOf("config_feature_1", "config_feature_2", "config_feature_3"),
mapOf(
Pair("FeatureKey1", "FeatureValue1"),
Pair("FeatureKey2", "FeatureValue2"),
Expand All @@ -67,7 +63,6 @@ internal class GetPluginFeaturesResponseTests {
val jsonString = """
{
"allowed_config_type_list":["config_type_1", "config_type_2", "config_type_3"],
"allowed_config_feature_list":["config_feature_1", "config_feature_2", "config_feature_3"],
"plugin_features":{
"FeatureKey1":"FeatureValue1",
"FeatureKey2":"FeatureValue2",
Expand All @@ -86,24 +81,6 @@ internal class GetPluginFeaturesResponseTests {
fun `Get Response should throw exception if allowed_config_type_list is absent in json`() {
val jsonString = """
{
"allowed_config_feature_list":["config_feature_1", "config_feature_2", "config_feature_3"],
"plugin_features":{
"FeatureKey1":"FeatureValue1",
"FeatureKey2":"FeatureValue2",
"FeatureKey3":"FeatureValue3"
}
}
""".trimIndent()
Assertions.assertThrows(IllegalArgumentException::class.java) {
createObjectFromJsonString(jsonString) { GetPluginFeaturesResponse.parse(it) }
}
}

@Test
fun `Get Response should throw exception if allowed_config_feature_list is absent in json`() {
val jsonString = """
{
"allowed_config_type_list":["config_type_1", "config_type_2", "config_type_3"],
"plugin_features":{
"FeatureKey1":"FeatureValue1",
"FeatureKey2":"FeatureValue2",
Expand All @@ -120,8 +97,7 @@ internal class GetPluginFeaturesResponseTests {
fun `Get Response should throw exception if plugin_features is absent in json`() {
val jsonString = """
{
"config_type_list":["config_type_1", "config_type_2", "config_type_3"],
"allowed_config_feature_list":["config_feature_1", "config_feature_2", "config_feature_3"]
"config_type_list":["config_type_1", "config_type_2", "config_type_3"]
}
""".trimIndent()
Assertions.assertThrows(IllegalArgumentException::class.java) {
Expand Down

0 comments on commit a6f4a0e

Please sign in to comment.