Avoid creating transient objects when origin file is reloaded (#483)

* Also add object creation timestamp to the objects.

components/proxy/src/main/java/com/hotels/styx/startup/StyxServerComponents.java

@@ -35,8 +35,6 @@
 import com.hotels.styx.common.format.SanitisedHttpMessageFormatter;
 import com.hotels.styx.infrastructure.configuration.yaml.JsonNodeConfig;
 import com.hotels.styx.proxy.plugin.NamedPlugin;
-import com.hotels.styx.routing.RoutingMetadataDecorator;
-import com.hotels.styx.routing.RoutingObject;
 import com.hotels.styx.routing.RoutingObjectRecord;
 import com.hotels.styx.routing.config.Builtins;
 import com.hotels.styx.routing.config.RoutingObjectFactory;
@@ -130,11 +128,12 @@ private StyxServerComponents(Builder builder) {
                 .map(StyxServerComponents::readComponents)
                 .orElse(ImmutableMap.of())
                 .forEach((name, definition) -> {
-                    RoutingObject routingObject = Builtins.build(ImmutableList.of(name), routingObjectContext, definition);
-                    RoutingMetadataDecorator adapter = new RoutingMetadataDecorator(routingObject);
-
-                    routeObjectStore.insert(name, new RoutingObjectRecord(definition.type(), ImmutableSet.copyOf(definition.tags()), definition.config(), adapter))
-                            .ifPresent(previous -> previous.getRoutingObject().stop());
+                    routeObjectStore.insert(name, RoutingObjectRecord.Companion.create(
+                            definition.type(),
+                            ImmutableSet.copyOf(definition.tags()),
+                            definition.config(),
+                            Builtins.build(ImmutableList.of(name), routingObjectContext, definition))
+                    ).ifPresent(previous -> previous.getRoutingObject().stop());
                 });
 
         this.environment.configuration().get("providers", JsonNode.class)

components/proxy/src/main/kotlin/com/hotels/styx/routing/RoutingObjectRecord.kt

@@ -16,6 +16,8 @@
 package com.hotels.styx.routing
 
 import com.fasterxml.jackson.databind.JsonNode
+import java.time.LocalDateTime
+import java.time.format.DateTimeFormatter.ISO_DATE_TIME
 
 /**
  * A routing object and its associated configuration metadata.
@@ -28,8 +30,16 @@ internal data class RoutingObjectRecord(
     companion object {
         fun create(type: String, tags: Set<String>, config: JsonNode, routingObject: RoutingObject) = RoutingObjectRecord(
                 type,
-                tags,
+                tags + "created:${timestamp()}",
                 config,
                 RoutingMetadataDecorator(routingObject))
     }
+
+    fun creationTime() = tags
+            .filter { it.startsWith("created:") }
+            .first()
+}
+
+private fun timestamp(): String {
+    return LocalDateTime.now().format(ISO_DATE_TIME)
 }

components/proxy/src/main/kotlin/com/hotels/styx/services/OriginsConfigConverter.kt

@@ -56,14 +56,12 @@ internal class OriginsConfigConverter(
 
     internal fun routingObjects(apps: List<BackendService>) =
             routingObjectConfigs(apps)
-                    .map { styxObjectDef ->
-                        Pair(styxObjectDef.name(), RoutingObjectRecord.create(
-                                styxObjectDef.type(),
-                                styxObjectDef.tags().toSet(),
-                                styxObjectDef.config(),
-                                Builtins.build(listOf(styxObjectDef.name()), context, styxObjectDef)
-                        ))
-                    }
+
+    internal fun routingObjectRecord(objectDef: StyxObjectDefinition) = RoutingObjectRecord.create(
+            objectDef.type(),
+            objectDef.tags().toSet(),
+            objectDef.config(),
+            Builtins.build(listOf(objectDef.name()), context, objectDef))
 
     internal fun routingObjectConfigs(apps: List<BackendService>): List<StyxObjectDefinition> =
             apps.flatMap { toBackendServiceObjects(it, originRestrictionCookie) } + pathPrefixRouter(apps)

components/proxy/src/main/kotlin/com/hotels/styx/services/YamlFileConfigurationService.kt

@@ -25,9 +25,11 @@ import com.hotels.styx.config.schema.SchemaDsl.string
 import com.hotels.styx.infrastructure.configuration.yaml.JsonNodeConfig
 import com.hotels.styx.routing.RoutingObjectRecord
 import com.hotels.styx.routing.config.RoutingObjectFactory
+import com.hotels.styx.routing.config.StyxObjectDefinition
 import com.hotels.styx.routing.db.StyxObjectStore
 import com.hotels.styx.routing.handlers.ProviderObjectRecord
 import com.hotels.styx.serviceproviders.ServiceProviderFactory
+import com.hotels.styx.services.OriginsConfigConverter.Companion.OBJECT_CREATOR_TAG
 import com.hotels.styx.services.OriginsConfigConverter.Companion.deserialiseOrigins
 import org.slf4j.LoggerFactory
 import java.time.Duration
@@ -52,7 +54,6 @@ internal class YamlFileConfigurationService(
         reloadAction(it)
     }
 
-    private val routingObjects = AtomicReference<List<Pair<String, RoutingObjectRecord>>>(listOf())
     private val healthMonitors = AtomicReference<List<Pair<String, ProviderObjectRecord>>>(listOf())
 
     companion object {
@@ -83,12 +84,12 @@ internal class YamlFileConfigurationService(
         kotlin.runCatching {
             val deserialised = deserialiseOrigins(content)
 
-            val routingObjects = converter.routingObjects(deserialised)
+            val routingObjectDefs = converter.routingObjects(deserialised)
             val healthMonitors = converter.healthCheckServices(deserialised)
 
-            Pair(healthMonitors, routingObjects)
-        }.mapCatching { (healthMonitors, routingObjects) ->
-            updateRoutingObjects(routingObjects)
+            Pair(healthMonitors, routingObjectDefs)
+        }.mapCatching { (healthMonitors, routingObjectDefs) ->
+            updateRoutingObjects(routingObjectDefs)
             updateHealthCheckServices(serviceDb, healthMonitors)
             initialised.countDown()
         }.onFailure {
@@ -98,20 +99,20 @@ internal class YamlFileConfigurationService(
 
     private fun changed(one: JsonNode, another: JsonNode) = !one.equals(another)
 
-    internal fun updateRoutingObjects(objects: List<Pair<String, RoutingObjectRecord>>) {
-        val oldObjectNames = routingObjects.get().map { it.first }
-        routingObjects.set(objects)
+    internal fun updateRoutingObjects(objectDefs: List<StyxObjectDefinition>) {
+        val previousObjectNames = routeDb.entrySet()
+                .filter { it.value.tags.contains(OBJECT_CREATOR_TAG) }
+                .map { it.key }
 
-        val newObjectNames = objects.map { it.first }
-        val removedObjects = oldObjectNames.minus(newObjectNames)
+        val newObjectNames = objectDefs.map { it.name() }
+        val removedObjects = previousObjectNames.minus(newObjectNames)
 
-        objects.forEach { (name, new) ->
-            routeDb.compute(name) { previous ->
-                if (previous == null || changed(new.config, previous.config)) {
+        objectDefs.forEach { objectDef ->
+            routeDb.compute(objectDef.name()) { previous ->
+                if (previous == null || changed(objectDef.config(), previous.config)) {
                     previous?.routingObject?.stop()
-                    new
+                    converter.routingObjectRecord(objectDef)
                 } else {
-                    new.routingObject.stop()
                     previous
                 }
             }

components/proxy/src/test/kotlin/com/hotels/styx/routing/RoutingObjectRecordTest.kt

@@ -0,0 +1,31 @@
+/*
+  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.routing
+
+import io.kotlintest.matchers.string.shouldContain
+import io.kotlintest.specs.StringSpec
+import io.mockk.mockk
+
+class RoutingObjectRecordTest : StringSpec({
+    "Creates with timestamp" {
+        val createdTag= RoutingObjectRecord.create("A", setOf("b"), mockk(), mockk())
+                .tags
+                .filter { it.startsWith("created") }
+                .first()
+
+        createdTag.shouldContain("created:20[0-9]{2}-[0-9]{1,2}-[0-9]{1,2}T[0-9]{1,2}:[0-9]{1,2}:[0-9]{1,2}.[0-9]{3}".toRegex())
+    }
+})

components/proxy/src/test/kotlin/com/hotels/styx/services/OriginsConfigConverterTest.kt

@@ -48,25 +48,25 @@ class OriginsConfigConverterTest : StringSpec({
                 .let {
                     it.size shouldBe 4
 
-                    it[0].first shouldBe "app.app1"
-                    it[0].second.tags.shouldContainAll("app", "source=OriginsFileConverter")
-                    it[0].second.type.shouldBe("HostProxy")
-                    it[0].second.routingObject.shouldNotBeNull()
-
-                    it[1].first shouldBe "app.app2"
-                    it[1].second.tags.shouldContainAll("app", "source=OriginsFileConverter")
-                    it[1].second.type.shouldBe("HostProxy")
-                    it[1].second.routingObject.shouldNotBeNull()
-
-                    it[2].first shouldBe "app"
-                    it[2].second.tags.shouldContainAll("source=OriginsFileConverter")
-                    it[2].second.type.shouldBe("LoadBalancingGroup")
-                    it[2].second.routingObject.shouldNotBeNull()
-
-                    it[3].first shouldBe ROOT_OBJECT_NAME
-                    it[3].second.tags.shouldContainAll("source=OriginsFileConverter")
-                    it[3].second.type.shouldBe("PathPrefixRouter")
-                    it[3].second.routingObject.shouldNotBeNull()
+                    it[0].name() shouldBe "app.app1"
+                    it[0].tags().shouldContainAll("app", "source=OriginsFileConverter")
+                    it[0].type().shouldBe("HostProxy")
+                    it[0].config().shouldNotBeNull()
+
+                    it[1].name() shouldBe "app.app2"
+                    it[1].tags().shouldContainAll("app", "source=OriginsFileConverter")
+                    it[1].type().shouldBe("HostProxy")
+                    it[1].config().shouldNotBeNull()
+
+                    it[2].name() shouldBe "app"
+                    it[2].tags().shouldContainAll("source=OriginsFileConverter")
+                    it[2].type().shouldBe("LoadBalancingGroup")
+                    it[2].config().shouldNotBeNull()
+
+                    it[3].name() shouldBe ROOT_OBJECT_NAME
+                    it[3].tags().shouldContainAll("source=OriginsFileConverter")
+                    it[3].type().shouldBe("PathPrefixRouter")
+                    it[3].config().shouldNotBeNull()
                 }
     }
 
@@ -134,25 +134,25 @@ class OriginsConfigConverterTest : StringSpec({
                 .let {
                     it.size shouldBe 4
 
-                    it[0].first shouldBe "app.app1"
-                    it[0].second.tags.shouldContainAll("app", "source=OriginsFileConverter")
-                    it[0].second.type.shouldBe("HostProxy")
-                    it[0].second.routingObject.shouldNotBeNull()
-
-                    it[1].first shouldBe "app.app2"
-                    it[1].second.tags.shouldContainAll("app", "source=OriginsFileConverter")
-                    it[1].second.type.shouldBe("HostProxy")
-                    it[1].second.routingObject.shouldNotBeNull()
-
-                    it[2].first shouldBe "app"
-                    it[2].second.tags.shouldContainAll("source=OriginsFileConverter")
-                    it[2].second.type.shouldBe("LoadBalancingGroup")
-                    it[2].second.routingObject.shouldNotBeNull()
-
-                    it[3].first shouldBe ROOT_OBJECT_NAME
-                    it[3].second.tags.shouldContainAll("source=OriginsFileConverter")
-                    it[3].second.type.shouldBe("PathPrefixRouter")
-                    it[3].second.routingObject.shouldNotBeNull()
+                    it[0].name() shouldBe "app.app1"
+                    it[0].tags().shouldContainAll("app", "source=OriginsFileConverter")
+                    it[0].type().shouldBe("HostProxy")
+                    it[0].config().shouldNotBeNull()
+
+                    it[1].name() shouldBe "app.app2"
+                    it[1].tags().shouldContainAll("app", "source=OriginsFileConverter")
+                    it[1].type().shouldBe("HostProxy")
+                    it[1].config().shouldNotBeNull()
+
+                    it[2].name() shouldBe "app"
+                    it[2].tags().shouldContainAll("source=OriginsFileConverter")
+                    it[2].type().shouldBe("LoadBalancingGroup")
+                    it[2].config().shouldNotBeNull()
+
+                    it[3].name() shouldBe ROOT_OBJECT_NAME
+                    it[3].tags().shouldContainAll("source=OriginsFileConverter")
+                    it[3].type().shouldBe("PathPrefixRouter")
+                    it[3].config().shouldNotBeNull()
                 }
     }
 
@@ -180,50 +180,50 @@ class OriginsConfigConverterTest : StringSpec({
                 .let {
                     it.size shouldBe 9
 
-                    it[0].first shouldBe "appA.appA-1"
-                    it[0].second.tags.shouldContainAll("appA", "source=OriginsFileConverter")
-                    it[0].second.type.shouldBe("HostProxy")
-                    it[0].second.routingObject.shouldNotBeNull()
-
-                    it[1].first shouldBe "appA.appA-2"
-                    it[1].second.tags.shouldContainAll("appA", "source=OriginsFileConverter")
-                    it[1].second.type.shouldBe("HostProxy")
-                    it[1].second.routingObject.shouldNotBeNull()
-
-                    it[2].first shouldBe "appA"
-                    it[2].second.tags.shouldContainAll("source=OriginsFileConverter")
-                    it[2].second.type.shouldBe("LoadBalancingGroup")
-                    it[2].second.routingObject.shouldNotBeNull()
-
-                    it[3].first shouldBe "appB.appB-1"
-                    it[3].second.tags.shouldContainAll("appB", "source=OriginsFileConverter")
-                    it[3].second.type.shouldBe("HostProxy")
-                    it[3].second.routingObject.shouldNotBeNull()
-
-                    it[4].first shouldBe "appB"
-                    it[4].second.tags.shouldContainAll("source=OriginsFileConverter")
-                    it[4].second.type.shouldBe("LoadBalancingGroup")
-                    it[4].second.routingObject.shouldNotBeNull()
-
-                    it[5].first shouldBe "appC.appC-1"
-                    it[5].second.tags.shouldContainAll("appC", "source=OriginsFileConverter")
-                    it[5].second.type.shouldBe("HostProxy")
-                    it[5].second.routingObject.shouldNotBeNull()
-
-                    it[6].first shouldBe "appC.appC-2"
-                    it[6].second.tags.shouldContainAll("appC", "source=OriginsFileConverter")
-                    it[6].second.type.shouldBe("HostProxy")
-                    it[6].second.routingObject.shouldNotBeNull()
-
-                    it[7].first shouldBe "appC"
-                    it[7].second.tags.shouldContainAll("source=OriginsFileConverter")
-                    it[7].second.type.shouldBe("LoadBalancingGroup")
-                    it[7].second.routingObject.shouldNotBeNull()
-
-                    it[8].first shouldBe "pathPrefixRouter"
-                    it[8].second.tags.shouldContainAll("source=OriginsFileConverter")
-                    it[8].second.type.shouldBe("PathPrefixRouter")
-                    it[8].second.routingObject.shouldNotBeNull()
+                    it[0].name() shouldBe "appA.appA-1"
+                    it[0].tags().shouldContainAll("appA", "source=OriginsFileConverter")
+                    it[0].type().shouldBe("HostProxy")
+                    it[0].config().shouldNotBeNull()
+
+                    it[1].name() shouldBe "appA.appA-2"
+                    it[1].tags().shouldContainAll("appA", "source=OriginsFileConverter")
+                    it[1].type().shouldBe("HostProxy")
+                    it[1].config().shouldNotBeNull()
+
+                    it[2].name() shouldBe "appA"
+                    it[2].tags().shouldContainAll("source=OriginsFileConverter")
+                    it[2].type().shouldBe("LoadBalancingGroup")
+                    it[2].config().shouldNotBeNull()
+
+                    it[3].name() shouldBe "appB.appB-1"
+                    it[3].tags().shouldContainAll("appB", "source=OriginsFileConverter")
+                    it[3].type().shouldBe("HostProxy")
+                    it[3].config().shouldNotBeNull()
+
+                    it[4].name() shouldBe "appB"
+                    it[4].tags().shouldContainAll("source=OriginsFileConverter")
+                    it[4].type().shouldBe("LoadBalancingGroup")
+                    it[4].config().shouldNotBeNull()
+
+                    it[5].name() shouldBe "appC.appC-1"
+                    it[5].tags().shouldContainAll("appC", "source=OriginsFileConverter")
+                    it[5].type().shouldBe("HostProxy")
+                    it[5].config().shouldNotBeNull()
+
+                    it[6].name() shouldBe "appC.appC-2"
+                    it[6].tags().shouldContainAll("appC", "source=OriginsFileConverter")
+                    it[6].type().shouldBe("HostProxy")
+                    it[6].config().shouldNotBeNull()
+
+                    it[7].name() shouldBe "appC"
+                    it[7].tags().shouldContainAll("source=OriginsFileConverter")
+                    it[7].type().shouldBe("LoadBalancingGroup")
+                    it[7].config().shouldNotBeNull()
+
+                    it[8].name() shouldBe "pathPrefixRouter"
+                    it[8].tags().shouldContainAll("source=OriginsFileConverter")
+                    it[8].type().shouldBe("PathPrefixRouter")
+                    it[8].config().shouldNotBeNull()
                 }
     }