From 1d7d3f630832e1618f808827db761aa763bc5f04 Mon Sep 17 00:00:00 2001
From: Mikko Karjalainen <mikko.e.karjalainen@gmail.com>
Date: Tue, 26 Nov 2019 11:37:52 +0000
Subject: [PATCH 1/5] Common tags: first cut & temp save.

---
 .../main/kotlin/com/hotels/styx/CommonTags.kt | 79 ++++++++++++++++
 .../main/kotlin/com/hotels/styx/ObjectTags.kt | 92 ++++++++++---------
 .../routing/handlers/LoadBalancingGroup.kt    | 12 +--
 .../services/HealthCheckMonitoringService.kt  |  9 +-
 .../styx/services/OriginsAdminHandler.kt      | 11 ++-
 .../styx/services/OriginsConfigConverter.kt   |  2 +-
 .../styx/services/OriginsPageRenderer.kt      |  4 +-
 .../services/YamlFileConfigurationService.kt  |  8 +-
 .../com/hotels/styx/ObjectTagsKtTest.kt       | 40 ++++----
 .../styx/services/OriginsAdminHandlerTest.kt  |  6 +-
 10 files changed, 173 insertions(+), 90 deletions(-)
 create mode 100644 components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt

diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt b/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt
new file mode 100644
index 0000000000..9ec7ef280a
--- /dev/null
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt
@@ -0,0 +1,79 @@
+/*
+  Copyright (C) 2013-2019 Expedia Inc.
+
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+ */
+package com.hotels.styx
+
+/*
+ * Common Tag Operations:
+ */
+
+sealed class CommonValueTag<T>(
+        val name: String,
+        val encode: (T) -> String?,
+        val decode: (String) -> T?) {
+
+    fun valuePart(tag: String) = if (tag.startsWith("$name=")) {
+        tag.removePrefix("$name=")
+    } else {
+        null
+    }
+
+    fun match(tag: String) = tag.startsWith("$name=")
+
+    fun valueOf(tag: String) = valuePart(tag)
+            ?.let { decode(it) }
+
+    fun find(tags: Set<String>) = tags.firstOrNull { this.match(it) }
+            ?.let { valuePart(it) }
+            ?.let { this.decode(it) }
+}
+
+
+class NullableValueTag<T>(
+        name: String,
+        encode: (T) -> String?,
+        decode: (String) -> T?) : CommonValueTag<T>(name, encode, decode) {
+
+    operator fun invoke(value: T): String? = encode(value)
+            ?.let {
+                "$name=$it"
+            }
+}
+
+class SafeValueTag<T>(
+        name: String,
+        encode: (T) -> String?,
+        decode: (String) -> T?) : CommonValueTag<T>(name, encode, decode) {
+
+    operator fun invoke(value: T): String = encode(value)
+            .let {
+                it!!
+                "$name=$it"
+            }
+}
+
+
+//fun Set<String>.contains(tag: CommonValueTag<*>) = this.firstOrNull { tag.match(it) } != null
+
+// TODO: Compare to ObjectTag.find()
+fun <T> Set<String>.valueOf(tag: CommonValueTag<T>): T? = this.firstOrNull { tag.match(it) }
+        ?.let { tag.valuePart(it) }
+        ?.let { tag.decode(it) }
+
+// TODO: Compare to ObjectTag.valueOf():
+fun <T> String.match(tag: CommonValueTag<T>) = tag.valuePart(this)
+        ?.let { tag.decode(it) }
+
+fun String.isA(tag: CommonValueTag<*>) = tag.match(this)
diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/ObjectTags.kt b/components/proxy/src/main/kotlin/com/hotels/styx/ObjectTags.kt
index 0a9f5bba20..e67319c2f3 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/ObjectTags.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/ObjectTags.kt
@@ -15,58 +15,64 @@
  */
 package com.hotels.styx
 
-private const val LBGROUP = "lbGroup"
-private val LBGROUP_REGEX = "$LBGROUP=(.+)".toRegex()
-fun lbGroupTag(name: String) = "lbGroup=$name"
-fun lbGroupTag(tags: Set<String>) = tags.firstOrNull(::isLbGroupTag)
-fun isLbGroupTag(tag: String) = LBGROUP_REGEX.matches(tag)
-fun lbGroupTagValue(tags: Set<String>) = lbGroupTagValue(lbGroupTag(tags)?:"")
-fun lbGroupTagValue(tag: String): String? = LBGROUP_REGEX.matchEntire(tag)
-        ?.groupValues
-        ?.get(1)
+/*
+ * TAG: lbGroup
+ */
+val lbGroupTag = SafeValueTag(
+        "lbGroup",
+        { it },
+        { it })
+
+/*
+ * TAG: source
+ */
+val sourceTag = SafeValueTag(
+        "source",
+        { it },
+        { it })
 
-fun sourceTag(creator: String) = "source=$creator"
-fun sourceTag(tags: Set<String>) = tags.firstOrNull { it.startsWith("source=") }
-fun sourceTagValue(tags: Set<String>) = sourceTag(tags)?.substring("source".length + 1)
 
-private const val STATE = "state"
+/*
+ * TAG: state
+ */
 const val STATE_ACTIVE = "active"
 const val STATE_UNREACHABLE = "unreachable"
 const val STATE_INACTIVE = "inactive"
-private val STATE_REGEX = "$STATE=(.+)".toRegex()
-fun stateTag(value: String) = "$STATE=$value"
-fun stateTag(tags: Set<String>) = tags.firstOrNull(::isStateTag)
-fun isStateTag(tag: String) = STATE_REGEX.matches(tag)
-fun stateTagValue(tags: Set<String>) = stateTagValue(stateTag(tags)?:"")
-fun stateTagValue(tag: String) = STATE_REGEX.matchEntire(tag)
-        ?.groupValues
-        ?.get(1)
 
+val stateTag = SafeValueTag(
+        "state",
+        { it },
+        { it })
+
+/*
+ * TAG: healthCheck
+ * healthCheck=on
+ * healthCheck=on;probes-OK:2
+ * healthCheck=on;probes-FAIL:1
+ */
 private const val HEALTHCHECK = "healthCheck"
 const val HEALTHCHECK_PASSING = "probes-OK"
 const val HEALTHCHECK_FAILING = "probes-FAIL"
 const val HEALTHCHECK_ON = "on"
+private val HEALTHCHECK_REGEX = "$HEALTHCHECK_ON(?:;(.+):([0-9]+))?".toRegex()
+
 
