From af081a7170c6de4cb9788247ba66272623024c39 Mon Sep 17 00:00:00 2001 From: Katarzyna Streich Date: Thu, 18 Jan 2018 16:23:41 +0000 Subject: [PATCH] Remove primary key constraint on DBHostAndPort (#2318) Remove primary key constraint on DBHostAndPort Return always first node if more are matching by address. --- .../core/internal/schemas/NodeInfoSchema.kt | 18 ++++++++--------- .../network/PersistentNetworkMapCacheTest.kt | 20 +++++++++++++++---- .../network/PersistentNetworkMapCache.kt | 6 +++--- .../services/network/NetworkMapCacheTest.kt | 12 +++++++++++ 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/core/src/main/kotlin/net/corda/core/internal/schemas/NodeInfoSchema.kt b/core/src/main/kotlin/net/corda/core/internal/schemas/NodeInfoSchema.kt index 6e35a631c4..fe7067d768 100644 --- a/core/src/main/kotlin/net/corda/core/internal/schemas/NodeInfoSchema.kt +++ b/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 - private val pk: PKHostAndPort + @Id + @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!!) } } diff --git a/node/src/integration-test/kotlin/net/corda/node/services/network/PersistentNetworkMapCacheTest.kt b/node/src/integration-test/kotlin/net/corda/node/services/network/PersistentNetworkMapCacheTest.kt index 1b98c4f54e..67ec3d592c 100644 --- a/node/src/integration-test/kotlin/net/corda/node/services/network/PersistentNetworkMapCacheTest.kt +++ b/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.BOB_NAME -import net.corda.testing.TestIdentity -import net.corda.testing.chooseIdentity +import net.corda.testing.* 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] diff --git a/node/src/main/kotlin/net/corda/node/services/network/PersistentNetworkMapCache.kt b/node/src/main/kotlin/net/corda/node/services/network/PersistentNetworkMapCache.kt index ca54c89019..1eafc11b00 100644 --- a/node/src/main/kotlin/net/corda/node/services/network/PersistentNetworkMapCache.kt +++ b/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 - else result.map { it.toNodeInfo() }.singleOrNull() ?: throw IllegalStateException("More than one node with the same host and port") + return result.map { it.toNodeInfo() }.singleOrNull() } /** Object Relational Mapping support. */ diff --git a/node/src/test/kotlin/net/corda/node/services/network/NetworkMapCacheTest.kt b/node/src/test/kotlin/net/corda/node/services/network/NetworkMapCacheTest.kt index eb95a985dd..b56728fcf6 100644 --- a/node/src/test/kotlin/net/corda/node/services/network/NetworkMapCacheTest.kt +++ b/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) + } }