A node that is exporting a network map service should not try to register with itself twice (one internally and once over the network).

Minor renamings and cleanups in the network map code.

Throw an exception if a production node isn't configured with any network map service at all.

node/src/main/kotlin/core/node/AbstractNode.kt

@@ -35,10 +35,10 @@ import java.util.*
  * A base node implementation that can be customised either for production (with real implementations that do real
  * I/O), or a mock implementation suitable for unit test environments.
  */
-// TODO: Where this node is the initial network map service, currently no initialNetworkMapAddress is provided.
+// TODO: Where this node is the initial network map service, currently no networkMapService is provided.
 // In theory the NodeInfo for the node should be passed in, instead, however currently this is constructed by the
 // AbstractNode. It should be possible to generate the NodeInfo outside of AbstractNode, so it can be passed in.
-abstract class AbstractNode(val dir: Path, val configuration: NodeConfiguration, val initialNetworkMapAddress: NodeInfo?,
+abstract class AbstractNode(val dir: Path, val configuration: NodeConfiguration, val networkMapService: NodeInfo?,
                             val advertisedServices: Set<ServiceType>, val platformClock: Clock) {
     companion object {
         val PRIVATE_KEY_FILE_NAME = "identity-private-key"
@@ -91,6 +91,10 @@ abstract class AbstractNode(val dir: Path, val configuration: NodeConfiguration,
     lateinit var net: MessagingService
     lateinit var api: APIServer
 
+    /** Completes once the node has successfully registered with the network map service. Null until [start] returns. */
+    @Volatile var networkMapRegistrationFuture: ListenableFuture<Unit>? = null
+        private set
+
     open fun start(): AbstractNode {
         log.info("Node starting up ...")
 
@@ -113,29 +117,36 @@ abstract class AbstractNode(val dir: Path, val configuration: NodeConfiguration,
         DataVendingService(net, storage)
 
         startMessagingService()
-
-        require(initialNetworkMapAddress == null || NetworkMapService.Type in initialNetworkMapAddress.advertisedServices)
-        { "Initial network map address must indicate a node that provides a network map service" }
-        configureNetworkMapCache()
-
+        networkMapRegistrationFuture = registerWithNetworkMap()
         return this
     }
     /**
      * Register this node with the network map cache, and load network map from a remote service (and register for
      * updates) if one has been supplied.
      */
-    private fun configureNetworkMapCache() {
+    private fun registerWithNetworkMap(): ListenableFuture<Unit> {
+        require(networkMapService == null || NetworkMapService.Type in networkMapService.advertisedServices) {
+            "Initial network map address must indicate a node that provides a network map service"
+        }
         services.networkMapCache.addNode(info)
-        if (initialNetworkMapAddress != null) {
-            // TODO: Return a future so the caller knows these operations may not have completed yet, and can monitor
-            // if needed
-            updateRegistration(initialNetworkMapAddress, AddOrRemove.ADD)
-            services.networkMapCache.addMapService(net, initialNetworkMapAddress, true, null)
-        }
-        if (inNodeNetworkMapService != null) {
-            // Register for updates
-            services.networkMapCache.addMapService(net, info, true, null)
+        if (networkMapService != null && networkMapService != info) {
+            // Only register if we are pointed at a network map service and it's not us.
+            // TODO: Return a future so the caller knows these operations may not have completed yet, and can monitor if needed
+            updateRegistration(networkMapService, AddOrRemove.ADD)
+            return services.networkMapCache.addMapService(net, networkMapService, true, null)
         }
+        // In the unit test environment, we may run without any network map service sometimes.
+        if (inNodeNetworkMapService == null)
+            return noNetworkMapConfigured()
+        // Register for updates, even if we're the one running the network map.
+        return services.networkMapCache.addMapService(net, info, true, null)
+    }
+
+    /** This is overriden by the mock node implementation to enable operation without any network map service */
+    protected open fun noNetworkMapConfigured(): ListenableFuture<Unit> {
+        // TODO: There should be a consistent approach to configuration error exceptions.
+        throw IllegalStateException("Configuration error: this node isn't being asked to act as the network map, nor " +
+                "has any other map node been configured.")
     }
 
     private fun updateRegistration(serviceInfo: NodeInfo, type: AddOrRemove): ListenableFuture<NetworkMapService.RegistrationResponse> {
@@ -178,8 +189,8 @@ abstract class AbstractNode(val dir: Path, val configuration: NodeConfiguration,
 
     protected open fun makeIdentityService(): IdentityService {
         val service = InMemoryIdentityService()
-        if (initialNetworkMapAddress != null)
-            service.registerIdentity(initialNetworkMapAddress.identity)
+        if (networkMapService != null)
+            service.registerIdentity(networkMapService.identity)
         service.registerIdentity(storage.myLegalIdentity)
 
         services.networkMapCache.partyNodes.forEach { service.registerIdentity(it.identity) }

node/src/main/kotlin/core/node/services/NetworkMapService.kt

@@ -18,13 +18,13 @@ import core.serialization.deserialize
 import core.serialization.serialize
 import core.utilities.AddOrRemove
 import org.slf4j.LoggerFactory
-import protocols.*
+import protocols.AbstractRequestMessage
 import java.security.PrivateKey
-import java.time.Period
 import java.time.Instant
-import java.util.ArrayList
-import java.util.concurrent.atomic.AtomicInteger
+import java.time.Period
+import java.util.*
 import java.util.concurrent.ConcurrentHashMap
+import java.util.concurrent.atomic.AtomicInteger
 import javax.annotation.concurrent.ThreadSafe
 
 

node/src/main/kotlin/core/testing/MockNode.kt

@@ -1,6 +1,7 @@
 package core.testing
 
 import com.google.common.jimfs.Jimfs
+import com.google.common.util.concurrent.Futures
 import core.crypto.Party
 import core.messaging.MessagingService
 import core.messaging.SingleMessageRecipient
@@ -27,6 +28,10 @@ import java.util.*
  * Mock network nodes require manual pumping by default: they will not run asynchronous. This means that
  * for message exchanges to take place (and associated handlers to run), you must call the [runNetwork]
  * method.
+ *
+ * You can get a printout of every message sent by using code like:
+ *
+ *    BriefLogFormatter.initVerbose("+messaging")
  */
 class MockNetwork(private val threadPerNode: Boolean = false,
                   private val defaultFactory: Factory = MockNetwork.DefaultFactory) {
@@ -82,6 +87,9 @@ class MockNetwork(private val threadPerNode: Boolean = false,
 
         override fun generateKeyPair(): KeyPair? = keyPair ?: super.generateKeyPair()
 
+        // It's OK to not have a network map service in the mock network.
+        override fun noNetworkMapConfigured() = Futures.immediateFuture(Unit)
+
         // There is no need to slow down the unit tests by initialising CityDatabase
         override fun findMyLocation(): PhysicalLocation? = null