Skip to content

Commit

Permalink
Remove feature and feature_list usage for Notifications (#136)
Browse files Browse the repository at this point in the history
* Remove feature_list from NotificationConfig

Signed-off-by: Mohammad Qureshi <[email protected]>

* Remove feature from FeatueChannelsListRequest

Signed-off-by: Mohammad Qureshi <[email protected]>

* Remove 'feature' from EventSource

Signed-off-by: Mohammad Qureshi <[email protected]>

* Remove feature from LegacyPublishNotification

Signed-off-by: Mohammad Qureshi <[email protected]>

* Updated descriptions related to publishLegacyNotification

Signed-off-by: Mohammad Qureshi <[email protected]>
Signed-off-by: Zelin Hao <[email protected]>
  • Loading branch information
qreshi authored and zelinh committed Aug 18, 2022
1 parent 6dfd708 commit 64c68b1
Show file tree
Hide file tree
Showing 23 changed files with 103 additions and 305 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
package org.opensearch.commons.destination.message;

/**
* Supported legacy notification destinations for Index Management
* Supported legacy notification destinations for Alerting and Index Management
*/
public enum LegacyDestinationType {
LEGACY_CHIME,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ object NotificationConstants {
const val NAME_TAG = "name"
const val DESCRIPTION_TAG = "description"
const val IS_ENABLED_TAG = "is_enabled"
const val FEATURE_LIST_TAG = "feature_list"
const val TITLE_TAG = "title"
const val SEVERITY_TAG = "severity"
const val TAGS_TAG = "tags"
Expand Down Expand Up @@ -70,9 +69,5 @@ object NotificationConstants {
const val ALLOWED_CONFIG_FEATURE_LIST_TAG = "allowed_config_feature_list"
const val PLUGIN_FEATURES_TAG = "plugin_features"

const val FEATURE_ALERTING = "alerting"
const val FEATURE_INDEX_MANAGEMENT = "index_management"
const val FEATURE_REPORTS = "reports"

const val DEFAULT_MAX_ITEMS = 1000
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import org.opensearch.action.ActionResponse
import org.opensearch.client.node.NodeClient
import org.opensearch.common.io.stream.Writeable
import org.opensearch.commons.ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT
import org.opensearch.commons.notifications.NotificationConstants.FEATURE_INDEX_MANAGEMENT
import org.opensearch.commons.notifications.action.BaseResponse
import org.opensearch.commons.notifications.action.CreateNotificationConfigRequest
import org.opensearch.commons.notifications.action.CreateNotificationConfigResponse
Expand Down Expand Up @@ -201,7 +200,7 @@ object NotificationsPluginInterface {

/**
* Publishes a notification API using the legacy notification implementation. No REST API.
* Internal API only for the Index Management plugin.
* Internal API only for the Alerting and Index Management plugin, other consumers should use [sendNotification].
* @param client Node client for making transport action
* @param request The legacy publish notification request
* @param listener The listener for getting response
Expand All @@ -211,11 +210,6 @@ object NotificationsPluginInterface {
request: LegacyPublishNotificationRequest,
listener: ActionListener<LegacyPublishNotificationResponse>
) {
if (request.feature != FEATURE_INDEX_MANAGEMENT) {
// Do not change this; do not pass in FEATURE_INDEX_MANAGEMENT if you are not the Index Management plugin.
throw IllegalArgumentException("The publish notification method only supports the Index Management feature.")
}

client.execute(
LEGACY_PUBLISH_NOTIFICATION_ACTION_TYPE,
request,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ import org.opensearch.common.xcontent.ToXContentObject
import org.opensearch.common.xcontent.XContentBuilder
import org.opensearch.common.xcontent.XContentParser
import org.opensearch.common.xcontent.XContentParserUtils
import org.opensearch.commons.notifications.NotificationConstants.FEATURE_TAG
import org.opensearch.commons.notifications.NotificationConstants.COMPACT_TAG
import org.opensearch.commons.utils.logger
import java.io.IOException

/**
* This request is plugin-only call. i.e. REST interface is not exposed.
*/
class GetFeatureChannelListRequest : ActionRequest, ToXContentObject {
val feature: String
val compact: Boolean // Dummy request parameter for transport request

companion object {
private val log by logger(GetFeatureChannelListRequest::class.java)
Expand All @@ -39,7 +39,7 @@ class GetFeatureChannelListRequest : ActionRequest, ToXContentObject {
@JvmStatic
@Throws(IOException::class)
fun parse(parser: XContentParser): GetFeatureChannelListRequest {
var feature: String? = null
var compact = false

XContentParserUtils.ensureExpectedToken(
XContentParser.Token.START_OBJECT,
Expand All @@ -50,32 +50,40 @@ class GetFeatureChannelListRequest : ActionRequest, ToXContentObject {
val fieldName = parser.currentName()
parser.nextToken()
when (fieldName) {
FEATURE_TAG -> feature = parser.text()
COMPACT_TAG -> compact = parser.booleanValue()
else -> {
parser.skipChildren()
log.info("Unexpected field: $fieldName, while parsing GetFeatureChannelListRequest")
}
}
}
feature ?: throw IllegalArgumentException("$FEATURE_TAG field absent")
return GetFeatureChannelListRequest(feature)
return GetFeatureChannelListRequest()
}
}

/**
* {@inheritDoc}
*/
override fun toXContent(builder: XContentBuilder?, params: ToXContent.Params?): XContentBuilder {
return builder!!.startObject()
.field(COMPACT_TAG, compact)
.endObject()
}

/**
* constructor for creating the class
* @param feature the caller plugin feature
* @param compact Dummy request parameter for transport request
*/
constructor(feature: String) {
this.feature = feature
constructor(compact: Boolean = false) {
this.compact = compact
}

/**
* {@inheritDoc}
*/
@Throws(IOException::class)
constructor(input: StreamInput) : super(input) {
feature = input.readString()
compact = input.readBoolean()
}

/**
Expand All @@ -84,17 +92,7 @@ class GetFeatureChannelListRequest : ActionRequest, ToXContentObject {
@Throws(IOException::class)
override fun writeTo(output: StreamOutput) {
super.writeTo(output)
output.writeString(feature)
}

/**
* {@inheritDoc}
*/
override fun toXContent(builder: XContentBuilder?, params: ToXContent.Params?): XContentBuilder {
builder!!
return builder.startObject()
.field(FEATURE_TAG, feature)
.endObject()
output.writeBoolean(compact)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import java.io.IOException
*/
class LegacyPublishNotificationRequest : ActionRequest {
val baseMessage: LegacyBaseMessage
val feature: String

companion object {
/**
Expand All @@ -35,14 +34,11 @@ class LegacyPublishNotificationRequest : ActionRequest {
/**
* constructor for creating the class
* @param baseMessage the base message to send
* @param feature the feature that is trying to use this request
*/
constructor(
baseMessage: LegacyBaseMessage,
feature: String
baseMessage: LegacyBaseMessage
) {
this.baseMessage = baseMessage
this.feature = feature
}

/**
Expand All @@ -55,7 +51,6 @@ class LegacyPublishNotificationRequest : ActionRequest {
LegacyDestinationType.LEGACY_CUSTOM_WEBHOOK -> LegacyCustomWebhookMessage(input)
LegacyDestinationType.LEGACY_SLACK -> LegacySlackMessage(input)
}
feature = input.readString()
}

/**
Expand All @@ -66,7 +61,6 @@ class LegacyPublishNotificationRequest : ActionRequest {
super.writeTo(output)
output.writeEnum(baseMessage.channelType)
baseMessage.writeTo(output)
output.writeString(feature)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ object NotificationsActions {

/**
* Publish legacy notification message. Internal only - Inter plugin communication.
* Only for the Index Management plugin.
* Only for the Alerting and Index Management plugins.
*/
const val LEGACY_PUBLISH_NOTIFICATION_NAME = "cluster:admin/opensearch/notifications/feature/publish"

Expand Down Expand Up @@ -106,7 +106,7 @@ object NotificationsActions {

/**
* Send legacy notification transport action type. Internal only - Inter plugin communication.
* Only for the Index Management plugin.
* Only for the Alerting and Index Management plugins.
*/
val LEGACY_PUBLISH_NOTIFICATION_ACTION_TYPE =
ActionType(LEGACY_PUBLISH_NOTIFICATION_NAME, ::LegacyPublishNotificationResponse)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,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.FEATURE_TAG
import org.opensearch.commons.notifications.NotificationConstants.REFERENCE_ID_TAG
import org.opensearch.commons.notifications.NotificationConstants.SEVERITY_TAG
import org.opensearch.commons.notifications.NotificationConstants.TAGS_TAG
Expand All @@ -27,7 +26,6 @@ import java.io.IOException
data class EventSource(
val title: String,
val referenceId: String,
val feature: String,
val severity: SeverityType = SeverityType.INFO,
val tags: List<String> = listOf()
) : BaseModel {
Expand All @@ -53,7 +51,6 @@ data class EventSource(
fun parse(parser: XContentParser): EventSource {
var title: String? = null
var referenceId: String? = null
var feature: String? = null
var severity: SeverityType = SeverityType.INFO
var tags: List<String> = emptyList()

Expand All @@ -68,7 +65,6 @@ data class EventSource(
when (fieldName) {
TITLE_TAG -> title = parser.text()
REFERENCE_ID_TAG -> referenceId = parser.text()
FEATURE_TAG -> feature = parser.text()
SEVERITY_TAG -> severity = SeverityType.fromTagOrDefault(parser.text())
TAGS_TAG -> tags = parser.stringList()
else -> {
Expand All @@ -79,12 +75,10 @@ data class EventSource(
}
title ?: throw IllegalArgumentException("$TITLE_TAG field absent")
referenceId ?: throw IllegalArgumentException("$REFERENCE_ID_TAG field absent")
feature ?: throw IllegalArgumentException("$FEATURE_TAG field absent")

return EventSource(
title,
referenceId,
feature,
severity,
tags
)
Expand All @@ -99,7 +93,6 @@ data class EventSource(
return builder.startObject()
.field(TITLE_TAG, title)
.field(REFERENCE_ID_TAG, referenceId)
.field(FEATURE_TAG, feature)
.field(SEVERITY_TAG, severity.tag)
.field(TAGS_TAG, tags)
.endObject()
Expand All @@ -112,7 +105,6 @@ data class EventSource(
constructor(input: StreamInput) : this(
title = input.readString(),
referenceId = input.readString(),
feature = input.readString(),
severity = input.readEnum(SeverityType::class.java),
tags = input.readStringList()
)
Expand All @@ -123,7 +115,6 @@ data class EventSource(
override fun writeTo(output: StreamOutput) {
output.writeString(title)
output.writeString(referenceId)
output.writeString(feature)
output.writeEnum(severity)
output.writeStringCollection(tags)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,13 @@ import org.opensearch.common.xcontent.XContentParser
import org.opensearch.common.xcontent.XContentParserUtils
import org.opensearch.commons.notifications.NotificationConstants.CONFIG_TYPE_TAG
import org.opensearch.commons.notifications.NotificationConstants.DESCRIPTION_TAG
import org.opensearch.commons.notifications.NotificationConstants.FEATURE_LIST_TAG
import org.opensearch.commons.notifications.NotificationConstants.IS_ENABLED_TAG
import org.opensearch.commons.notifications.NotificationConstants.NAME_TAG
import org.opensearch.commons.notifications.model.config.ConfigDataProperties.createConfigData
import org.opensearch.commons.notifications.model.config.ConfigDataProperties.getReaderForConfigType
import org.opensearch.commons.notifications.model.config.ConfigDataProperties.validateConfigData
import org.opensearch.commons.utils.STRING_READER
import org.opensearch.commons.utils.STRING_WRITER
import org.opensearch.commons.utils.fieldIfNotNull
import org.opensearch.commons.utils.logger
import org.opensearch.commons.utils.stringList
import java.io.IOException

/**
Expand All @@ -34,7 +30,6 @@ data class NotificationConfig(
val name: String,
val description: String,
val configType: ConfigType,
val features: Set<String>,
val configData: BaseConfigData?,
val isEnabled: Boolean = true
) : BaseModel {
Expand Down Expand Up @@ -68,7 +63,6 @@ data class NotificationConfig(
var name: String? = null
var description = ""
var configType: ConfigType? = null
var features: Set<String>? = null
var isEnabled = true
var configData: BaseConfigData? = null
XContentParserUtils.ensureExpectedToken(
Expand All @@ -83,7 +77,6 @@ data class NotificationConfig(
NAME_TAG -> name = parser.text()
DESCRIPTION_TAG -> description = parser.text()
CONFIG_TYPE_TAG -> configType = ConfigType.fromTagOrDefault(parser.text())
FEATURE_LIST_TAG -> features = parser.stringList().toSet()
IS_ENABLED_TAG -> isEnabled = parser.booleanValue()
else -> {
val configTypeForTag = ConfigType.fromTagOrDefault(fieldName)
Expand All @@ -98,12 +91,10 @@ data class NotificationConfig(
}
name ?: throw IllegalArgumentException("$NAME_TAG field absent")
configType ?: throw IllegalArgumentException("$CONFIG_TYPE_TAG field absent")
features ?: throw IllegalArgumentException("$FEATURE_LIST_TAG field absent")
return NotificationConfig(
name,
description,
configType,
features,
configData,
isEnabled
)
Expand All @@ -119,7 +110,6 @@ data class NotificationConfig(
.field(NAME_TAG, name)
.field(DESCRIPTION_TAG, description)
.field(CONFIG_TYPE_TAG, configType.tag)
.field(FEATURE_LIST_TAG, features)
.field(IS_ENABLED_TAG, isEnabled)
.fieldIfNotNull(configType.tag, configData)
.endObject()
Expand All @@ -133,7 +123,6 @@ data class NotificationConfig(
name = input.readString(),
description = input.readString(),
configType = input.readEnum(ConfigType::class.java),
features = input.readSet(STRING_READER),
isEnabled = input.readBoolean(),
configData = input.readOptionalWriteable(getReaderForConfigType(input.readEnum(ConfigType::class.java)))
)
Expand All @@ -145,7 +134,6 @@ data class NotificationConfig(
output.writeString(name)
output.writeString(description)
output.writeEnum(configType)
output.writeCollection(features, STRING_WRITER)
output.writeBoolean(isEnabled)
// Reading config types multiple times in constructor
output.writeEnum(configType)
Expand Down
Loading

0 comments on commit 64c68b1

Please sign in to comment.