Skip to content

Commit 95b7e8c

Browse files
toepkerd-zztoepkerd-zz
committed
refactored MonitorV2 parse to read outer monitor type field and delegate to correct parse func accordingly
1 parent 29ae09b commit 95b7e8c

File tree

4 files changed

+36
-49
lines changed

4 files changed

+36
-49
lines changed

‎src/main/kotlin/org/opensearch/commons/alerting/model/MonitorV2.kt‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import org.opensearch.common.CheckedFunction
66
import org.opensearch.commons.alerting.model.Monitor.Companion
77
import org.opensearch.commons.alerting.model.Monitor.Companion.INPUTS_FIELD
88
import org.opensearch.commons.alerting.model.PPLMonitor.Companion.PPL_MONITOR_TYPE
9+
import org.opensearch.commons.alerting.model.PPLTrigger.Companion.PPL_TRIGGER_FIELD
910
import org.opensearch.commons.alerting.model.Trigger.Type
1011
import org.opensearch.commons.alerting.util.IndexUtils.Companion._ID
1112
import org.opensearch.commons.alerting.util.IndexUtils.Companion._VERSION
@@ -21,7 +22,6 @@ import org.opensearch.core.xcontent.XContentBuilder
2122
import org.opensearch.core.xcontent.XContentParser
2223
import org.opensearch.core.xcontent.XContentParserUtils
2324

