From 0cab721b47834199c2fdff0059767e3826c2d58a Mon Sep 17 00:00:00 2001 From: Joel Dudley Date: Fri, 8 Jun 2018 10:23:05 +0100 Subject: [PATCH 1/3] Documents use of $ to access internal classes. (#3325) --- docs/source/shell.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/source/shell.rst b/docs/source/shell.rst index a83c018655..726c5fd5a6 100644 --- a/docs/source/shell.rst +++ b/docs/source/shell.rst @@ -233,6 +233,8 @@ simple JSON-like language. The key features of Yaml are: * Strings do not need to be surrounded by quotes unless they contain commas, colons or embedded quotes * Class names must be fully-qualified (e.g. ``java.lang.String``) +* Nested classes are referenced using ``$``. For example, the ``net.corda.finance.contracts.asset.Cash.State`` + class is referenced as ``net.corda.finance.contracts.asset.Cash$State`` (note the ``$``) .. note:: If your CorDapp is written in Java, named arguments won't work unless you compiled the node using the ``-parameters`` argument to javac. See :doc:`generating-a-node` for how to specify it via Gradle. From b9bf624e7ac6c577f5355a8672eaaea095f2a5dc Mon Sep 17 00:00:00 2001 From: Tudor Malene Date: Fri, 8 Jun 2018 10:24:00 +0100 Subject: [PATCH 2/3] ENT-1873 address code review changes (#3323) --- node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt b/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt index bfe1856851..c294d9fe09 100644 --- a/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt +++ b/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt @@ -1025,7 +1025,7 @@ fun configureDatabase(hikariProperties: Properties, } catch (ex: Exception) { when { ex is HikariPool.PoolInitializationException -> throw CouldNotCreateDataSourceException("Could not connect to the database. Please check your JDBC connection URL, or the connectivity to the database.") - ex.cause is ClassNotFoundException -> throw CouldNotCreateDataSourceException("Could not find the database driver class. Please add it to the 'drivers' folders. See: https://docs.corda.net/corda-configuration-file.html") + ex.cause is ClassNotFoundException -> throw CouldNotCreateDataSourceException("Could not find the database driver class. Please add it to the 'drivers' folder. See: https://docs.corda.net/corda-configuration-file.html") else -> throw CouldNotCreateDataSourceException("Could not create the DataSource: ${ex.message}", ex) } } From baa6479575326173c9aff013653400d854195f6c Mon Sep 17 00:00:00 2001 From: szymonsztuka Date: Fri, 8 Jun 2018 10:25:50 +0100 Subject: [PATCH 3/3] ENT-1712 Warning on cross-reference between JPA entities from different mapped schemas (#3282) At startup node warns about any MappedSchema containing a JPA entity referencing to another JPA entity from a different MappedSchema. Cross reference between JPA entities across different MappedSchema objects can cause operational issues while evolving one MappedSchema object or migrating it's data. Doc API: Persistence documentation no longer suggests mapping between different schemas. --- .../net/corda/core/schemas/PersistentTypes.kt | 42 ++++++- .../corda/core/schemas/BadSchemaJavaV1.java | 37 ++++++ .../core/schemas/BadSchemaNoGetterJavaV1.java | 29 +++++ .../corda/core/schemas/GoodSchemaJavaV1.java | 26 +++++ .../core/schemas/PoliteSchemaJavaV1.java | 37 ++++++ .../core/schemas/TestJavaSchemaFamily.java | 4 + .../core/schemas/TrickySchemaJavaV1.java | 36 ++++++ ...ppedSchemasCrossReferenceDetectionTests.kt | 108 ++++++++++++++++++ docs/source/api-persistence.rst | 10 +- docs/source/changelog.rst | 4 + .../net/corda/node/internal/AbstractNode.kt | 5 + .../node/services/schema/NodeSchemaService.kt | 7 ++ 12 files changed, 341 insertions(+), 4 deletions(-) create mode 100644 core/src/test/java/net/corda/core/schemas/BadSchemaJavaV1.java create mode 100644 core/src/test/java/net/corda/core/schemas/BadSchemaNoGetterJavaV1.java create mode 100644 core/src/test/java/net/corda/core/schemas/GoodSchemaJavaV1.java create mode 100644 core/src/test/java/net/corda/core/schemas/PoliteSchemaJavaV1.java create mode 100644 core/src/test/java/net/corda/core/schemas/TestJavaSchemaFamily.java create mode 100644 core/src/test/java/net/corda/core/schemas/TrickySchemaJavaV1.java create mode 100644 core/src/test/kotlin/net/corda/core/schemas/MappedSchemasCrossReferenceDetectionTests.kt diff --git a/core/src/main/kotlin/net/corda/core/schemas/PersistentTypes.kt b/core/src/main/kotlin/net/corda/core/schemas/PersistentTypes.kt index 8d4d48cc49..fccbbf1ffb 100644 --- a/core/src/main/kotlin/net/corda/core/schemas/PersistentTypes.kt +++ b/core/src/main/kotlin/net/corda/core/schemas/PersistentTypes.kt @@ -90,4 +90,44 @@ data class PersistentStateRef( /** * Marker interface to denote a persistable Corda state entity that will always have a transaction id and index */ -interface StatePersistable : Serializable \ No newline at end of file +interface StatePersistable : Serializable + +object MappedSchemaValidator { + fun fieldsFromOtherMappedSchema(schema: MappedSchema) : List = + schema.mappedTypes.map { entity -> + entity.declaredFields.filter { field -> + field.type.enclosingClass != null + && MappedSchema::class.java.isAssignableFrom(field.type.enclosingClass) + && hasJpaAnnotation(field.declaredAnnotations) + && field.type.enclosingClass != schema.javaClass + }.map { field -> SchemaCrossReferenceReport(schema.javaClass.name, entity.simpleName, field.type.enclosingClass.name, field.name, field.type.simpleName)} + }.flatMap { it.toSet() } + + fun methodsFromOtherMappedSchema(schema: MappedSchema) : List = + schema.mappedTypes.map { entity -> + entity.declaredMethods.filter { method -> + method.returnType.enclosingClass != null + && MappedSchema::class.java.isAssignableFrom(method.returnType.enclosingClass) + && method.returnType.enclosingClass != schema.javaClass + && hasJpaAnnotation(method.declaredAnnotations) + }.map { method -> SchemaCrossReferenceReport(schema.javaClass.name, entity.simpleName, method.returnType.enclosingClass.name, method.name, method.returnType.simpleName)} + }.flatMap { it.toSet() } + + fun crossReferencesToOtherMappedSchema(schema: MappedSchema) : List = + fieldsFromOtherMappedSchema(schema) + methodsFromOtherMappedSchema(schema) + + /** Returns true if [javax.persistence] annotation expect [javax.persistence.Transient] is found. */ + private inline fun hasJpaAnnotation(annotations: Array) = + annotations.any { annotation -> annotation.toString().startsWith("@javax.persistence.") && annotation !is javax.persistence.Transient } + + class SchemaCrossReferenceReport(private val schema: String, private val entity: String, private val referencedSchema: String, + private val fieldOrMethod: String, private val fieldOrMethodType: String) { + + override fun toString() = "Cross-reference between MappedSchemas '$schema' and '$referencedSchema'. " + + "MappedSchema '${schema.substringAfterLast(".")}' entity '$entity' field '$fieldOrMethod' is of type '$fieldOrMethodType' " + + "defined in another MappedSchema '${referencedSchema.substringAfterLast(".")}'." + + fun toWarning() = toString() + " This may cause issues when evolving MappedSchema or migrating its data, " + + "ensure JPA entities are defined within the same enclosing MappedSchema." + } +} diff --git a/core/src/test/java/net/corda/core/schemas/BadSchemaJavaV1.java b/core/src/test/java/net/corda/core/schemas/BadSchemaJavaV1.java new file mode 100644 index 0000000000..63178184d9 --- /dev/null +++ b/core/src/test/java/net/corda/core/schemas/BadSchemaJavaV1.java @@ -0,0 +1,37 @@ +package net.corda.core.schemas; + +import javax.persistence.*; +import java.util.Arrays; + +public class BadSchemaJavaV1 extends MappedSchema { + + public BadSchemaJavaV1() { + super(TestJavaSchemaFamily.class, 1, Arrays.asList(State.class)); + } + + @Entity + public static class State extends PersistentState { + private String id; + private GoodSchemaJavaV1.State other; + + @Column + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + @JoinColumns({@JoinColumn(name = "itid"), @JoinColumn(name = "outid")}) + @OneToOne + @MapsId + public GoodSchemaJavaV1.State getOther() { + return other; + } + + public void setOther(GoodSchemaJavaV1.State other) { + this.other = other; + } + } +} \ No newline at end of file diff --git a/core/src/test/java/net/corda/core/schemas/BadSchemaNoGetterJavaV1.java b/core/src/test/java/net/corda/core/schemas/BadSchemaNoGetterJavaV1.java new file mode 100644 index 0000000000..36534df566 --- /dev/null +++ b/core/src/test/java/net/corda/core/schemas/BadSchemaNoGetterJavaV1.java @@ -0,0 +1,29 @@ +package net.corda.core.schemas; + +import javax.persistence.*; +import java.util.Arrays; + +public class BadSchemaNoGetterJavaV1 extends MappedSchema { + + public BadSchemaNoGetterJavaV1() { + super(TestJavaSchemaFamily.class, 1, Arrays.asList(State.class)); + } + + @Entity + public static class State extends PersistentState { + @JoinColumns({@JoinColumn(name = "itid"), @JoinColumn(name = "outid")}) + @OneToOne + @MapsId + public GoodSchemaJavaV1.State other; + private String id; + + @Column + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + } +} diff --git a/core/src/test/java/net/corda/core/schemas/GoodSchemaJavaV1.java b/core/src/test/java/net/corda/core/schemas/GoodSchemaJavaV1.java new file mode 100644 index 0000000000..a7b507b2b0 --- /dev/null +++ b/core/src/test/java/net/corda/core/schemas/GoodSchemaJavaV1.java @@ -0,0 +1,26 @@ +package net.corda.core.schemas; + +import javax.persistence.Column; +import javax.persistence.Entity; +import java.util.Arrays; + +public class GoodSchemaJavaV1 extends MappedSchema { + + public GoodSchemaJavaV1() { + super(TestJavaSchemaFamily.class, 1, Arrays.asList(State.class)); + } + + @Entity + public static class State extends PersistentState { + private String id; + + @Column + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + } +} \ No newline at end of file diff --git a/core/src/test/java/net/corda/core/schemas/PoliteSchemaJavaV1.java b/core/src/test/java/net/corda/core/schemas/PoliteSchemaJavaV1.java new file mode 100644 index 0000000000..e1bd6b1970 --- /dev/null +++ b/core/src/test/java/net/corda/core/schemas/PoliteSchemaJavaV1.java @@ -0,0 +1,37 @@ +package net.corda.core.schemas; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.Transient; +import java.util.Arrays; + +public class PoliteSchemaJavaV1 extends MappedSchema { + + public PoliteSchemaJavaV1() { + super(TestJavaSchemaFamily.class, 1, Arrays.asList(State.class)); + } + + @Entity + public static class State extends PersistentState { + private String id; + private GoodSchemaJavaV1.State other; + + @Column + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + @Transient + public GoodSchemaJavaV1.State getOther() { + return other; + } + + public void setOther(GoodSchemaJavaV1.State other) { + this.other = other; + } + } +} \ No newline at end of file diff --git a/core/src/test/java/net/corda/core/schemas/TestJavaSchemaFamily.java b/core/src/test/java/net/corda/core/schemas/TestJavaSchemaFamily.java new file mode 100644 index 0000000000..c2131097f3 --- /dev/null +++ b/core/src/test/java/net/corda/core/schemas/TestJavaSchemaFamily.java @@ -0,0 +1,4 @@ +package net.corda.core.schemas; + +public class TestJavaSchemaFamily { +} diff --git a/core/src/test/java/net/corda/core/schemas/TrickySchemaJavaV1.java b/core/src/test/java/net/corda/core/schemas/TrickySchemaJavaV1.java new file mode 100644 index 0000000000..1d3926440b --- /dev/null +++ b/core/src/test/java/net/corda/core/schemas/TrickySchemaJavaV1.java @@ -0,0 +1,36 @@ +package net.corda.core.schemas; + +import javax.persistence.Column; +import javax.persistence.Entity; +import java.util.Arrays; + +public class TrickySchemaJavaV1 extends MappedSchema { + + public TrickySchemaJavaV1() { + super(TestJavaSchemaFamily.class, 1, Arrays.asList(TrickySchemaJavaV1.State.class)); + } + + @Entity + public static class State extends PersistentState { + private String id; + private GoodSchemaJavaV1.State other; + + @Column + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + //the field is a cross-reference to other MappedSchema however the field is not persistent (no JPA annotation) + public GoodSchemaJavaV1.State getOther() { + return other; + } + + public void setOther(GoodSchemaJavaV1.State other) { + this.other = other; + } + } +} \ No newline at end of file diff --git a/core/src/test/kotlin/net/corda/core/schemas/MappedSchemasCrossReferenceDetectionTests.kt b/core/src/test/kotlin/net/corda/core/schemas/MappedSchemasCrossReferenceDetectionTests.kt new file mode 100644 index 0000000000..e194d8c3dd --- /dev/null +++ b/core/src/test/kotlin/net/corda/core/schemas/MappedSchemasCrossReferenceDetectionTests.kt @@ -0,0 +1,108 @@ +package net.corda.core.schemas + +import net.corda.core.schemas.MappedSchemaValidator.fieldsFromOtherMappedSchema +import net.corda.core.schemas.MappedSchemaValidator.methodsFromOtherMappedSchema +import net.corda.finance.schemas.CashSchema +import org.assertj.core.api.Assertions.assertThat +import org.junit.Test +import javax.persistence.* + +class MappedSchemasCrossReferenceDetectionTests { + + object GoodSchema : MappedSchema(schemaFamily = CashSchema.javaClass, version = 1, mappedTypes = listOf(State::class.java)) { + @Entity + class State( + @Column + var id: String + ) : PersistentState() + } + + object BadSchema : MappedSchema(schemaFamily = CashSchema.javaClass, version = 1, mappedTypes = listOf(State::class.java)) { + @Entity + class State( + @Column + var id: String, + + @JoinColumns(JoinColumn(name = "itid"), JoinColumn(name = "outid")) + @OneToOne + @MapsId + var other: GoodSchema.State + ) : PersistentState() + } + + object TrickySchema : MappedSchema(schemaFamily = CashSchema.javaClass, version = 1, mappedTypes = listOf(State::class.java)) { + @Entity + class State( + @Column + var id: String, + + //the field is a cross-reference to other MappedSchema however the field is not persistent (no JPA annotation) + var other: GoodSchema.State + ) : PersistentState() + } + + object PoliteSchema : MappedSchema(schemaFamily = CashSchema.javaClass, version = 1, mappedTypes = listOf(State::class.java)) { + @Entity + class State( + @Column + var id: String, + + @Transient + var other: GoodSchema.State + ) : PersistentState() + } + + @Test + fun `no cross reference to other schema`() { + assertThat(fieldsFromOtherMappedSchema(GoodSchema)).isEmpty() + assertThat(methodsFromOtherMappedSchema(GoodSchema)).isEmpty() + } + + @Test + fun `cross reference to other schema is detected`() { + assertThat(fieldsFromOtherMappedSchema(BadSchema)).isNotEmpty + assertThat(methodsFromOtherMappedSchema(BadSchema)).isEmpty() + } + + @Test + fun `cross reference via non JPA field is allowed`() { + assertThat(fieldsFromOtherMappedSchema(TrickySchema)).isEmpty() + assertThat(methodsFromOtherMappedSchema(TrickySchema)).isEmpty() + } + + @Test + fun `cross reference via transient field is allowed`() { + assertThat(fieldsFromOtherMappedSchema(PoliteSchema)).isEmpty() + assertThat(methodsFromOtherMappedSchema(PoliteSchema)).isEmpty() + } + + @Test + fun `no cross reference to other schema java`() { + assertThat(fieldsFromOtherMappedSchema(GoodSchemaJavaV1())).isEmpty() + assertThat(methodsFromOtherMappedSchema(GoodSchemaJavaV1())).isEmpty() + } + + @Test + fun `cross reference to other schema is detected java`() { + assertThat(fieldsFromOtherMappedSchema(BadSchemaJavaV1())).isEmpty() + assertThat(methodsFromOtherMappedSchema(BadSchemaJavaV1())).isNotEmpty + } + + @Test + fun `cross reference to other schema via field is detected java`() { + assertThat(fieldsFromOtherMappedSchema(BadSchemaNoGetterJavaV1())).isNotEmpty + assertThat(methodsFromOtherMappedSchema(BadSchemaNoGetterJavaV1())).isEmpty() + } + + @Test + fun `cross reference via non JPA field is allowed java`() { + assertThat(fieldsFromOtherMappedSchema(TrickySchemaJavaV1())).isEmpty() + assertThat(methodsFromOtherMappedSchema(TrickySchemaJavaV1())).isEmpty() + } + + @Test + fun `cross reference via transient field is allowed java`() { + assertThat(fieldsFromOtherMappedSchema(PoliteSchemaJavaV1())).isEmpty() + assertThat(methodsFromOtherMappedSchema(PoliteSchemaJavaV1())).isEmpty() + } +} \ No newline at end of file diff --git a/docs/source/api-persistence.rst b/docs/source/api-persistence.rst index 0cf26089bf..67d1bd2767 100644 --- a/docs/source/api-persistence.rst +++ b/docs/source/api-persistence.rst @@ -57,11 +57,15 @@ integration points and not necessarily with every upgrade to the contract code. ``MappedSchema`` offered by a ``QueryableState``, automatically upgrade to a later version of a schema or even provide a ``MappedSchema`` not originally offered by the ``QueryableState``. -It is expected that multiple different contract state implementations might provide mappings to some common schema. -For example an Interest Rate Swap contract and an Equity OTC Option contract might both provide a mapping to a common -Derivative schema. The schemas should typically not be part of the contract itself and should exist independently of it +It is expected that multiple different contract state implementations might provide mappings within a single schema. +For example an Interest Rate Swap contract and an Equity OTC Option contract might both provide a mapping to +a Derivative contract within the same schema. The schemas should typically not be part of the contract itself and should exist independently to encourage re-use of a common set within a particular business area or Cordapp. +.. note:: It's advisable to avoid cross-references between different schemas as this may cause issues when evolving ``MappedSchema`` + or migrating its data. At startup, nodes log such violations as warnings stating that there's a cross-reference between ``MappedSchema``'s. + The detailed messages incorporate information about what schemas, entities and fields are involved. + ``MappedSchema`` offer a family name that is disambiguated using Java package style name-spacing derived from the class name of a *schema family* class that is constant across versions, allowing the ``SchemaService`` to select a preferred version of a schema. diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst index 7a4de25f3d..22df9ee1b8 100644 --- a/docs/source/changelog.rst +++ b/docs/source/changelog.rst @@ -132,6 +132,10 @@ Unreleased * Table name with a typo changed from ``NODE_ATTCHMENTS_CONTRACTS`` to ``NODE_ATTACHMENTS_CONTRACTS``. +* Node logs a warning for any ``MappedSchema`` containing a JPA entity referencing another JPA entity from a different ``MappedSchema`. + The log entry starts with `Cross-reference between MappedSchemas.`. + API: Persistence documentation no longer suggests mapping between different schemas. + .. _changelog_v3.1: Version 3.1 diff --git a/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt b/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt index c294d9fe09..e6a90d4379 100644 --- a/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt +++ b/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt @@ -232,6 +232,11 @@ abstract class AbstractNode(val configuration: NodeConfiguration, initCertificate() initialiseJVMAgents() val schemaService = NodeSchemaService(cordappLoader.cordappSchemas, configuration.notary != null) + schemaService.mappedSchemasWarnings().forEach { + val warning = it.toWarning() + log.warn(warning) + Node.printWarning(warning) + } val (identity, identityKeyPair) = obtainIdentity(notaryConfig = null) // Wrapped in an atomic reference just to allow setting it before the closure below gets invoked. diff --git a/node/src/main/kotlin/net/corda/node/services/schema/NodeSchemaService.kt b/node/src/main/kotlin/net/corda/node/services/schema/NodeSchemaService.kt index 18de950bf9..0421e00220 100644 --- a/node/src/main/kotlin/net/corda/node/services/schema/NodeSchemaService.kt +++ b/node/src/main/kotlin/net/corda/node/services/schema/NodeSchemaService.kt @@ -7,6 +7,8 @@ import net.corda.core.schemas.CommonSchemaV1 import net.corda.core.schemas.MappedSchema import net.corda.core.schemas.PersistentState import net.corda.core.schemas.QueryableState +import net.corda.core.schemas.* +import net.corda.core.schemas.MappedSchemaValidator.crossReferencesToOtherMappedSchema import net.corda.core.serialization.SingletonSerializeAsToken import net.corda.node.internal.schemas.NodeInfoSchemaV1 import net.corda.node.services.api.SchemaService @@ -91,4 +93,9 @@ class NodeSchemaService(extraSchemas: Set = emptySet(), includeNot return VaultSchemaV1.VaultFungibleStates(state.owner, state.amount.quantity, state.amount.token.issuer.party, state.amount.token.issuer.reference, state.participants) return (state as QueryableState).generateMappedObject(schema) } + + /** Returns list of [MappedSchemaValidator.SchemaCrossReferenceReport] violations. */ + fun mappedSchemasWarnings(): List = + schemaOptions.keys.map { schema -> crossReferencesToOtherMappedSchema(schema) }.flatMap { it.toList() } + }