-// healthCheck=on
-// healthCheck=on;probes-OK:2
-// healthCheck=on;probes-FAIL:1
-private val HEALTHCHECK_REGEX = "$HEALTHCHECK=$HEALTHCHECK_ON(?:;(.+):([0-9]+))?".toRegex()
-fun healthCheckTag(value: Pair<String, Int>?) =
-        if (value != null && value.first.isNotBlank() && value.second > 0) {
-            "$HEALTHCHECK=$HEALTHCHECK_ON;${value.first}:${value.second}"
-        } else if (value != null && value.first.isNotBlank() && value.second == 0) {
-            "$HEALTHCHECK=$HEALTHCHECK_ON"
-        } else {
-            null
-        }
-fun healthCheckTag(tags: Set<String>) = tags.firstOrNull(::isHealthCheckTag)
-fun isHealthCheckTag(tag: String) = HEALTHCHECK_REGEX.matches(tag)
-fun healthCheckTagValue(tags: Set<String>) = healthCheckTagValue(healthCheckTag(tags)?:"")
-fun healthCheckTagValue(tag: String) = HEALTHCHECK_REGEX.matchEntire(tag)
-        ?.groupValues
-        ?.let {
-            if (it[1].isNotEmpty()) {
-                Pair(it[1], it[2].toInt())
+val healthCheckTag = NullableValueTag(
+        "healthCheck",
+        { value -> if (value.first.isNotBlank() && value.second > 0) {
+                "$HEALTHCHECK_ON;${value.first}:${value.second}"
+            } else if (value.first.isNotBlank() && value.second == 0) {
+                HEALTHCHECK_ON
             } else {
-                Pair(HEALTHCHECK_ON, 0)
-            }}
+                null
+            }
+        },
+        { tagValue -> HEALTHCHECK_REGEX.matchEntire(tagValue)
+                ?.groupValues
+                ?.let {
+                    if (it[1].isNotEmpty()) {
+                        Pair(it[1], it[2].toInt())
+                    } else {
+                        Pair(HEALTHCHECK_ON, 0)
+                    }}
+        })
diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/routing/handlers/LoadBalancingGroup.kt b/components/proxy/src/main/kotlin/com/hotels/styx/routing/handlers/LoadBalancingGroup.kt
index c2a7b104ed..950d3cb136 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/routing/handlers/LoadBalancingGroup.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/routing/handlers/LoadBalancingGroup.kt
@@ -16,7 +16,6 @@
 package com.hotels.styx.routing.handlers
 
 import com.fasterxml.jackson.annotation.JsonProperty
-import com.hotels.styx.*
 import com.hotels.styx.api.Eventual
 import com.hotels.styx.api.HttpInterceptor
 import com.hotels.styx.api.Id
@@ -40,10 +39,12 @@ import com.hotels.styx.config.schema.SchemaDsl.integer
 import com.hotels.styx.config.schema.SchemaDsl.optional
 import com.hotels.styx.config.schema.SchemaDsl.string
 import com.hotels.styx.infrastructure.configuration.yaml.JsonNodeConfig
+import com.hotels.styx.lbGroupTag
 import com.hotels.styx.routing.RoutingObject
 import com.hotels.styx.routing.RoutingObjectRecord
 import com.hotels.styx.routing.config.RoutingObjectFactory
 import com.hotels.styx.routing.config.StyxObjectDefinition
+import com.hotels.styx.stateTag
 import org.slf4j.LoggerFactory
 import reactor.core.Disposable
 import reactor.core.publisher.toFlux
