From 9d254bd9248f3c17c980e0350ac6daa8a17378d6 Mon Sep 17 00:00:00 2001 From: Ross Nicoll Date: Fri, 4 Nov 2016 10:53:50 +0000 Subject: [PATCH] Use UUID only for UniqueIdentifier equality The UniqueIdentifier class exists to ensure any external ID for a state is kept coupled to a proper unique ID, however in doing so it requires both UUID and external ID to find linear head states in the vault. This modifies the equality and hashing algorithms to use the UUID only, so that lookup can be done without knowing the external ID. --- .../r3corda/core/contracts/FinanceTypes.kt | 40 +++++++++---------- .../com/r3corda/core/FinanceTypesTest.kt | 27 +++++++++++++ 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/core/src/main/kotlin/com/r3corda/core/contracts/FinanceTypes.kt b/core/src/main/kotlin/com/r3corda/core/contracts/FinanceTypes.kt index 46f6c0fac6..54ab27fd9c 100644 --- a/core/src/main/kotlin/com/r3corda/core/contracts/FinanceTypes.kt +++ b/core/src/main/kotlin/com/r3corda/core/contracts/FinanceTypes.kt @@ -8,6 +8,7 @@ import com.fasterxml.jackson.databind.JsonSerializer import com.fasterxml.jackson.databind.SerializerProvider import com.fasterxml.jackson.databind.annotation.JsonDeserialize import com.fasterxml.jackson.databind.annotation.JsonSerialize +import com.google.common.annotations.VisibleForTesting import java.math.BigDecimal import java.time.DayOfWeek import java.time.LocalDate @@ -447,39 +448,36 @@ data class Commodity(val commodityCode: String, } /** - * This class provides a truly unique identifier of a trade, state, or other business object. + * This class provides a truly unique identifier of a trade, state, or other business object, bound to any existing + * external ID. Equality and comparison are based on the unique ID only; if two states somehow have the same UUID but + * different external IDs, it would indicate a problem with handling of IDs. * - * @param externalId If there is an existing weak identifier e.g. trade reference id. - * This should be set here the first time a UniqueIdentifier identifier is created as part of an issue, - * or ledger on-boarding activity. This ensure that the human readable identity is paired with the strong id. + * @param externalId Any existing weak identifier such as trade reference ID. + * This should be set here the first time a [UniqueIdentifier] is created as part of state issuance, + * or ledger on-boarding activity. This ensure that the human readable identity is paired with the strong ID. * @param id Should never be set by user code and left as default initialised. * So that the first time a state is issued this should be given a new UUID. - * Subsequent copies and evolutions of a state should just copy the externalId and Id fields unmodified. + * Subsequent copies and evolutions of a state should just copy the [externalId] and [id] fields unmodified. */ data class UniqueIdentifier(val externalId: String? = null, val id: UUID = UUID.randomUUID()) : Comparable { override fun toString(): String = if (externalId != null) "${externalId}_$id" else id.toString() companion object { + /** + * Helper function for unit tests where the UUID needs to be manually initialised for consistency. + */ + @VisibleForTesting fun fromString(name: String) : UniqueIdentifier = UniqueIdentifier(null, UUID.fromString(name)) } - override fun compareTo(other: UniqueIdentifier): Int { - val idCompare = id.compareTo(other.id) - return if (idCompare == 0) - compareExternalIds(other) + override fun compareTo(other: UniqueIdentifier): Int = id.compareTo(other.id) + + override fun equals(other: Any?): Boolean { + return if (other is UniqueIdentifier) + id == other.id else - idCompare + false } - private fun compareExternalIds(other: UniqueIdentifier): Int - = if (other.externalId == null) - if (externalId == null) - 0 - else - 1 - else - if (externalId == null) - -1 - else - externalId.compareTo(externalId) + override fun hashCode(): Int = id.hashCode() } diff --git a/core/src/test/kotlin/com/r3corda/core/FinanceTypesTest.kt b/core/src/test/kotlin/com/r3corda/core/FinanceTypesTest.kt index cb7c87aeed..1043e3437a 100644 --- a/core/src/test/kotlin/com/r3corda/core/FinanceTypesTest.kt +++ b/core/src/test/kotlin/com/r3corda/core/FinanceTypesTest.kt @@ -6,6 +6,7 @@ import java.time.LocalDate import java.util.* import kotlin.test.assertEquals import kotlin.test.assertFailsWith +import kotlin.test.assertNotEquals class FinanceTypesTest { @@ -152,5 +153,31 @@ class FinanceTypesTest { } + @Test + fun `unique identifier comparison`() { + val ids = listOf(UniqueIdentifier.fromString("e363f00e-4759-494d-a7ca-0dc966a92494"), + UniqueIdentifier.fromString("10ed0cc3-7bdf-4000-b610-595e36667d7d"), + UniqueIdentifier("Test", UUID.fromString("10ed0cc3-7bdf-4000-b610-595e36667d7d")) + ) + assertEquals(-1, ids[0].compareTo(ids[1])) + assertEquals(1, ids[1].compareTo(ids[0])) + assertEquals(0, ids[0].compareTo(ids[0])) + // External ID is not taken into account + assertEquals(0, ids[1].compareTo(ids[2])) + } + @Test + fun `unique identifier equality`() { + val ids = listOf(UniqueIdentifier.fromString("e363f00e-4759-494d-a7ca-0dc966a92494"), + UniqueIdentifier.fromString("10ed0cc3-7bdf-4000-b610-595e36667d7d"), + UniqueIdentifier("Test", UUID.fromString("10ed0cc3-7bdf-4000-b610-595e36667d7d")) + ) + assertEquals(ids[0], ids[0]) + assertNotEquals(ids[0], ids[1]) + assertEquals(ids[0].hashCode(), ids[0].hashCode()) + assertNotEquals(ids[0].hashCode(), ids[1].hashCode()) + // External ID is not taken into account + assertEquals(ids[1], ids[2]) + assertEquals(ids[1].hashCode(), ids[2].hashCode()) + } } \ No newline at end of file