Remove primary key constraint on DBHostAndPort (#2318)

Remove primary key constraint on DBHostAndPort

Return always first node if more are matching by address.

core/src/main/kotlin/net/corda/core/internal/schemas/NodeInfoSchema.kt

@@ -63,26 +63,24 @@ object NodeInfoSchemaV1 : MappedSchema(
         }
     }
 
-    @Embeddable
-    data class PKHostAndPort(
-            val host: String? = null,
-            val port: Int? = null
-    ) : Serializable
-
     @Entity
     @Table(name = "node_info_hosts")
     data class DBHostAndPort(
-            @EmbeddedId
+            @Id
-            private val pk: PKHostAndPort
+            @GeneratedValue
+            @Column(name = "hosts_id")
+            var id: Int,
+            val host: String? = null,
+            val port: Int? = null
     ) {
         companion object {
             fun fromHostAndPort(hostAndPort: NetworkHostAndPort) = DBHostAndPort(
-                    PKHostAndPort(hostAndPort.host, hostAndPort.port)
+                    0, hostAndPort.host, hostAndPort.port
             )
         }
 
         fun toHostAndPort(): NetworkHostAndPort {
-            return NetworkHostAndPort(this.pk.host!!, this.pk.port!!)
+            return NetworkHostAndPort(host!!, port!!)
         }
     }
 

node/src/integration-test/kotlin/net/corda/node/services/network/PersistentNetworkMapCacheTest.kt

@@ -1,20 +1,19 @@
 package net.corda.node.services.network
 
+import net.corda.core.crypto.generateKeyPair
 import net.corda.core.identity.CordaX500Name
 import net.corda.core.identity.Party
 import net.corda.core.node.NodeInfo
 import net.corda.core.utilities.NetworkHostAndPort
 import net.corda.node.internal.Node
 import net.corda.node.internal.StartedNode
-import net.corda.testing.ALICE_NAME
+import net.corda.testing.*
-import net.corda.testing.BOB_NAME
-import net.corda.testing.TestIdentity
-import net.corda.testing.chooseIdentity
 import net.corda.testing.node.internal.NodeBasedTest
 import org.junit.Before
 import org.junit.Test
 import kotlin.test.assertEquals
 
+// TODO Clean up these tests, they were written with old network map design in place.
 class PersistentNetworkMapCacheTest : NodeBasedTest() {
     private companion object {
         val ALICE = TestIdentity(ALICE_NAME, 70).party
@@ -58,6 +57,19 @@ class PersistentNetworkMapCacheTest : NodeBasedTest() {
         }
     }
 
+    // This test has to be done as normal node not mock, because MockNodes don't have addresses.
+    @Test
+    fun `insert two node infos with the same host and port`() {
+        val aliceNode = startNode(ALICE_NAME)
+        val charliePartyCert = getTestPartyAndCertificate(CHARLIE_NAME, generateKeyPair().public)
+        val aliceCache = aliceNode.services.networkMapCache
+        aliceCache.addNode(aliceNode.info.copy(legalIdentitiesAndCerts = listOf(charliePartyCert)))
+        val res = aliceNode.database.transaction {
+            aliceCache.allNodes.filter { aliceNode.info.addresses[0] in it.addresses }
+        }
+        assertEquals(2, res.size)
+    }
+
     @Test
     fun `restart node with DB map cache`() {
         val alice = startNodesWithPort(listOf(ALICE))[0]

node/src/main/kotlin/net/corda/node/services/network/PersistentNetworkMapCache.kt

@@ -292,13 +292,13 @@ open class PersistentNetworkMapCache(
 
     private fun queryByAddress(session: Session, hostAndPort: NetworkHostAndPort): NodeInfo? {
         val query = session.createQuery(
-                "SELECT n FROM ${NodeInfoSchemaV1.PersistentNodeInfo::class.java.name} n JOIN n.addresses a WHERE a.pk.host = :host AND a.pk.port = :port",
+                "SELECT n FROM ${NodeInfoSchemaV1.PersistentNodeInfo::class.java.name} n JOIN n.addresses a WHERE a.host = :host AND a.port = :port",
                 NodeInfoSchemaV1.PersistentNodeInfo::class.java)
         query.setParameter("host", hostAndPort.host)
         query.setParameter("port", hostAndPort.port)
+        query.setMaxResults(1)
         val result = query.resultList
-        return if (result.isEmpty()) null
+        return result.map { it.toNodeInfo() }.singleOrNull()
-        else result.map { it.toNodeInfo() }.singleOrNull() ?: throw IllegalStateException("More than one node with the same host and port")
     }
 
     /** Object Relational Mapping support. */

node/src/test/kotlin/net/corda/node/services/network/NetworkMapCacheTest.kt

@@ -1,9 +1,11 @@
 package net.corda.node.services.network
 
+import net.corda.core.crypto.generateKeyPair
 import net.corda.core.node.services.NetworkMapCache
 import net.corda.node.services.api.NetworkMapCacheInternal
 import net.corda.testing.ALICE_NAME
 import net.corda.testing.BOB_NAME
+import net.corda.testing.getTestPartyAndCertificate
 import net.corda.testing.node.MockNetwork
 import net.corda.testing.node.MockNodeParameters
 import net.corda.testing.singleIdentity
@@ -106,4 +108,14 @@ class NetworkMapCacheTest {
             assertThat(bobCache.getNodeByLegalName(alice.name) == null)
         }
     }
+
+    @Test
+    fun `add two nodes the same name different keys`() {
+        val aliceNode = mockNet.createPartyNode(ALICE_NAME)
+        val aliceCache = aliceNode.services.networkMapCache
+        val alicePartyAndCert2 = getTestPartyAndCertificate(ALICE_NAME, generateKeyPair().public)
+        aliceCache.addNode(aliceNode.info.copy(legalIdentitiesAndCerts = listOf(alicePartyAndCert2)))
+        // This is correct behaviour as we may have distributed service nodes.
+        assertEquals(2, aliceCache.getNodesByLegalName(ALICE_NAME).size)
+    }
 }