@@ -121,17 +122,16 @@ internal class LoadBalancingGroup(val client: StyxBackendServiceClient, val chan
 
         private fun routeDatabaseChanged(appId: String, snapshot: ObjectStore<RoutingObjectRecord>, remoteHosts: AtomicReference<Set<RemoteHost>>) {
             val newSet = snapshot.entrySet()
-                    .filter { taggedWith(it, ::lbGroupTagValue, appId) }
-                    .filter { taggedWith(it, ::stateTagValue, STATE_ACTIVE, null) }
+                    .filter { it.value.tags.contains(lbGroupTag(appId)) }
+                    .filter { stateTag.find(it.value.tags)
+                            .let { it == null || it == "active" }
+                    }
                     .map { toRemoteHost(appId, it) }
                     .toSet()
 
             remoteHosts.set(newSet)
         }
 
-        private fun taggedWith(recordEntry: Map.Entry<String, RoutingObjectRecord>, tagValue: (Set<String>) -> String?, vararg values: String?) =
-                values.contains(tagValue(recordEntry.value.tags))
-
         private fun toRemoteHost(appId: String, record: Map.Entry<String, RoutingObjectRecord>): RemoteHost {
             val routingObject = record.value.routingObject
             val originName = record.key
diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/services/HealthCheckMonitoringService.kt b/components/proxy/src/main/kotlin/com/hotels/styx/services/HealthCheckMonitoringService.kt
index 3b2d79644d..44690d1720 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/services/HealthCheckMonitoringService.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/services/HealthCheckMonitoringService.kt
@@ -102,7 +102,7 @@ internal class HealthCheckMonitoringService(
                 .filter { (_, record) -> record.tags.contains(lbGroupTag(application)) }
                 .map { (name, record) ->
                     val tags = record.tags
-                    val objectHealth = objectHealthFrom(stateTagValue(tags), healthCheckTagValue(tags))
+                    val objectHealth = objectHealthFrom(stateTag.find(tags), tags.valueOf(healthCheckTag))
                     Triple(name, record, objectHealth)
                 }
 
@@ -177,6 +177,7 @@ internal fun objectHealthFrom(state: String?, health: Pair<String, Int>?) =
 
 internal class ObjectDisappearedException : RuntimeException("Object disappeared")
 
+
 private fun markObject(db: StyxObjectStore<RoutingObjectRecord>, name: String, newStatus: ObjectHealth) {
     // The ifPresent is not ideal, but compute() does not allow the computation to return null. So we can't preserve
     // a state where the object does not exist using compute alone. But even with ifPresent, as we are open to
@@ -203,13 +204,13 @@ private fun markObject(db: StyxObjectStore<RoutingObjectRecord>, name: String, n
 
 internal fun reTag(tags: Set<String>, newStatus: ObjectHealth) =
     tags.asSequence()
-            .filterNot { isStateTag(it) || isHealthCheckTag(it) }
+            .filterNot { it.isA(stateTag) || it.isA(healthCheckTag) }
             .plus(stateTag(newStatus.state()))
-            .plus(healthCheckTag(newStatus.health()))
+            .plus(healthCheckTag(newStatus.health()!!))
             .filterNotNull()
             .toSet()
 
 private val RELEVANT_STATES = setOf(STATE_ACTIVE, STATE_UNREACHABLE)
 private fun containsRelevantStateTag(entry: Map.Entry<String, RoutingObjectRecord>) =
-        stateTagValue(entry.value.tags) in RELEVANT_STATES
+        entry.value.tags.valueOf(stateTag) in RELEVANT_STATES
 
diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsAdminHandler.kt b/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsAdminHandler.kt
index 27fff536b3..8437e4bcfc 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsAdminHandler.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsAdminHandler.kt
@@ -63,7 +63,7 @@ internal class OriginsAdminHandler(
     private fun getState(objectId: String) : HttpResponse {
         val origin = findOrigin(objectId)
         return if (origin != null) {
-            textValueResponse(stateTagValue(origin.tags) ?: "")
+            textValueResponse(origin.tags.valueOf(stateTag) ?: "")
         } else {
             errorResponse(NOT_FOUND, "No origin found for ID $objectId")
         }
@@ -92,7 +92,7 @@ internal class OriginsAdminHandler(
             if (!isValidOrigin(origin)) {
                 throw HttpStatusException(NOT_FOUND, "No origin found for ID $objectId")
             }
-            newState = when(stateTagValue(origin!!.tags)) {
+            newState = when(origin!!.tags.valueOf(stateTag)) {
                 STATE_INACTIVE, STATE_ACTIVE -> STATE_ACTIVE
                 STATE_UNREACHABLE -> STATE_UNREACHABLE
                 else -> STATE_ACTIVE
@@ -119,9 +119,10 @@ internal class OriginsAdminHandler(
     private fun updateStateTag(origin: RoutingObjectRecord, newValue: String, clearHealthcheck: Boolean = false) : RoutingObjectRecord {
         val oldTags = origin.tags
         val newTags = oldTags
-                .filterNot{ clearHealthcheck && isHealthCheckTag(it) }
-                .filterNot(::isStateTag)
-                .plus(stateTag(newValue)).toSet()
+                .filterNot{ clearHealthcheck && it.isA(healthCheckTag) }
+                .filterNot{ it.isA(stateTag) }
+                .plus(stateTag(newValue))
+                .toSet()
         return if (oldTags != newTags) {
             origin.copy(tags = newTags)
         } else {
diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsConfigConverter.kt b/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsConfigConverter.kt
index 4fd1beaebd..1fc4933259 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsConfigConverter.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsConfigConverter.kt
@@ -151,7 +151,7 @@ internal class OriginsConfigConverter(
         }
 
         internal fun hostProxy(app: BackendService, origin: Origin) : StyxObjectDefinition {
-            val healthCheckTag :String = if (isHealthCheckConfigured(app)) stateTag(STATE_UNREACHABLE) else stateTag(STATE_ACTIVE);
+            val healthCheckTag :String = if (isHealthCheckConfigured(app)) stateTag(STATE_UNREACHABLE)!! else stateTag(STATE_ACTIVE)!!
 
             return StyxObjectDefinition(
                 "${app.id()}.${origin.id()}",
diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsPageRenderer.kt b/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsPageRenderer.kt
index 23149fb163..8b4786a6d3 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsPageRenderer.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsPageRenderer.kt
@@ -18,7 +18,7 @@ package com.hotels.styx.services
 import com.fasterxml.jackson.databind.JsonNode
 import com.hotels.styx.infrastructure.configuration.yaml.JsonNodeConfig
 import com.hotels.styx.lbGroupTag
-import com.hotels.styx.lbGroupTagValue
+import com.hotels.styx.match
 import com.hotels.styx.routing.RoutingObjectRecord
 import com.hotels.styx.routing.config.RoutingConfigParser.toRoutingConfigNode
 import com.hotels.styx.routing.config.StyxObjectDefinition
@@ -252,7 +252,7 @@ internal class OriginsPageRenderer(val assetsRoot: String, val provider: String,
     private fun tagsExceptStatusAndAppname(tags: Set<String>) = tags
             .filterNot { it.startsWith("state:active") }
             .filterNot { it.startsWith("state:inactive") }
-            .filter { lbGroupTagValue(it) == null }
+            .filter { lbGroupTag.valueOf(it) == null }
 
     private fun isSecureHost(config: JsonNode) = config.get("tlsSettings") != null
 }
diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/services/YamlFileConfigurationService.kt b/components/proxy/src/main/kotlin/com/hotels/styx/services/YamlFileConfigurationService.kt
index f8a354dcfd..e612665529 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/services/YamlFileConfigurationService.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/services/YamlFileConfigurationService.kt
@@ -36,7 +36,7 @@ import com.hotels.styx.server.handlers.ClassPathResourceHandler
 import com.hotels.styx.serviceproviders.ServiceProviderFactory
 import com.hotels.styx.services.OriginsConfigConverter.Companion.deserialiseOrigins
 import com.hotels.styx.sourceTag
-import com.hotels.styx.sourceTagValue
+import com.hotels.styx.valueOf
 import org.slf4j.LoggerFactory
 import java.lang.RuntimeException
 import java.nio.charset.StandardCharsets.UTF_8
@@ -61,7 +61,7 @@ internal class YamlFileConfigurationService(
 
     private val ingressObjectName = if (config.ingressObject.isNotBlank()) config.ingressObject else "$name-router"
 
-    private val objectSourceTag = sourceTag(name)
+    private val objectSourceTag = sourceTag(name)!!
 
     private val fileMonitoringService = FileMonitoringService("YamlFileCoinfigurationService", config.originsFile, pollInterval) {
         reloadAction(it)
@@ -116,7 +116,7 @@ internal class YamlFileConfigurationService(
 
             routingObjectDefs.forEach { objectDef ->
                 routeDb.get(objectDef.name()).ifPresent {
-                    if (sourceTagValue(it.tags) != name) {
+                    if (it.tags.valueOf(sourceTag) != name) {
                         throw DuplicateObjectException("Object name='${objectDef.name()}' already exists. Provider='${name}', file='${config.originsFile}'.")
                     }
                 }
@@ -127,7 +127,7 @@ internal class YamlFileConfigurationService(
 
             healthMonitors.forEach { (objectName, _) ->
                 serviceDb.get(objectName).ifPresent {
-                    if (sourceTagValue(it.tags) != name) {
+                    if (it.tags.valueOf(sourceTag) != name) {
                         throw DuplicateObjectException("Health Monitor name='${objectName}' already exists. Provider='${name}', file='${config.originsFile}'.")
                     }
                 }
diff --git a/components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsKtTest.kt b/components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsKtTest.kt
index 473978213e..e5b1afc2d1 100644
--- a/components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsKtTest.kt
+++ b/components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsKtTest.kt
@@ -23,14 +23,14 @@ class ObjectTagsKtTest : BehaviorSpec({
     given("An lbGroup tag matcher") {
         `when`("tag matches") {
             then("returns tag value") {
-                lbGroupTagValue("lbGroup=abc").shouldBe("abc")
+                "lbGroup=abc".match(lbGroupTag).shouldBe("abc")
             }
         }
         `when`("tag doesn't match") {
             then("returns null") {
-                lbGroupTagValue("abc").shouldBeNull()
-                lbGroupTagValue("lbGroup=").shouldBeNull()
-                lbGroupTagValue("").shouldBeNull()
+                "abc".match(lbGroupTag).shouldBeNull()
+//                "lbGroup=".match(lbGroupTag).shouldBeNull()
+                "".match(lbGroupTag).shouldBeNull()
             }
         }
     }
@@ -45,6 +45,7 @@ class ObjectTagsKtTest : BehaviorSpec({
         }
         `when`("the label is blank") {
             then("null is returned") {
+                // Will now throw:
                 healthCheckTag(Pair("", 7)) shouldBe null
             }
         }
@@ -54,33 +55,28 @@ class ObjectTagsKtTest : BehaviorSpec({
                 healthCheckTag(Pair("failing", -1)) shouldBe null
             }
         }
-        `when`("the factory data is null") {
-            then("null is returned") {
-                healthCheckTag(null) shouldBe null
-            }
-        }
     }
 
     given("a healthCheck tag decoding method") {
         `when`("a valid tag is decoded") {
             then("decoded data is returned") {
-                healthCheckTagValue("healthCheck=on;probesOK:1") shouldBe Pair("probesOK", 1)
-                healthCheckTagValue("healthCheck=on;probesNOK:2") shouldBe Pair("probesNOK", 2)
-                healthCheckTagValue("healthCheck=on") shouldBe Pair("on", 0)
+                "healthCheck=on;probesOK:1".match(healthCheckTag) shouldBe Pair("probesOK", 1)
+                "healthCheck=on;probesNOK:2".match(healthCheckTag) shouldBe Pair("probesNOK", 2)
+                "healthCheck=on".match(healthCheckTag) shouldBe Pair("on", 0)
             }
         }
         `when`("an invalid tag is decoded") {
             then("null is returned") {
-                healthCheckTagValue("healthCheck=on;probesOK:-1") shouldBe null
-                healthCheckTagValue("healthCheck=") shouldBe null
-                healthCheckTagValue("healthCheck=on;probesOK") shouldBe null
-                healthCheckTagValue("healthCheck=on;probesOK:") shouldBe null
-                healthCheckTagValue("healthCheck=:1") shouldBe null
-                healthCheckTagValue("healthCheck") shouldBe null
-                healthCheckTagValue("healthCheckXX=on;probesOK:0") shouldBe null
-                healthCheckTagValue("XXhealthCheck=on;probesOK:0") shouldBe null
-                healthCheckTagValue("healthCheck=on;probesOK:0X") shouldBe null
-                healthCheckTagValue("") shouldBe null
+                "healthCheck=on;probesOK:-1".match(healthCheckTag) shouldBe null
+                "healthCheck=".match(healthCheckTag) shouldBe null
+                "healthCheck=on;probesOK".match(healthCheckTag) shouldBe null
+                "healthCheck=on;probesOK:".match(healthCheckTag) shouldBe null
+                "healthCheck=:1".match(healthCheckTag) shouldBe null
+                "healthCheck".match(healthCheckTag) shouldBe null
+                "healthCheckXX=probesOK:0".match(healthCheckTag) shouldBe null
+                "XXhealthCheck=probesOK:0".match(healthCheckTag) shouldBe null
+                "healthCheck=probesOK:0X".match(healthCheckTag) shouldBe null
+                "".match(healthCheckTag) shouldBe null
             }
         }
     }
diff --git a/components/proxy/src/test/kotlin/com/hotels/styx/services/OriginsAdminHandlerTest.kt b/components/proxy/src/test/kotlin/com/hotels/styx/services/OriginsAdminHandlerTest.kt
index 8b88709d78..d5c521dff7 100644
--- a/components/proxy/src/test/kotlin/com/hotels/styx/services/OriginsAdminHandlerTest.kt
+++ b/components/proxy/src/test/kotlin/com/hotels/styx/services/OriginsAdminHandlerTest.kt
@@ -129,7 +129,7 @@ class OriginsAdminHandlerTest : FeatureSpec({
             store.insert("app.origin", RoutingObjectRecord("HostProxy",
                     setOf(sourceTag("testProvider"),
                             stateTag(initialState),
-                            initialHealthTag), mockk(), mockObject))
+                            healthCheckTag(Pair(HEALTHCHECK_FAILING, 2))!!), mockk(), mockObject))
 
             val request = HttpRequest.put("http://host:7777/base/path/app/origin/state")
                     .body(mapper.writeValueAsString(requestedState), UTF_8)
@@ -141,9 +141,9 @@ class OriginsAdminHandlerTest : FeatureSpec({
             val tags = store.get("app.origin").get().tags
             tags shouldContain stateTag(expectedState)
             if (expectHealthTagCleared) {
-                healthCheckTag(tags) shouldBe null
+                healthCheckTag.find(tags) shouldBe null
             } else {
-                healthCheckTag(tags) shouldBe initialHealthTag
+                healthCheckTag.find(tags) shouldBe Pair(HEALTHCHECK_FAILING, 2)
             }
         }
 

From 79190810d162b6244988f87c0083d1bfbf11d1e7 Mon Sep 17 00:00:00 2001
From: Mikko Karjalainen <mikko.e.karjalainen@gmail.com>
Date: Mon, 2 Dec 2019 12:27:30 +0000
Subject: [PATCH 2/5] Tests pass.

---
 .../service/spi/AbstractStyxService.java      |  5 +-
 .../main/kotlin/com/hotels/styx/CommonTags.kt |  4 ++
 .../services/HealthCheckMonitoringService.kt  | 42 +++++++++++----
 .../services/YamlFileConfigurationService.kt  | 11 ++++
 .../com/hotels/styx/CommonValueTagTest.kt     | 53 +++++++++++++++++++
 5 files changed, 102 insertions(+), 13 deletions(-)
 create mode 100644 components/proxy/src/test/kotlin/com/hotels/styx/CommonValueTagTest.kt

diff --git a/components/api/src/main/java/com/hotels/styx/api/extension/service/spi/AbstractStyxService.java b/components/api/src/main/java/com/hotels/styx/api/extension/service/spi/AbstractStyxService.java
index ea45fe3365..a88b037e8b 100644
--- a/components/api/src/main/java/com/hotels/styx/api/extension/service/spi/AbstractStyxService.java
+++ b/components/api/src/main/java/com/hotels/styx/api/extension/service/spi/AbstractStyxService.java
@@ -1,5 +1,5 @@
 /*
-  Copyright (C) 2013-2018 Expedia Inc.
+  Copyright (C) 2013-2019 Expedia Inc.
 
   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
@@ -29,6 +29,7 @@
 import static com.hotels.styx.api.HttpResponse.response;
 import static com.hotels.styx.api.HttpResponseStatus.OK;
 import static com.hotels.styx.api.extension.service.spi.StyxServiceStatus.CREATED;
+import static com.hotels.styx.api.extension.service.spi.StyxServiceStatus.FAILED;
 import static com.hotels.styx.api.extension.service.spi.StyxServiceStatus.RUNNING;
 import static com.hotels.styx.api.extension.service.spi.StyxServiceStatus.STARTING;
 import static com.hotels.styx.api.extension.service.spi.StyxServiceStatus.STOPPED;
@@ -91,7 +92,7 @@ public CompletableFuture<Void> stop() {
 
     private Function<Throwable, Void> failWithMessage(String message) {
         return cause -> {
-            status.set(StyxServiceStatus.FAILED);
+            status.set(FAILED);
             throw new ServiceFailureException(message, cause);
         };
     }
diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt b/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt
index 9ec7ef280a..2f670ed17b 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt
@@ -38,6 +38,10 @@ sealed class CommonValueTag<T>(
     fun find(tags: Set<String>) = tags.firstOrNull { this.match(it) }
             ?.let { valuePart(it) }
             ?.let { this.decode(it) }
+
+    fun remove(tags: Set<String>) = tags
+            .filterNot { this.match(it) }
+            .toSet()
 }
 
 
diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/services/HealthCheckMonitoringService.kt b/components/proxy/src/main/kotlin/com/hotels/styx/services/HealthCheckMonitoringService.kt
index 44690d1720..84f3f31adc 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/services/HealthCheckMonitoringService.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/services/HealthCheckMonitoringService.kt
@@ -68,7 +68,7 @@ internal class HealthCheckMonitoringService(
 
         internal val EXECUTOR = ScheduledThreadPoolExecutor(2)
 
-        private val LOGGER = LoggerFactory.getLogger(HealthCheckMonitoringService::class.java)
+        internal val LOGGER = LoggerFactory.getLogger(HealthCheckMonitoringService::class.java)
     }
 
     private val probe = urlProbe(HttpRequest.get(urlPath).build(), Duration.ofMillis(1000))
@@ -76,7 +76,7 @@ internal class HealthCheckMonitoringService(
     private val futureRef: AtomicReference<ScheduledFuture<*>> = AtomicReference()
 
     override fun startService() = CompletableFuture.runAsync {
-        LOGGER.info("started - {} - {}", period.toMillis(), period.toMillis())
+        LOGGER.info("started service for {} - {} - {}", arrayOf(application, period.toMillis(), period.toMillis()))
         futureRef.set(executor.scheduleAtFixedRate(
                 { runChecks(application, objectStore) },
                 period.toMillis(),
@@ -85,12 +85,32 @@ internal class HealthCheckMonitoringService(
     }
 
     override fun stopService() = CompletableFuture.runAsync {
-        LOGGER.info("stopped")
+        LOGGER.info("stopped service for {}", application)
 
         objectStore.entrySet()
                 .filter(::containsRelevantStateTag)
-                .forEach { (name, _) ->
-                    markObject(objectStore, name, ObjectActive(0, false))
+                .forEach { (name, record) ->
+                    objectStore.get(name).ifPresent {
+                        try {
+                            objectStore.compute(name) { previous ->
+                                if (previous == null) throw ObjectDisappearedException()
+
+                                val newTags = record.tags
+                                        .let { healthCheckTag.remove(it) }
+                                        .let { stateTag.remove(it) }
+                                        .plus(stateTag(STATE_ACTIVE))
+
+                                if (previous.tags != newTags)
+                                    it.copy(tags = newTags)
+                                else
+                                    previous
+                            }
+                        } catch (e: ObjectDisappearedException) {
+                            // Object disappeared between the ifPresent check and the compute, but we don't really mind.
+                            // We just want to exit the compute, to avoid re-creating it.
+                            // (The ifPresent is not strictly required, but a pre-emptive check is preferred to an exception)
+                        }
+                    }
                 }
 
         futureRef.get().cancel(false)
@@ -203,12 +223,12 @@ private fun markObject(db: StyxObjectStore<RoutingObjectRecord>, name: String, n
 }
 
 internal fun reTag(tags: Set<String>, newStatus: ObjectHealth) =
-    tags.asSequence()
-            .filterNot { it.isA(stateTag) || it.isA(healthCheckTag) }
-            .plus(stateTag(newStatus.state()))
-            .plus(healthCheckTag(newStatus.health()!!))
-            .filterNotNull()
-            .toSet()
+        tags.asSequence()
+                .filterNot { it.isA(stateTag) || it.isA(healthCheckTag) }
+                .plus(stateTag(newStatus.state()))
+                .plus(healthCheckTag(newStatus.health()!!))
+                .filterNotNull()
+                .toSet()
 
 private val RELEVANT_STATES = setOf(STATE_ACTIVE, STATE_UNREACHABLE)
 private fun containsRelevantStateTag(entry: Map.Entry<String, RoutingObjectRecord>) =
diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/services/YamlFileConfigurationService.kt b/components/proxy/src/main/kotlin/com/hotels/styx/services/YamlFileConfigurationService.kt
index e612665529..11dcfd7cf0 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/services/YamlFileConfigurationService.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/services/YamlFileConfigurationService.kt
@@ -38,6 +38,8 @@ import com.hotels.styx.services.OriginsConfigConverter.Companion.deserialiseOrig
 import com.hotels.styx.sourceTag
 import com.hotels.styx.valueOf
 import org.slf4j.LoggerFactory
+import java.io.PrintWriter
+import java.io.StringWriter
 import java.lang.RuntimeException
 import java.nio.charset.StandardCharsets.UTF_8
 import java.time.Duration
@@ -185,6 +187,15 @@ internal class YamlFileConfigurationService(
                 if (previous == null || changed(new.config, previous.config)) {
                     new.styxService.start()
                     previous?.styxService?.stop()
+                            ?.whenComplete({ void, throwable ->
+                                if (throwable != null) {
+                                    val stack = StringWriter().let {
+                                        throwable.printStackTrace(PrintWriter(it))
+                                        it.toString()
+                                    }
+                                    LOGGER.warn("Service failed to terminate cleanly. cause=$throwable stack=$stack")
+                                }
+                            })
                     new
                 } else {
                     // No need to shout down the new one. It has yet been started.
diff --git a/components/proxy/src/test/kotlin/com/hotels/styx/CommonValueTagTest.kt b/components/proxy/src/test/kotlin/com/hotels/styx/CommonValueTagTest.kt
new file mode 100644
index 0000000000..f80c12b3a0
--- /dev/null
+++ b/components/proxy/src/test/kotlin/com/hotels/styx/CommonValueTagTest.kt
@@ -0,0 +1,53 @@
+/*
+  Copyright (C) 2013-2019 Expedia Inc.
+
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+  http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+ */
+package com.hotels.styx
+
+import io.kotlintest.matchers.collections.shouldBeEmpty
+import io.kotlintest.shouldBe
+import io.kotlintest.specs.FunSpec
+
+class CommonValueTagTest : FunSpec({
+
+    val intTag = NullableValueTag(
+            "intTag",
+            { value -> value.toString() },
+            { tagValue ->
+                kotlin.runCatching { tagValue.toInt() }
+                        .getOrNull()
+            })
+
+
+    context("remove() method") {
+        test("Removes a tag") {
+            intTag.remove(setOf("intTag=5", "abc=6", "def=7")).shouldBe(setOf("abc=6", "def=7"))
+            intTag.remove(setOf("intTag=5")).shouldBeEmpty()
+        }
+
+        test("Removes all matching tags") {
+            intTag.remove(setOf("intTag=5", "abc=6", "intTag=7", "def=7")).shouldBe(setOf("abc=6", "def=7"))
+            intTag.remove(setOf("intTag=5", "intTag=7")).shouldBeEmpty()
+        }
+
+        test("Does nothing when tag is not present") {
+            intTag.remove(setOf("foo=5", "abc=6", "bar=7", "def=7")).shouldBe(setOf("foo=5", "abc=6", "bar=7", "def=7"))
+        }
+
+        test("Does nothing when empty map is passed in") {
+            intTag.remove(setOf()).shouldBeEmpty()
+        }
+    }
+
+})
\ No newline at end of file

From 4e1e0ea011044b178c84aa64425530cc8d9b5698 Mon Sep 17 00:00:00 2001
From: Mikko Karjalainen <mikko.e.karjalainen@gmail.com>
Date: Mon, 2 Dec 2019 16:04:53 +0000
Subject: [PATCH 3/5] Add Kotlindocs for CommonTags.kt.

---
 .../main/kotlin/com/hotels/styx/CommonTags.kt | 72 +++++++++++++++----
 .../services/HealthCheckMonitoringService.kt  |  6 +-
 .../styx/services/OriginsAdminHandler.kt      |  8 +--
 .../styx/services/OriginsPageRenderer.kt      |  1 -
 .../services/YamlFileConfigurationService.kt  |  7 +-
 .../com/hotels/styx/ObjectTagsKtTest.kt       | 34 ++++-----
 6 files changed, 84 insertions(+), 44 deletions(-)

diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt b/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt
index 2f670ed17b..9a341e5f13 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt
@@ -15,36 +15,85 @@
  */
 package com.hotels.styx
 
-/*
- * Common Tag Operations:
+/**
+ * A common tag representation.
+ *
+ * Enforces a common tag format and provides a set of higher order functionality
+ * that operate on provided `name`, `encode` and `decode` properties.
+ *
+ * A tag string is a name-value pair separated by equal ('=') sign:
+ *
+ *     tag-string = name "=" value-part
+ *
+ * `encode` and `decode` are functions to encode a T to a value-part string,
+ * and to decode the value-part string back to T.
+ *
+ * @property name a tag name
+ * @property encode a function that encodes a value of T as a tag value string
+ * @property decode a function that decodes a tag value string as a T
  */
-
 sealed class CommonValueTag<T>(
         val name: String,
         val encode: (T) -> String?,
         val decode: (String) -> T?) {
 
+    /**
+     * Extracts value part (the right hand side) from a tag string.
+     *
+     * @param tag a tag string
+     * @return the tag value
+     */
     fun valuePart(tag: String) = if (tag.startsWith("$name=")) {
         tag.removePrefix("$name=")
     } else {
         null
     }
 
+    /**
+     * Tests if a given string matches this tag. A match is positive when the string
+     * starts with `name` followed by `=`.
+     *
+     * @param a tag string
+     * @return True if this string is possibly a matching tag. Otherwise return false.
+     */
     fun match(tag: String) = tag.startsWith("$name=")
 
+    /**
+     * Decodes given tag string to its typed value.
+     *
+     * @param tag a tag string
+     * @return a decoded tag value, or null if decoding failed
+     */
     fun valueOf(tag: String) = valuePart(tag)
             ?.let { decode(it) }
 
+    /**
+     * Find this tag from a set of tag strings. If found, decode the tag value. Return null if
+     * tag was not found, or if decoding failed.
+     *
+     * @param tags a set of tag strings
+     * @return A decoded value if tag was found. Otherwise return null.
+     */
     fun find(tags: Set<String>) = tags.firstOrNull { this.match(it) }
             ?.let { valuePart(it) }
             ?.let { this.decode(it) }
 
+    /**
+     * Removes all instances of this tag from a set of tag strings.
+     * Return a new Set<String> with all instances of this tag removed.
+     *
+     * @param tags a set of tag strings
+     * @return A new set of strings without this tag.
+     */
     fun remove(tags: Set<String>) = tags
             .filterNot { this.match(it) }
             .toSet()
 }
 
-
+/**
+ * A NullableValueTag invoke method returns null when value cannot be encoded to string.
+ * The API consumer must handle this situation.
+ */
 class NullableValueTag<T>(
         name: String,
         encode: (T) -> String?,
@@ -56,6 +105,10 @@ class NullableValueTag<T>(
             }
 }
 
+/**
+ * A SafeValueTag invoke method throws a KotlinNullPointerException when the
+ * tag value cannot be encoded to string.
+ */
 class SafeValueTag<T>(
         name: String,
         encode: (T) -> String?,
@@ -68,16 +121,5 @@ class SafeValueTag<T>(
             }
 }
 
-
-//fun Set<String>.contains(tag: CommonValueTag<*>) = this.firstOrNull { tag.match(it) } != null
-
-// TODO: Compare to ObjectTag.find()
-fun <T> Set<String>.valueOf(tag: CommonValueTag<T>): T? = this.firstOrNull { tag.match(it) }
-        ?.let { tag.valuePart(it) }
-        ?.let { tag.decode(it) }
-
-// TODO: Compare to ObjectTag.valueOf():
 fun <T> String.match(tag: CommonValueTag<T>) = tag.valuePart(this)
         ?.let { tag.decode(it) }
-
-fun String.isA(tag: CommonValueTag<*>) = tag.match(this)
diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/services/HealthCheckMonitoringService.kt b/components/proxy/src/main/kotlin/com/hotels/styx/services/HealthCheckMonitoringService.kt
index 84f3f31adc..4b880f7afa 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/services/HealthCheckMonitoringService.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/services/HealthCheckMonitoringService.kt
@@ -122,7 +122,7 @@ internal class HealthCheckMonitoringService(
                 .filter { (_, record) -> record.tags.contains(lbGroupTag(application)) }
                 .map { (name, record) ->
                     val tags = record.tags
-                    val objectHealth = objectHealthFrom(stateTag.find(tags), tags.valueOf(healthCheckTag))
+                    val objectHealth = objectHealthFrom(stateTag.find(tags), healthCheckTag.find(tags))
                     Triple(name, record, objectHealth)
                 }
 
@@ -224,7 +224,7 @@ private fun markObject(db: StyxObjectStore<RoutingObjectRecord>, name: String, n
 
 internal fun reTag(tags: Set<String>, newStatus: ObjectHealth) =
         tags.asSequence()
-                .filterNot { it.isA(stateTag) || it.isA(healthCheckTag) }
+                .filterNot { stateTag.match(it) || healthCheckTag.match(it) }
                 .plus(stateTag(newStatus.state()))
                 .plus(healthCheckTag(newStatus.health()!!))
                 .filterNotNull()
@@ -232,5 +232,5 @@ internal fun reTag(tags: Set<String>, newStatus: ObjectHealth) =
 
 private val RELEVANT_STATES = setOf(STATE_ACTIVE, STATE_UNREACHABLE)
 private fun containsRelevantStateTag(entry: Map.Entry<String, RoutingObjectRecord>) =
-        entry.value.tags.valueOf(stateTag) in RELEVANT_STATES
+        stateTag.find(entry.value.tags) in RELEVANT_STATES
 
diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsAdminHandler.kt b/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsAdminHandler.kt
index 8437e4bcfc..5bd5cdeb7d 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsAdminHandler.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsAdminHandler.kt
@@ -63,7 +63,7 @@ internal class OriginsAdminHandler(
     private fun getState(objectId: String) : HttpResponse {
         val origin = findOrigin(objectId)
         return if (origin != null) {
-            textValueResponse(origin.tags.valueOf(stateTag) ?: "")
+            textValueResponse(stateTag.find(origin.tags) ?: "")
         } else {
             errorResponse(NOT_FOUND, "No origin found for ID $objectId")
         }
@@ -92,7 +92,7 @@ internal class OriginsAdminHandler(
             if (!isValidOrigin(origin)) {
                 throw HttpStatusException(NOT_FOUND, "No origin found for ID $objectId")
             }
-            newState = when(origin!!.tags.valueOf(stateTag)) {
+            newState = when(stateTag.find(origin!!.tags)) {
                 STATE_INACTIVE, STATE_ACTIVE -> STATE_ACTIVE
                 STATE_UNREACHABLE -> STATE_UNREACHABLE
                 else -> STATE_ACTIVE
@@ -119,8 +119,8 @@ internal class OriginsAdminHandler(
     private fun updateStateTag(origin: RoutingObjectRecord, newValue: String, clearHealthcheck: Boolean = false) : RoutingObjectRecord {
         val oldTags = origin.tags
         val newTags = oldTags
-                .filterNot{ clearHealthcheck && it.isA(healthCheckTag) }
-                .filterNot{ it.isA(stateTag) }
+                .filterNot{ clearHealthcheck && healthCheckTag.match(it) }
+                .filterNot{ stateTag.match(it) }
                 .plus(stateTag(newValue))
                 .toSet()
         return if (oldTags != newTags) {
diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsPageRenderer.kt b/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsPageRenderer.kt
index 8b4786a6d3..39403454d5 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsPageRenderer.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsPageRenderer.kt
@@ -18,7 +18,6 @@ package com.hotels.styx.services
 import com.fasterxml.jackson.databind.JsonNode
 import com.hotels.styx.infrastructure.configuration.yaml.JsonNodeConfig
 import com.hotels.styx.lbGroupTag
-import com.hotels.styx.match
 import com.hotels.styx.routing.RoutingObjectRecord
 import com.hotels.styx.routing.config.RoutingConfigParser.toRoutingConfigNode
 import com.hotels.styx.routing.config.StyxObjectDefinition
diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/services/YamlFileConfigurationService.kt b/components/proxy/src/main/kotlin/com/hotels/styx/services/YamlFileConfigurationService.kt
index 11dcfd7cf0..72215adf3f 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/services/YamlFileConfigurationService.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/services/YamlFileConfigurationService.kt
@@ -36,7 +36,6 @@ import com.hotels.styx.server.handlers.ClassPathResourceHandler
 import com.hotels.styx.serviceproviders.ServiceProviderFactory
 import com.hotels.styx.services.OriginsConfigConverter.Companion.deserialiseOrigins
 import com.hotels.styx.sourceTag
-import com.hotels.styx.valueOf
 import org.slf4j.LoggerFactory
 import java.io.PrintWriter
 import java.io.StringWriter
@@ -63,7 +62,7 @@ internal class YamlFileConfigurationService(
 
     private val ingressObjectName = if (config.ingressObject.isNotBlank()) config.ingressObject else "$name-router"
 
-    private val objectSourceTag = sourceTag(name)!!
+    private val objectSourceTag = sourceTag(name)
 
     private val fileMonitoringService = FileMonitoringService("YamlFileCoinfigurationService", config.originsFile, pollInterval) {
         reloadAction(it)
@@ -118,7 +117,7 @@ internal class YamlFileConfigurationService(
 
             routingObjectDefs.forEach { objectDef ->
                 routeDb.get(objectDef.name()).ifPresent {
-                    if (it.tags.valueOf(sourceTag) != name) {
+                    if (sourceTag.find(it.tags) != name) {
                         throw DuplicateObjectException("Object name='${objectDef.name()}' already exists. Provider='${name}', file='${config.originsFile}'.")
                     }
                 }
@@ -129,7 +128,7 @@ internal class YamlFileConfigurationService(
 
             healthMonitors.forEach { (objectName, _) ->
                 serviceDb.get(objectName).ifPresent {
-                    if (it.tags.valueOf(sourceTag) != name) {
+                    if (sourceTag.find(it.tags) != name) {
                         throw DuplicateObjectException("Health Monitor name='${objectName}' already exists. Provider='${name}', file='${config.originsFile}'.")
                     }
                 }
diff --git a/components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsKtTest.kt b/components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsKtTest.kt
index e5b1afc2d1..a6b247fb3f 100644
--- a/components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsKtTest.kt
+++ b/components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsKtTest.kt
@@ -23,14 +23,14 @@ class ObjectTagsKtTest : BehaviorSpec({
     given("An lbGroup tag matcher") {
         `when`("tag matches") {
             then("returns tag value") {
-                "lbGroup=abc".match(lbGroupTag).shouldBe("abc")
+                lbGroupTag.valueOf("lbGroup=abc").shouldBe("abc")
+                lbGroupTag.valueOf("lbGroup=").shouldBe("")
             }
         }
         `when`("tag doesn't match") {
             then("returns null") {
-                "abc".match(lbGroupTag).shouldBeNull()
-//                "lbGroup=".match(lbGroupTag).shouldBeNull()
-                "".match(lbGroupTag).shouldBeNull()
+                lbGroupTag.valueOf("abc").shouldBeNull()
+                lbGroupTag.valueOf("").shouldBeNull()
             }
         }
     }
@@ -60,23 +60,23 @@ class ObjectTagsKtTest : BehaviorSpec({
     given("a healthCheck tag decoding method") {
         `when`("a valid tag is decoded") {
             then("decoded data is returned") {
-                "healthCheck=on;probesOK:1".match(healthCheckTag) shouldBe Pair("probesOK", 1)
-                "healthCheck=on;probesNOK:2".match(healthCheckTag) shouldBe Pair("probesNOK", 2)
-                "healthCheck=on".match(healthCheckTag) shouldBe Pair("on", 0)
+                healthCheckTag.valueOf("healthCheck=on;probesOK:1") shouldBe Pair("probesOK", 1)
+                healthCheckTag.valueOf("healthCheck=on;probesNOK:2") shouldBe Pair("probesNOK", 2)
+                healthCheckTag.valueOf("healthCheck=on") shouldBe Pair("on", 0)
             }
         }
         `when`("an invalid tag is decoded") {
             then("null is returned") {
-                "healthCheck=on;probesOK:-1".match(healthCheckTag) shouldBe null
-                "healthCheck=".match(healthCheckTag) shouldBe null
-                "healthCheck=on;probesOK".match(healthCheckTag) shouldBe null
-                "healthCheck=on;probesOK:".match(healthCheckTag) shouldBe null
-                "healthCheck=:1".match(healthCheckTag) shouldBe null
-                "healthCheck".match(healthCheckTag) shouldBe null
-                "healthCheckXX=probesOK:0".match(healthCheckTag) shouldBe null
-                "XXhealthCheck=probesOK:0".match(healthCheckTag) shouldBe null
-                "healthCheck=probesOK:0X".match(healthCheckTag) shouldBe null
-                "".match(healthCheckTag) shouldBe null
+                healthCheckTag.valueOf("healthCheck=on;probesOK:-1") shouldBe null
+                healthCheckTag.valueOf("healthCheck=") shouldBe null
+                healthCheckTag.valueOf("healthCheck=on;probesOK") shouldBe null
+                healthCheckTag.valueOf("healthCheck=on;probesOK:") shouldBe null
+                healthCheckTag.valueOf("healthCheck=:1") shouldBe null
+                healthCheckTag.valueOf("healthCheck") shouldBe null
+                healthCheckTag.valueOf("healthCheckXX=probesOK:0") shouldBe null
+                healthCheckTag.valueOf("XXhealthCheck=probesOK:0") shouldBe null
+                healthCheckTag.valueOf("healthCheck=probesOK:0X") shouldBe null
+                healthCheckTag.valueOf("") shouldBe null
             }
         }
     }

From cdfee8849ea449ead519b2dff832164d4d42cf8c Mon Sep 17 00:00:00 2001
From: Mikko Karjalainen <mikko.e.karjalainen@gmail.com>
Date: Tue, 3 Dec 2019 09:04:34 +0000
Subject: [PATCH 4/5] Improve unit tests. Rename few files.

---
 .../main/kotlin/com/hotels/styx/CommonTags.kt |  3 --
 ...ommonValueTagTest.kt => CommonTagsTest.kt} | 38 ++++++++++++++++++-
 ...{ObjectTagsKtTest.kt => ObjectTagsTest.kt} |  2 +-
 3 files changed, 38 insertions(+), 5 deletions(-)
 rename components/proxy/src/test/kotlin/com/hotels/styx/{CommonValueTagTest.kt => CommonTagsTest.kt} (55%)
 rename components/proxy/src/test/kotlin/com/hotels/styx/{ObjectTagsKtTest.kt => ObjectTagsTest.kt} (98%)

diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt b/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt
index 9a341e5f13..7eadd69d3e 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt
@@ -120,6 +120,3 @@ class SafeValueTag<T>(
                 "$name=$it"
             }
 }
-
-fun <T> String.match(tag: CommonValueTag<T>) = tag.valuePart(this)
-        ?.let { tag.decode(it) }
diff --git a/components/proxy/src/test/kotlin/com/hotels/styx/CommonValueTagTest.kt b/components/proxy/src/test/kotlin/com/hotels/styx/CommonTagsTest.kt
similarity index 55%
rename from components/proxy/src/test/kotlin/com/hotels/styx/CommonValueTagTest.kt
rename to components/proxy/src/test/kotlin/com/hotels/styx/CommonTagsTest.kt
index f80c12b3a0..f7a6f34b6a 100644
--- a/components/proxy/src/test/kotlin/com/hotels/styx/CommonValueTagTest.kt
+++ b/components/proxy/src/test/kotlin/com/hotels/styx/CommonTagsTest.kt
@@ -16,10 +16,11 @@
 package com.hotels.styx
 
 import io.kotlintest.matchers.collections.shouldBeEmpty
+import io.kotlintest.matchers.types.shouldBeNull
 import io.kotlintest.shouldBe
 import io.kotlintest.specs.FunSpec
 
-class CommonValueTagTest : FunSpec({
+class CommonTagsTest : FunSpec({
 
     val intTag = NullableValueTag(
             "intTag",
@@ -29,6 +30,41 @@ class CommonValueTagTest : FunSpec({
                         .getOrNull()
             })
 
+    context("invoke() method") {
+        test("Creates a new tag string from a valid input") {
+            intTag(5) shouldBe "intTag=5"
+            intTag(-55) shouldBe "intTag=-55"
+        }
+    }
+
+    context("valueOf() method") {
+        test("Decodes a value from a valid tag string") {
+            intTag.valueOf("intTag=99") shouldBe 99
+            intTag.valueOf("intTag=-99") shouldBe -99
+        }
+
+        test("Returns null for non-conforming tag strings") {
+            intTag.valueOf("").shouldBeNull()
+            intTag.valueOf("intT").shouldBeNull()
+            intTag.valueOf("intTag").shouldBeNull()
+            intTag.valueOf("intTag=").shouldBeNull()
+            intTag.valueOf("intTag=abc").shouldBeNull()
+            intTag.valueOf("=abc").shouldBeNull()
+            intTag.valueOf("=").shouldBeNull()
+        }
+    }
+
+    context("find() method") {
+        test("Returns the first found tag value") {
+            intTag.find(setOf("intTag=99")) shouldBe 99
+            intTag.find(setOf("abc", "blah=", "foo=bar", "intTag=99", "otherTag=")) shouldBe 99
+        }
+
+        test("Returns null when it encounters non-conforming tag") {
+            // Returns null despite a correctly formatted tag is present
+            intTag.find(setOf("intTag=", "intTag=99")).shouldBeNull()
+        }
+    }
 
     context("remove() method") {
         test("Removes a tag") {
diff --git a/components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsKtTest.kt b/components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsTest.kt
similarity index 98%
rename from components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsKtTest.kt
rename to components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsTest.kt
index a6b247fb3f..d0abbe715a 100644
--- a/components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsKtTest.kt
+++ b/components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsTest.kt
@@ -19,7 +19,7 @@ import io.kotlintest.matchers.types.shouldBeNull
 import io.kotlintest.shouldBe
 import io.kotlintest.specs.BehaviorSpec
 
-class ObjectTagsKtTest : BehaviorSpec({
+class ObjectTagsTest : BehaviorSpec({
     given("An lbGroup tag matcher") {
         `when`("tag matches") {
             then("returns tag value") {

From dcfd28cb0998226c689760efb58b99e4b3f5de30 Mon Sep 17 00:00:00 2001
From: Mikko Karjalainen <mikko.e.karjalainen@gmail.com>
Date: Tue, 3 Dec 2019 19:04:33 +0000
Subject: [PATCH 5/5] Address code review comments.

---
 components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt | 3 +--
 components/proxy/src/main/kotlin/com/hotels/styx/ObjectTags.kt | 1 -
 .../com/hotels/styx/services/HealthCheckMonitoringService.kt   | 2 +-
 .../kotlin/com/hotels/styx/services/OriginsConfigConverter.kt  | 2 +-
 .../proxy/src/test/kotlin/com/hotels/styx/ObjectTagsTest.kt    | 1 -
 5 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt b/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt
index 7eadd69d3e..fa19194a4e 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/CommonTags.kt
@@ -75,8 +75,7 @@ sealed class CommonValueTag<T>(
      * @return A decoded value if tag was found. Otherwise return null.
      */
     fun find(tags: Set<String>) = tags.firstOrNull { this.match(it) }
-            ?.let { valuePart(it) }
-            ?.let { this.decode(it) }
+            ?.let { valueOf(it) }
 
     /**
      * Removes all instances of this tag from a set of tag strings.
diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/ObjectTags.kt b/components/proxy/src/main/kotlin/com/hotels/styx/ObjectTags.kt
index e67319c2f3..717d62c750 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/ObjectTags.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/ObjectTags.kt
@@ -50,7 +50,6 @@ val stateTag = SafeValueTag(
  * healthCheck=on;probes-OK:2
  * healthCheck=on;probes-FAIL:1
  */
-private const val HEALTHCHECK = "healthCheck"
 const val HEALTHCHECK_PASSING = "probes-OK"
 const val HEALTHCHECK_FAILING = "probes-FAIL"
 const val HEALTHCHECK_ON = "on"
diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/services/HealthCheckMonitoringService.kt b/components/proxy/src/main/kotlin/com/hotels/styx/services/HealthCheckMonitoringService.kt
index 4b880f7afa..00dc37cf49 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/services/HealthCheckMonitoringService.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/services/HealthCheckMonitoringService.kt
@@ -95,7 +95,7 @@ internal class HealthCheckMonitoringService(
                             objectStore.compute(name) { previous ->
                                 if (previous == null) throw ObjectDisappearedException()
 
-                                val newTags = record.tags
+                                val newTags = previous.tags
                                         .let { healthCheckTag.remove(it) }
                                         .let { stateTag.remove(it) }
                                         .plus(stateTag(STATE_ACTIVE))
diff --git a/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsConfigConverter.kt b/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsConfigConverter.kt
index 1fc4933259..15b6430137 100644
--- a/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsConfigConverter.kt
+++ b/components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsConfigConverter.kt
@@ -151,7 +151,7 @@ internal class OriginsConfigConverter(
         }
 
         internal fun hostProxy(app: BackendService, origin: Origin) : StyxObjectDefinition {
-            val healthCheckTag :String = if (isHealthCheckConfigured(app)) stateTag(STATE_UNREACHABLE)!! else stateTag(STATE_ACTIVE)!!
+            val healthCheckTag :String = if (isHealthCheckConfigured(app)) stateTag(STATE_UNREACHABLE) else stateTag(STATE_ACTIVE)
 
             return StyxObjectDefinition(
                 "${app.id()}.${origin.id()}",
diff --git a/components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsTest.kt b/components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsTest.kt
index d0abbe715a..ff5c69be2b 100644
--- a/components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsTest.kt
+++ b/components/proxy/src/test/kotlin/com/hotels/styx/ObjectTagsTest.kt
@@ -45,7 +45,6 @@ class ObjectTagsTest : BehaviorSpec({
         }
         `when`("the label is blank") {
             then("null is returned") {
-                // Will now throw:
                 healthCheckTag(Pair("", 7)) shouldBe null
             }
         }