24-
// TODO: maybe make this abstract class? put init block logic here for all monitors?
2525
interface MonitorV2 : ScheduledJob {
2626
override val id: String
2727
override val version: Long
@@ -50,7 +50,6 @@ interface MonitorV2 : ScheduledJob {
5050

5151
companion object {
5252
// scheduled job field names
53-
const val TYPE_FIELD = "type"
5453
const val MONITOR_V2_TYPE = "monitor_v2" // scheduled job type is MonitorV2
5554

5655
// field names
@@ -76,25 +75,26 @@ interface MonitorV2 : ScheduledJob {
7675
@JvmOverloads
7776
@Throws(IOException::class)
7877
fun parse(xcp: XContentParser): MonitorV2 {
79-
/*
80-
TODO: this default implementation is short-term and inextensible
81-
a correct implementation should 1) scan for monitor type field
82-
2) delegate to the parse function of the MonitorV2 implementation,
83-
just like how TriggerV2 interface does it.
84-
The problem is the (internal) monitor type field is at the same
85-
level as all the other monitor fields, which means we would need some
86-
way of parsing the same XContent twice
87-
possible work around: require monitor type to be very first field
88-
if first monitor type field is absent, assume ppl monitor as default
89-
*/
90-
return PPLMonitor.parse(xcp)
78+
/* parse outer object for monitorV2 type, then delegate to correct monitorV2 parser */
79+
80+
XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, xcp.currentToken(), xcp) // outer monitor object start
81+
82+
XContentParserUtils.ensureExpectedToken(XContentParser.Token.FIELD_NAME, xcp.nextToken(), xcp) // monitor type field name
83+
val monitorTypeText = xcp.currentName()
84+
val monitorType = MonitorV2Type.enumFromString(monitorTypeText)
85+
?: throw IllegalStateException("when parsing MonitorV2, received invalid monitor type: $monitorTypeText")
86+
87+
XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, xcp.nextToken(), xcp) // inner monitor object start
88+
89+
return when (monitorType) {
90+
MonitorV2Type.PPL_MONITOR -> PPLMonitor.parse(xcp)
91+
}
9192
}
9293

9394
fun readFrom(sin: StreamInput): MonitorV2 {
94-
val monitorType = sin.readEnum(MonitorV2Type::class.java)
95-
return when (monitorType) {
95+
return when (val monitorType = sin.readEnum(MonitorV2Type::class.java)) {
9696
MonitorV2Type.PPL_MONITOR -> PPLMonitor(sin)
97-
else -> throw IllegalStateException("Unexpected input [$monitorType] when reading MonitorV2")
97+
else -> throw IllegalStateException("Unexpected input \"$monitorType\" when reading MonitorV2")
9898
}
9999
}
100100

‎src/main/kotlin/org/opensearch/commons/alerting/model/PPLMonitor.kt‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,6 @@ import org.opensearch.commons.alerting.model.MonitorV2.Companion.NO_ID
1313
import org.opensearch.commons.alerting.model.MonitorV2.Companion.NO_VERSION
1414
import org.opensearch.commons.alerting.model.MonitorV2.Companion.SCHEDULE_FIELD
1515
import org.opensearch.commons.alerting.model.MonitorV2.Companion.TRIGGERS_FIELD
16-
import org.opensearch.commons.alerting.model.MonitorV2.Companion.TYPE_FIELD
17-
import org.opensearch.commons.alerting.model.PPLTrigger.Companion.NUM_RESULTS_CONDITION_FIELD
18-
import org.opensearch.commons.alerting.model.PPLTrigger.NumResultsCondition
19-
import org.opensearch.commons.alerting.model.PPLTrigger.TriggerMode
2016
import org.opensearch.commons.alerting.util.IndexUtils.Companion._ID
2117
import org.opensearch.commons.alerting.util.IndexUtils.Companion._VERSION
2218
import org.opensearch.commons.alerting.util.instant
@@ -94,16 +90,18 @@ data class PPLMonitor(
9490
)
9591

9692
override fun toXContent(builder: XContentBuilder, params: ToXContent.Params): XContentBuilder {
97-
builder.startObject()
93+
builder.startObject() // overall start object
9894

9995
// if this is being written as ScheduledJob, add extra object layer and add ScheduledJob
10096
// related metadata, default to false
10197
if (params.paramAsBoolean("with_type", false)) {
10298
builder.startObject(MONITOR_V2_TYPE)
10399
}
104100

105-
// this field is ScheduledJob metadata, include despite it not being a class field
106-
builder.field(MONITOR_TYPE_FIELD, PPL_MONITOR_TYPE)
101+
// wrap PPLMonitor in outer object named after its monitor type
102+
// required for MonitorV2 XContentParser to first encounter this,
103+
// read in monitor type, then delegate to correct parse() function
104+
builder.startObject(PPL_MONITOR_TYPE) // monitor type start object
107105

108106
builder.field(NAME_FIELD, name)
109107
builder.field(SCHEDULE_FIELD, schedule)
@@ -113,13 +111,16 @@ data class PPLMonitor(
113111
builder.field(TRIGGERS_FIELD, triggers.toTypedArray())
114112
builder.field(QUERY_LANGUAGE_FIELD, queryLanguage.value)
115113
builder.field(QUERY_FIELD, query)
116-
builder.endObject()
114+
115+
builder.endObject() // monitor type end object
117116

118117
// if ScheduledJob metadata was added, end the extra object layer that was created
119118
if (params.paramAsBoolean("with_type", false)) {
120119
builder.endObject()
121120
}
122121

122+
builder.endObject() // overall end object
123+
123124
return builder
124125
}
125126

‎src/main/kotlin/org/opensearch/commons/alerting/model/PPLTrigger.kt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,14 @@ data class PPLTrigger(
193193

194194
/* parse */
195195
XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, xcp.currentToken(), xcp) // outer trigger object start
196+
196197
XContentParserUtils.ensureExpectedToken(XContentParser.Token.FIELD_NAME, xcp.nextToken(), xcp) // ppl_trigger field name
198+
val triggerType = xcp.currentName()
199+
if (triggerType != PPL_TRIGGER_FIELD) {
200+
throw IllegalStateException("when parsing PPLMonitor, expected trigger to be of type $PPL_TRIGGER_FIELD " +
201+
"but instead got \"$triggerType\"")
202+
}
203+
197204
XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, xcp.nextToken(), xcp) // inner trigger object start
198205

199206
while (xcp.nextToken() != XContentParser.Token.END_OBJECT) {

‎src/main/kotlin/org/opensearch/commons/alerting/model/TriggerV2.kt‎

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ interface TriggerV2 : BaseModel {
1313
val id: String
1414
val name: String
1515
val severity: Severity
16-
val suppressDuration: TimeValue? // TODO: move to MonitorV2 definition
17-
val expireDuration: TimeValue? // TODO: move to MonitorV2 definition
16+
val suppressDuration: TimeValue?
17+
val expireDuration: TimeValue?
1818
var lastTriggeredTime: Instant?
1919
val actions: List<Action>
2020

@@ -49,33 +49,12 @@ interface TriggerV2 : BaseModel {
4949
const val EXPIRE_FIELD = "expires"
5050
const val ACTIONS_FIELD = "actions"
5151

52-
// @Throws(IOException::class)
53-
// fun parse(xcp: XContentParser): TriggerV2 {
54-
// // TODO: dead code until a MonitorV2 interface level parse() that delegates by monitor type is implemented
55-
// val trigger: TriggerV2
56-
//
57-
// val triggerV2TypeNames = TriggerV2Type.entries.map { it.value }
58-
//
59-
// XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, xcp.currentToken(), xcp)
60-
// XContentParserUtils.ensureExpectedToken(XContentParser.Token.FIELD_NAME, xcp.nextToken(), xcp)
61-
//
62-
// if (!triggerV2TypeNames.contains(xcp.currentName())) {
63-
// throw IllegalArgumentException("Invalid trigger type ${xcp.currentName()}")
64-
// }
65-
//
66-
// XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, xcp.nextToken(), xcp)
67-
// trigger = xcp.namedObject(TriggerV2::class.java, xcp.currentName(), null)
68-
// XContentParserUtils.ensureExpectedToken(XContentParser.Token.END_OBJECT, xcp.nextToken(), xcp)
69-
//
70-
// return trigger
71-
// }
72-
7352
@JvmStatic
7453
@Throws(IOException::class)
7554
fun readFrom(sin: StreamInput): TriggerV2 {
7655
return when (val type = sin.readEnum(TriggerV2Type::class.java)) {
7756
TriggerV2Type.PPL_TRIGGER -> PPLTrigger(sin)
78-
else -> throw IllegalStateException("Unexpected input [$type] when reading TriggerV2")
57+
else -> throw IllegalStateException("Unexpected input \"$type\" when reading TriggerV2")
7958
}
8059
}
8160
}

0 commit comments

Comments
 (0)