diff --git a/query/src/main/kotlin/jetbrains/exodus/query/metadata/OModelMetaData.kt b/query/src/main/kotlin/jetbrains/exodus/query/metadata/OModelMetaData.kt index 3603b9712..e24fb9ecc 100644 --- a/query/src/main/kotlin/jetbrains/exodus/query/metadata/OModelMetaData.kt +++ b/query/src/main/kotlin/jetbrains/exodus/query/metadata/OModelMetaData.kt @@ -17,8 +17,6 @@ package jetbrains.exodus.query.metadata import com.orientechnologies.orient.core.db.ODatabaseSession import com.orientechnologies.orient.core.metadata.schema.OClass -import com.orientechnologies.orient.core.record.ODirection -import com.orientechnologies.orient.core.record.OVertex import jetbrains.exodus.entitystore.orientdb.* class OModelMetaData( @@ -54,28 +52,27 @@ class OModelMetaData( inClassName: String ): OClass { /** - * It is not enough to check the existence of the edge class. + * It is enough to check the existence of the edge class. * We reuse the same edge class for all the links with the same name. - * So, we have to check the existence of the edge class + in and out properties of the connected classes. */ - // edge class val edgeClassName = OVertexEntity.edgeClassName(linkName) val oClass = session.getClass(edgeClassName) - // out-property - val outClass = session.getClass(outClassName) ?: throw IllegalStateException("$outClassName not found. It must not ever happen.") - val outPropName = OVertex.getEdgeLinkFieldName(ODirection.OUT, edgeClassName) - val outProp = outClass.getProperty(outPropName) - // in-property - val inClass = session.getClass(inClassName) ?: throw IllegalStateException("$inClassName not found. It must not ever happen.") - val inPropName = OVertex.getEdgeLinkFieldName(ODirection.IN, edgeClassName) - val inProp = inClass.getProperty(inPropName) - if (oClass != null && outProp != null && inProp != null) { + if (oClass != null) { return oClass } return session.executeInASeparateSessionIfCurrentHasTransaction(dbProvider) { sessionToWork -> val link = LinkMetadata(name = linkName, outClassName = outClassName, inClassName = inClassName, AssociationEndCardinality._0_n) - val result = sessionToWork.addAssociation(link, indicesContainingLink = listOf(), applyLinkCardinality = true) + + /** + * We do not apply link cardinality because: + * 1. We do not have any cardinality restrictions for ad-hoc links. + * 2. Applying the cardinality causes adding extra properties to existing vertices, + * that in turn potentially causes OConcurrentModificationException in the original session. + * Keep in mind that we create this edge class (and those extra properties) in a separate session. + * So, there is an original session with the business logic that may fail if we change vertices here. + */ + val result = sessionToWork.addAssociation(link, indicesContainingLink = listOf(), applyLinkCardinality = false) sessionToWork.initializeIndices(result) sessionToWork.getClass(edgeClassName) ?: throw IllegalStateException("$edgeClassName not found, it must never happen") } diff --git a/query/src/main/kotlin/jetbrains/exodus/query/metadata/OrientDbSchemaInitializer.kt b/query/src/main/kotlin/jetbrains/exodus/query/metadata/OrientDbSchemaInitializer.kt index d864a27f4..64ee0fa72 100644 --- a/query/src/main/kotlin/jetbrains/exodus/query/metadata/OrientDbSchemaInitializer.kt +++ b/query/src/main/kotlin/jetbrains/exodus/query/metadata/OrientDbSchemaInitializer.kt @@ -353,33 +353,34 @@ internal class OrientDbSchemaInitializer( appendLine() withPadding { - // class1.prop1 -> edgeClass -> class2 - applyLink( + if (applyLinkCardinality) { + applyLinkCardinality( + edgeClass, + outClass, + link.cardinality, + inClass, + ) + } + applyIndices( link.name, edgeClass, outClass, - link.cardinality, - inClass, indicesContainingLink ) } } - private fun applyLink( - linkName: String, + private fun applyLinkCardinality( edgeClass: OClass, outClass: OClass, outCardinality: AssociationEndCardinality, inClass: OClass, - indicesContainingLink: List ) { val linkOutPropName = OVertex.getEdgeLinkFieldName(ODirection.OUT, edgeClass.name) append("outProp: ${outClass.name}.$linkOutPropName") val outProp = outClass.createLinkPropertyIfAbsent(linkOutPropName) - if (applyLinkCardinality) { - // applying cardinality only to out direct property - outProp.applyCardinality(outCardinality) - } + // applying cardinality only to out direct property + outProp.applyCardinality(outCardinality) appendLine() val linkInPropName = OVertex.getEdgeLinkFieldName(ODirection.IN, edgeClass.name) @@ -391,7 +392,14 @@ internal class OrientDbSchemaInitializer( * We do not apply cardinality for the in-properties because, we do not know if there is any restrictions. * Because AssociationEndCardinality describes the cardinality of a single end. * */ + } + private fun applyIndices( + linkName: String, + edgeClass: OClass, + outClass: OClass, + indicesContainingLink: List + ) { addIndex(linkUniqueIndex(edgeClass.name)) if (indicesContainingLink.isNotEmpty()) { diff --git a/query/src/test/kotlin/jetbrains/exodus/query/metadata/OModelMetaDataTest.kt b/query/src/test/kotlin/jetbrains/exodus/query/metadata/OModelMetaDataTest.kt index 8b6c42e74..a4ce078b6 100644 --- a/query/src/test/kotlin/jetbrains/exodus/query/metadata/OModelMetaDataTest.kt +++ b/query/src/test/kotlin/jetbrains/exodus/query/metadata/OModelMetaDataTest.kt @@ -286,8 +286,8 @@ class OModelMetaDataTest: OTestMixin { withSession { session -> // check that the necessary schema parts have been initialized - session.assertAssociationExists("type1", "type2", "link1", AssociationEndCardinality._0_n) - session.assertAssociationExists("type2", "type1", "link2", AssociationEndCardinality._0_n) + session.assertAssociationExists("type1", "type2", "link1", cardinality = null) + session.assertAssociationExists("type2", "type1", "link2", cardinality = null) /** * It is a tricky one. @@ -309,7 +309,40 @@ class OModelMetaDataTest: OTestMixin { // check that the necessary schema parts have been initialized withSession { session -> - session.assertAssociationExists("type2", "type1", "link1", AssociationEndCardinality._0_n) + session.assertAssociationExists("type2", "type1", "link1", cardinality = null) + } + } + + @Test + fun `adding new link type does not cause OConcurrentModificationException`() { + val model = oModel(orientDb.provider) { + entity("type1") + entity("type2") + } + // initialize the entity types + model.prepare() + val store = OPersistentEntityStore(orientDb.provider, orientDb.dbName, model) + + // entity1 has already existed for a while + val id1 = store.computeInTransaction { tx -> + val e1 = tx.newEntity("type1") + e1.setProperty("trista", "opca") + e1.id + } + + /** + * Initializing new links must not affect vertices. + * 1. All the business logic happens in session1. + * 1. Initializing new links happens in a separate session, session2. + * 2. Everything that happens in session2 is concurrent changes from the session1 point of view. + * 3. So, if the initialization of new links affects vertices, session1 will fail with OConcurrentModificationException. + */ + store.executeInTransaction { tx -> + val e1 = tx.getEntity(id1) + val e2 = tx.newEntity("type2") + e1.addLink("link1", e2) + e2.addLink("link2", e1) + e1.setProperty("trista", "drista") } } } diff --git a/query/src/test/kotlin/jetbrains/exodus/query/metadata/SchemaTestUtils.kt b/query/src/test/kotlin/jetbrains/exodus/query/metadata/SchemaTestUtils.kt index 2c172bb8c..a809a4db8 100644 --- a/query/src/test/kotlin/jetbrains/exodus/query/metadata/SchemaTestUtils.kt +++ b/query/src/test/kotlin/jetbrains/exodus/query/metadata/SchemaTestUtils.kt @@ -53,26 +53,28 @@ internal fun ODatabaseSession.assertAssociationExists( outClassName: String, inClassName: String, edgeName: String, - cardinality: AssociationEndCardinality? + cardinality: AssociationEndCardinality?, ) { val edgeClassName = edgeName.asEdgeClass val edgeClass = getClass(edgeClassName) val inClass = getClass(inClassName)!! val outClass = getClass(outClassName)!! - val outPropName = OVertex.getEdgeLinkFieldName(ODirection.OUT, edgeClassName) - val directOutProp = outClass.getProperty(outPropName)!! - assertEquals(OType.LINKBAG, directOutProp.type) - directOutProp.assertCardinality(cardinality) + assertTrue(edgeClass.areIndexed(OEdge.DIRECTION_IN, OEdge.DIRECTION_OUT)) - val inPropName = OVertex.getEdgeLinkFieldName(ODirection.IN, edgeClassName) - val directInProp = inClass.getProperty(inPropName)!! - assertEquals(OType.LINKBAG, directInProp.type) + if (cardinality != null) { + val outPropName = OVertex.getEdgeLinkFieldName(ODirection.OUT, edgeClassName) + val directOutProp = outClass.getProperty(outPropName)!! + assertEquals(OType.LINKBAG, directOutProp.type) + directOutProp.assertCardinality(cardinality) - assertTrue(edgeClass.areIndexed(OEdge.DIRECTION_IN, OEdge.DIRECTION_OUT)) + val inPropName = OVertex.getEdgeLinkFieldName(ODirection.IN, edgeClassName) + val directInProp = inClass.getProperty(inPropName)!! + assertEquals(OType.LINKBAG, directInProp.type) + } } -private fun OProperty.assertCardinality(cardinality: AssociationEndCardinality?) { +private fun OProperty.assertCardinality(cardinality: AssociationEndCardinality) { when (cardinality) { AssociationEndCardinality._0_1 -> { assertTrue(!this.isMandatory) @@ -97,12 +99,6 @@ private fun OProperty.assertCardinality(cardinality: AssociationEndCardinality?) assertTrue(this.min == "1") assertTrue(this.max == null) } - - null -> { - assertTrue(!this.isMandatory) - assertTrue(this.min == null) - assertTrue(this.max == null) - } } }