Merge pull request #206 from JetBrains/new-link-types-in-txs

Support adding new links in a transaction

entity-store/src/main/kotlin/jetbrains/exodus/entitystore/orientdb/OPersistentEntityStore.kt

@@ -119,14 +119,8 @@ class OPersistentEntityStore(
     }
 
     override fun executeInTransaction(executable: StoreTransactionalExecutable) {
-        //i'm not sure about implementation
-        val txn = beginTransaction() as OStoreTransactionImpl
-        try {
-            executable.execute(txn)
-            txn.commit()
-        } catch (e: Exception) {
-            txn.abort()
-            throw e
+        computeInTransaction { tx ->
+            executable.execute(tx)
         }
     }
 
@@ -137,15 +131,15 @@ class OPersistentEntityStore(
         executeInTransaction(executable)
 
     override fun <T : Any?> computeInTransaction(computable: StoreTransactionalComputable<T>): T {
-        //i'm not sure about implementation
-        val txn = beginTransaction() as OStoreTransactionImpl
+        val tx = beginTransaction() as OStoreTransactionImpl
         try {
-            val result = computable.compute(txn)
-            txn.commit()
+            val result = computable.compute(tx)
+            tx.commit()
             return result
-        } catch (e: Exception) {
-            txn.abort()
-            throw e
+        } finally {
+            if (!tx.isFinished) {
+                tx.abort()
+            }
         }
     }
 

entity-store/src/main/kotlin/jetbrains/exodus/entitystore/orientdb/OSchemaBuddy.kt

@@ -50,6 +50,8 @@ interface OSchemaBuddy {
     fun updateSequence(session: ODatabaseSession, sequenceName: String, currentValue: Long)
 
     fun renameOClass(session: ODatabaseSession, oldName: String, newName: String)
+
+    fun getOrCreateEdgeClass(session: ODatabaseSession, linkName: String, outClassName: String, inClassName: String): OClass
 }
 
 class OSchemaBuddyImpl(
@@ -83,26 +85,26 @@ class OSchemaBuddyImpl(
         val oSequence = session.metadata.sequenceLibrary.getSequence(sequenceName)
         if (oSequence != null) return oSequence
 
-        return executeInASeparateSessionIfCurrentHasTransaction(session) { sessionToWork ->
+        return session.executeInASeparateSessionIfCurrentHasTransaction(dbProvider) { sessionToWork ->
             val params = CreateParams().setStart(initialValue).setIncrement(1)
             sessionToWork.metadata.sequenceLibrary.createSequence(sequenceName, SEQUENCE_TYPE.ORDERED, params)
         }
     }
 
     override fun renameOClass(session: ODatabaseSession, oldName: String, newName: String) {
-        executeInASeparateSessionIfCurrentHasTransaction(session) { sessionToWork ->
+        session.executeInASeparateSessionIfCurrentHasTransaction(dbProvider) { sessionToWork ->
             val oldClass = sessionToWork.metadata.schema.getClass(oldName) ?: throw IllegalArgumentException("Class $oldName not found")
             oldClass.setName(newName)
         }
     }
 
-    private fun <T> executeInASeparateSessionIfCurrentHasTransaction(session: ODatabaseSession, action: (ODatabaseSession) -> T): T {
-        return if (session.hasActiveTransaction()) {
-            dbProvider.executeInASeparateSession(session) { newSession ->
-                action(newSession)
-            }
-        } else {
-            action(session)
+    override fun getOrCreateEdgeClass(session: ODatabaseSession, linkName: String, outClassName: String, inClassName: String): OClass {
+        val edgeClassName = OVertexEntity.edgeClassName(linkName)
+        val oClass = session.getClass(edgeClassName)
+        if (oClass != null) return oClass
+
+        return session.executeInASeparateSessionIfCurrentHasTransaction(dbProvider) { sessionToWork ->
+            sessionToWork.createEdgeClass(edgeClassName)
         }
     }
 
@@ -115,7 +117,7 @@ class OSchemaBuddyImpl(
     }
 
     override fun updateSequence(session: ODatabaseSession, sequenceName: String, currentValue: Long) {
-        executeInASeparateSessionIfCurrentHasTransaction(session) { sessionToWork ->
+        session.executeInASeparateSessionIfCurrentHasTransaction(dbProvider) { sessionToWork ->
             getSequence(sessionToWork, sequenceName).updateParams(CreateParams().setCurrentValue(currentValue))
         }
     }
@@ -152,6 +154,16 @@ class OSchemaBuddyImpl(
 
 }
 
+fun <T> ODatabaseSession.executeInASeparateSessionIfCurrentHasTransaction(dbProvider: ODatabaseProvider, action: (ODatabaseSession) -> T): T {
+    return if (this.hasActiveTransaction()) {
+        dbProvider.executeInASeparateSession(this) { newSession ->
+            action(newSession)
+        }
+    } else {
+        action(this)
+    }
+}
+
 fun ODatabaseSession.createClassIdSequenceIfAbsent(startFrom: Long = -1L) {
     createSequenceIfAbsent(CLASS_ID_SEQUENCE_NAME, startFrom)
 }

entity-store/src/main/kotlin/jetbrains/exodus/entitystore/orientdb/OStoreTransaction.kt

@@ -15,6 +15,7 @@
  */
 package jetbrains.exodus.entitystore.orientdb
 
+import com.orientechnologies.orient.core.metadata.schema.OClass
 import com.orientechnologies.orient.core.metadata.sequence.OSequence
 import com.orientechnologies.orient.core.record.ORecord
 import com.orientechnologies.orient.core.record.OVertex
@@ -59,4 +60,6 @@ interface OStoreTransaction : StoreTransaction {
     fun updateOSequence(sequenceName: String, currentValue: Long)
 
     fun renameOClass(oldName: String, newName: String)
+
+    fun getOrCreateEdgeClass(linkName: String, outClassName: String, inClassName: String): OClass
 }

entity-store/src/main/kotlin/jetbrains/exodus/entitystore/orientdb/OStoreTransactionImpl.kt

@@ -17,6 +17,7 @@ package jetbrains.exodus.entitystore.orientdb
 
 import com.orientechnologies.orient.core.db.ODatabase
 import com.orientechnologies.orient.core.db.ODatabaseSession
+import com.orientechnologies.orient.core.metadata.schema.OClass
 import com.orientechnologies.orient.core.metadata.sequence.OSequence
 import com.orientechnologies.orient.core.record.ORecord
 import com.orientechnologies.orient.core.record.OVertex
@@ -192,6 +193,7 @@ class OStoreTransactionImpl(
 
     override fun newEntity(entityType: String, localEntityId: Long): OVertexEntity {
         requireActiveWritableTransaction()
+        schemaBuddy.requireTypeExists(session, entityType)
         val vertex = session.newVertex(entityType)
         vertex.setProperty(LOCAL_ENTITY_ID_PROPERTY_NAME, localEntityId)
         vertex.save<OVertex>()
@@ -400,6 +402,11 @@ class OStoreTransactionImpl(
         schemaBuddy.renameOClass(session, oldName, newName)
     }
 
+    override fun getOrCreateEdgeClass(linkName: String, outClassName: String, inClassName: String): OClass {
+        requireActiveTransaction()
+        return schemaBuddy.getOrCreateEdgeClass(session, linkName, outClassName, inClassName)
+    }
+
     override fun setQueryCancellingPolicy(policy: QueryCancellingPolicy?) {
         require(policy is OQueryCancellingPolicy) { "Only OQueryCancellingPolicy is supported, but was ${policy?.javaClass?.simpleName}" }
         this.queryCancellingPolicy = policy

entity-store/src/main/kotlin/jetbrains/exodus/entitystore/orientdb/OVertexEntity.kt

@@ -15,7 +15,6 @@
  */
 package jetbrains.exodus.entitystore.orientdb
 
-import com.orientechnologies.orient.core.db.ODatabaseSession
 import com.orientechnologies.orient.core.db.record.OTrackedSet
 import com.orientechnologies.orient.core.db.record.ridbag.ORidBag
 import com.orientechnologies.orient.core.id.ORID
@@ -259,9 +258,9 @@ open class OVertexEntity(internal val vertex: OVertex, private val store: OEntit
     // Add links
 
     override fun addLink(linkName: String, target: Entity): Boolean {
-        requireActiveWritableTransaction()
+        val currentTx = requireActiveWritableTransaction()
         require(target is OVertexEntity) { "Only OVertexEntity is supported, but was ${target.javaClass.simpleName}" }
-        return addLinkImpl(linkName, target.vertex)
+        return currentTx.addLinkImpl(linkName, target.vertex)
     }
 
     override fun addLink(linkName: String, targetId: EntityId): Boolean {
@@ -271,12 +270,14 @@ open class OVertexEntity(internal val vertex: OVertex, private val store: OEntit
             return false
         }
         val target = currentTx.getRecord<OVertex>(targetOId) ?: return false
-        return addLinkImpl(linkName, target)
+        return currentTx.addLinkImpl(linkName, target)
     }
 
-    private fun addLinkImpl(linkName: String, target: OVertex): Boolean {
+    private fun OStoreTransaction.addLinkImpl(linkName: String, target: OVertex): Boolean {
+        val outClassName = vertex.requireSchemaClass().name
+        val inClassName = target.requireSchemaClass().name
+        val edgeClass = getOrCreateEdgeClass(linkName, outClassName, inClassName)
         val edgeClassName = edgeClassName(linkName)
-        val edgeClass = ODatabaseSession.getActiveSession().getClass(edgeClassName) ?: throw IllegalStateException("$edgeClassName edge class not found in the database. Sorry, pal, it is required for adding a link.")
 
         /*
         We check for duplicates only if there is an appropriate index for it.
@@ -318,16 +319,16 @@ open class OVertexEntity(internal val vertex: OVertex, private val store: OEntit
     // Delete links
 
     override fun deleteLink(linkName: String, target: Entity): Boolean {
-        requireActiveWritableTransaction()
+        val currentTx = requireActiveWritableTransaction()
         target as OVertexEntity
         val targetOId = target.oEntityId.asOId()
-        return deleteLinkImpl(linkName, targetOId)
+        return currentTx.deleteLinkImpl(linkName, targetOId)
     }
 
     override fun deleteLink(linkName: String, targetId: EntityId): Boolean {
-        requireActiveWritableTransaction()
+        val currentTx = requireActiveWritableTransaction()
         val targetOId = store.requireOEntityId(targetId).asOId()
-        return deleteLinkImpl(linkName, targetOId)
+        return currentTx.deleteLinkImpl(linkName, targetOId)
     }
 
     override fun deleteLinks(linkName: String) {
@@ -340,7 +341,7 @@ open class OVertexEntity(internal val vertex: OVertex, private val store: OEntit
         vertex.save<OVertex>()
     }
 
-    private fun deleteLinkImpl(linkName: String, targetId: ORID): Boolean {
+    private fun OStoreTransaction.deleteLinkImpl(linkName: String, targetId: ORID): Boolean {
         val edgeClassName = edgeClassName(linkName)
 
         val edge = findEdge(edgeClassName, targetId)
@@ -375,9 +376,9 @@ open class OVertexEntity(internal val vertex: OVertex, private val store: OEntit
     // Set links
 
     override fun setLink(linkName: String, target: Entity?): Boolean {
-        requireActiveWritableTransaction()
+        val currentTx = requireActiveWritableTransaction()
         require(target is OVertexEntity?) { "Only OVertexEntity is supported, but was ${target?.javaClass?.simpleName}" }
-        return setLinkImpl(linkName, target?.vertex)
+        return currentTx.setLinkImpl(linkName, target?.vertex)
     }
 
     override fun setLink(linkName: String, targetId: EntityId): Boolean {
@@ -387,10 +388,10 @@ open class OVertexEntity(internal val vertex: OVertex, private val store: OEntit
             return false
         }
         val target = currentTx.getRecord<OVertex>(targetOId) ?: return false
-        return setLinkImpl(linkName, target)
+        return currentTx.setLinkImpl(linkName, target)
     }
 
-    private fun setLinkImpl(linkName: String, target: OVertex?): Boolean {
+    private fun OStoreTransaction.setLinkImpl(linkName: String, target: OVertex?): Boolean {
         val currentLink = getLinkImpl(linkName)
 
         if (currentLink == target) {
@@ -445,9 +446,9 @@ open class OVertexEntity(internal val vertex: OVertex, private val store: OEntit
             .map { it.substringBefore(EDGE_CLASS_SUFFIX) })
     }
 
-    private fun findEdge(edgeClassName: String, targetId: ORID): OEdge? {
+    private fun OStoreTransaction.findEdge(edgeClassName: String, targetId: ORID): OEdge? {
         val query = "SELECT FROM $edgeClassName WHERE ${OEdge.DIRECTION_OUT} = :outId AND ${OEdge.DIRECTION_IN} = :inId"
-        val result = ODatabaseSession.getActiveSession().query(query, mapOf("outId" to vertex.identity, "inId" to targetId))
+        val result = query(query, mapOf("outId" to vertex.identity, "inId" to targetId))
         val foundEdge = result.edgeStream().findFirst()
         return foundEdge.getOrNull()
     }

entity-store/src/test/kotlin/jetbrains/exodus/entitystore/orientdb/ODatabaseProviderTest.kt

@@ -34,19 +34,21 @@ class ODatabaseProviderTest: OTestMixin {
     @Test
     fun `read-only mode works`() {
         // by default it is read-write
-        withSession { session ->
+        withTxSession { session ->
             val v = session.newVertex()
             v.save<OVertex>()
         }
 
         orientDb.provider.readOnly = true
-        withSession { session ->
-            val v = session.newVertex()
-            assertFailsWith<OModificationOperationProhibitedException> { v.save<OVertex>() }
+        assertFailsWith<OModificationOperationProhibitedException> {
+            withTxSession { session ->
+                val v = session.newVertex()
+                v.save<OVertex>()
+            }
         }
 
         orientDb.provider.readOnly = false
-        withSession { session ->
+        withTxSession { session ->
             val v = session.newVertex()
             v.save<OVertex>()
         }

entity-store/src/test/kotlin/jetbrains/exodus/entitystore/orientdb/OEntityTest.kt

@@ -19,9 +19,7 @@ import com.orientechnologies.orient.core.metadata.schema.OType
 import jetbrains.exodus.entitystore.EntityRemovedInDatabaseException
 import jetbrains.exodus.entitystore.PersistentEntityId
 import jetbrains.exodus.entitystore.orientdb.OVertexEntity.Companion.linkTargetEntityIdPropertyName
-import jetbrains.exodus.entitystore.orientdb.testutil.InMemoryOrientDB
-import jetbrains.exodus.entitystore.orientdb.testutil.Issues
-import jetbrains.exodus.entitystore.orientdb.testutil.createIssue
+import jetbrains.exodus.entitystore.orientdb.testutil.*
 import jetbrains.exodus.entitystore.orientdb.testutil.createIssueImpl
 import org.junit.Rule
 import org.junit.Test
@@ -30,11 +28,13 @@ import java.util.*
 import kotlin.random.Random
 import kotlin.test.*
 
-class OEntityTest {
+class OEntityTest: OTestMixin {
 
     @Rule
     @JvmField
-    val orientDb = InMemoryOrientDB()
+    val orientDbRule = InMemoryOrientDB()
+
+    override val orientDb = orientDbRule
 
     @Test
     fun `create entities`() {
@@ -633,4 +633,14 @@ class OEntityTest {
         assertEquals(1001, localIdSet.size)
         assertEquals(1, typeIdSet.size)
     }
+
+    @Test
+    fun `add new link types in a transaction`() {
+        withStoreTx { tx ->
+            val iss1 = tx.createIssue("iss1")
+            val iss2 = tx.createIssue("iss2")
+
+            iss1.addLink("trista", iss2)
+        }
+    }
 }

entity-store/src/test/kotlin/jetbrains/exodus/entitystore/orientdb/OPersistentStoreTest.kt

@@ -15,9 +15,13 @@
  */
 package jetbrains.exodus.entitystore.orientdb
 
+import com.orientechnologies.orient.core.metadata.schema.OClass
+import com.orientechnologies.orient.core.metadata.schema.OType
 import com.orientechnologies.orient.core.record.OVertex
+import com.orientechnologies.orient.core.storage.ORecordDuplicatedException
 import jetbrains.exodus.entitystore.EntityRemovedInDatabaseException
 import jetbrains.exodus.entitystore.PersistentEntityId
+import jetbrains.exodus.entitystore.StoreTransaction
 import jetbrains.exodus.entitystore.orientdb.testutil.InMemoryOrientDB
 import jetbrains.exodus.entitystore.orientdb.testutil.Issues
 import jetbrains.exodus.entitystore.orientdb.testutil.Issues.CLASS
@@ -218,4 +222,39 @@ class OPersistentStoreTest: OTestMixin {
             assertEquals(ORIDEntityId.EMPTY_ID, orientDb.store.requireOEntityId(PersistentEntityId.EMPTY_ID))
         }
     }
+
+    @Test
+    fun `computeInTransaction and Co handle exceptions properly`() {
+        withSession { session ->
+            val t1 = session.getOrCreateVertexClass("type1")
+            t1.createProperty("name", OType.STRING)
+            t1.createIndex("opca_index", OClass.INDEX_TYPE.UNIQUE, "name")
+        }
+        fun StoreTransaction.violateIndexRestriction() {
+            val e1 = this.newEntity("type1")
+            val e2 = this.newEntity("type1")
+            e1.setProperty("name", "trista")
+            e2.setProperty("name", "trista")
+        }
+        /**
+         * Here we check that nothing happens with the exception on the way up.
+         * Our code that finishes the transaction must work correctly if there is no active session.
+         */
+        val store = orientDb.store
+        assertFailsWith<ORecordDuplicatedException> {
+            store.computeInTransaction { tx ->
+                tx.violateIndexRestriction()
+            }
+        }
+        assertFailsWith<ORecordDuplicatedException> {
+            store.executeInTransaction { tx ->
+                tx.violateIndexRestriction()
+            }
+        }
+        assertFailsWith<ORecordDuplicatedException> {
+            withStoreTx { tx ->
+                tx.violateIndexRestriction()
+            }
+        }
+    }
 }