Skip to content

Commit

Permalink
Bugfix/spline 1266 tx info discrepancy and index (#1267)
Browse files Browse the repository at this point in the history
* issue #1266 Rename wrongly named property `_tx_info` to correct `_txInfo`

* issue #1266 Delete documents of the rolling back transaction by comparing the `_txInfo.uid` property only (no need to look at other '_txInfo.*' props)

* issue #1266 Add persistent index on `_txInfo.uid` property to all nodes and edges

* issue #1266 Fix some ArangoDb driver deprecations

* issue #1266 Minor refactoring: rename "ArangoManager.initialize()" to "createDatabase()"

* issue #1266 Add tests for "ArangoManager.createDatabase"
  • Loading branch information
wajda authored Sep 6, 2023
1 parent a550061 commit 3143bbc
Show file tree
Hide file tree
Showing 13 changed files with 262 additions and 56 deletions.
6 changes: 5 additions & 1 deletion .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,12 @@ max_line_length = 140

[*.scala]
indent_size = 2
ij_scala_indent_first_parameter = false
ij_scala_indent_first_parameter_clause = true
ij_scala_method_parameters_new_line_after_left_paren = true
ij_scala_method_parameters_right_paren_on_new_line = true

[{*.ts, *.js}]
[{*.ts,*.js}]
indent_size = 4

# quotes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ class AdminCLI(dbManagerFactory: ArangoManagerFactory, maybeConsole: Option[Inpu
case _ => Fail
}
val dbManager = interactiveDbManagerFactory.create(url, sslCtxOpt, conf.dryRun)
val wasInitialized = Await.result(dbManager.initialize(onExistsAction, options), Duration.Inf)
val wasInitialized = Await.result(dbManager.createDatabase(onExistsAction, options), Duration.Inf)
if (!wasInitialized) println(ansi"%yellow{Skipped. DB is already initialized}")

case DBUpgrade(url) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ trait ArangoManager {
/**
* @return `true` if actual initialization was performed.
*/
def initialize(onExistsAction: OnDBExistsAction, options: DatabaseCreateOptions): Future[Boolean]
def createDatabase(onExistsAction: OnDBExistsAction, options: DatabaseCreateOptions): Future[Boolean]
def upgrade(): Future[Unit]
def execute(actions: AuxiliaryDBAction*): Future[Unit]
def prune(retentionPeriod: Duration): Future[Unit]
Expand All @@ -65,7 +65,7 @@ class ArangoManagerImpl(

import ArangoManagerImpl._

def initialize(onExistsAction: OnDBExistsAction, options: DatabaseCreateOptions): Future[Boolean] = {
def createDatabase(onExistsAction: OnDBExistsAction, options: DatabaseCreateOptions): Future[Boolean] = {
log.debug("Initialize database")
db.exists.toScala.flatMap { exists =>
if (exists && onExistsAction == Skip) {
Expand Down Expand Up @@ -144,17 +144,17 @@ class ArangoManagerImpl(
for {
exists <- db.exists.toScala
_ <- if (exists && !dropIfExists)
throw new IllegalArgumentException(s"Arango Database ${db.dbName} already exists")
throw new IllegalArgumentException(s"Arango Database ${db.name} already exists")
else if (exists && dropIfExists) {
log.info(s"Drop database: ${db.dbName}")
log.info(s"Drop database: ${db.name}")
unlessDryRunAsync(db.drop().toScala)
}
else Future.successful({})
} yield {}
}

private def createDb() = {
log.info(s"Create database: ${db.dbName}")
log.info(s"Create database: ${db.name}")
unlessDryRunAsync(db.create().toScala)
}

Expand Down Expand Up @@ -211,18 +211,19 @@ class ArangoManagerImpl(
Future.sequence(
for {
colDef <- sealedInstancesOf[CollectionDef]
idxDef <- colDef.indexDefs
idxDef <- colDef.indexDefs ++ colDef.commonIndexDefs
} yield {
val idxOpts = idxDef.options
log.debug(s"Ensure ${idxOpts.indexType} index: ${colDef.name} [${idxDef.fields.mkString(",")}]")
val dbCol = db.collection(colDef.name)
val fields = idxDef.fields.asJava
unlessDryRunAsync {
(idxOpts match {
case opts: FulltextIndexOptions => dbCol.ensureFulltextIndex(fields, opts)
case opts: GeoIndexOptions => dbCol.ensureGeoIndex(fields, opts)
case opts: InvertedIndexOptions => dbCol.ensureInvertedIndex(opts)
case opts: PersistentIndexOptions => dbCol.ensurePersistentIndex(fields, opts)
case opts: TtlIndexOptions => dbCol.ensureTtlIndex(fields, opts)
case opts: ZKDIndexOptions => dbCol.ensureZKDIndex(fields, opts)
}).toScala
}
})
Expand Down Expand Up @@ -279,7 +280,7 @@ class ArangoManagerImpl(
log.debug(s"Delete search analyzers")
for {
analyzers <- db.getSearchAnalyzers.toScala.map(_.asScala)
userAnalyzers = analyzers.filter(_.getName.startsWith(s"${db.dbName}::"))
userAnalyzers = analyzers.filter(_.getName.startsWith(s"${db.name}::"))
_ <- Future.traverse(userAnalyzers)(ua => {
log.info(s"Delete search analyzer: ${ua.getName}")
unlessDryRunAsync(db.deleteSearchAnalyzer(ua.getName).toScala)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class AdminCLISpec
thenReturn arangoManagerMock)

(when(
arangoManagerMock.initialize(actionFlgCaptor.capture, dbOptionCaptor.capture))
arangoManagerMock.createDatabase(actionFlgCaptor.capture, dbOptionCaptor.capture))
thenReturn Future.successful(true))

(when(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@

package za.co.absa.spline.arango

import com.arangodb.async.ArangoDatabaseAsync
import com.arangodb.async.{ArangoCollectionAsync, ArangoDatabaseAsync}
import com.arangodb.entity._
import com.arangodb.entity.arangosearch.analyzer.SearchAnalyzer
import org.mockito.ArgumentMatchers._
import org.mockito.Mockito.{times, verify, when}
import org.mockito.{ArgumentMatchers, Mockito}
import org.mockito.Mockito._
import org.mockito.{ArgumentCaptor, ArgumentMatchers, Mockito}
import org.scalatest.concurrent.ScalaFutures.whenReady
import org.scalatest.flatspec.AsyncFlatSpec
import org.scalatest.matchers.should.Matchers
Expand All @@ -29,10 +31,14 @@ import org.scalatestplus.mockito.MockitoSugar.mock
import za.co.absa.commons.version.impl.SemVer20Impl.SemanticVersion
import za.co.absa.spline.arango.ArangoManagerImplSpec._
import za.co.absa.spline.arango.foxx.FoxxManager
import za.co.absa.spline.dummy
import za.co.absa.spline.persistence.DatabaseVersionManager
import za.co.absa.spline.persistence.migration.Migrator

import java.lang.Boolean.{FALSE, TRUE}
import java.time.{Clock, Instant, ZoneId, ZonedDateTime}
import java.util.concurrent.CompletableFuture.completedFuture
import scala.collection.JavaConverters._
import scala.concurrent.duration._
import scala.concurrent.{ExecutionContext, Future}

Expand All @@ -44,9 +50,165 @@ class ArangoManagerImplSpec

import za.co.absa.commons.version.Version._

behavior of "createDatabase()"

it should "create database with all components when database does NOT exist" in {
val dbMock = mock[ArangoDatabaseAsync]
val dbVerMgrMock = mock[DatabaseVersionManager]
val foxxMgrMock = mock[FoxxManager]
val colMock = mock[ArangoCollectionAsync]

val createdColNamesCaptor: ArgumentCaptor[String] = ArgumentCaptor.forClass(classOf[String])

when(dbMock.exists()) thenReturn completedFuture(FALSE)
when(dbMock.create()) thenReturn completedFuture(TRUE)
when(dbMock.createCollection(createdColNamesCaptor.capture(), any())) thenReturn completedFuture(dummy[CollectionEntity])
when(dbMock.createSearchAnalyzer(any())) thenReturn completedFuture(dummy[SearchAnalyzer])
when(dbMock.createArangoSearch(any(), any())) thenReturn completedFuture(dummy[ViewEntity])
when(dbMock.collection(any())) thenReturn colMock

when(dbVerMgrMock.insertDbVersion(any())) thenReturn Future.successful(dummy[SemanticVersion])

when(colMock.insertDocuments(any())) thenReturn completedFuture(dummy[MultiDocumentEntity[DocumentCreateEntity[Nothing]]])
when(colMock.ensurePersistentIndex(any(), any())) thenReturn completedFuture(dummy[IndexEntity])

when(foxxMgrMock.install(any(), any())) thenReturn Future.successful(())

val manager = newManager(
db = dbMock,
foxxManager = foxxMgrMock,
dbVersionManager = dbVerMgrMock
)

for {
_ <- manager.createDatabase(OnDBExistsAction.Skip, DatabaseCreateOptions())
} yield {
val inOrder = Mockito.inOrder(dbMock)

inOrder.verify(dbMock, atLeastOnce()).exists()
inOrder.verify(dbMock, times(1)).create()
inOrder.verify(dbMock, times(24)).createCollection(any(), any())
createdColNamesCaptor.getAllValues.asScala.toSet should ((have size 24) and (contain allOf("txInfo", "dbVersion", "counter")))
inOrder.verify(dbMock, times(1)).createSearchAnalyzer(any())
inOrder.verify(dbMock, times(2)).createArangoSearch(any(), any())

verify(dbVerMgrMock).insertDbVersion(notNull())
verify(foxxMgrMock).install(ArgumentMatchers.eq("/spline"), notNull())

verify(colMock, atLeastOnce()).insertDocuments(notNull())
verify(colMock, atLeastOnce()).ensurePersistentIndex(any(), any())

Assertions.succeed
}
}

it should "create database with all components when database EXISTS and 'Drop' option is used" in {
val dbMock = mock[ArangoDatabaseAsync]
val dbVerMgrMock = mock[DatabaseVersionManager]
val foxxMgrMock = mock[FoxxManager]
val colMock = mock[ArangoCollectionAsync]

val createdColNamesCaptor: ArgumentCaptor[String] = ArgumentCaptor.forClass(classOf[String])

when(dbMock.exists()) thenReturn completedFuture(TRUE)
when(dbMock.create()) thenReturn completedFuture(TRUE)
when(dbMock.drop()) thenReturn completedFuture(TRUE)
when(dbMock.createCollection(createdColNamesCaptor.capture(), any())) thenReturn completedFuture(dummy[CollectionEntity])
when(dbMock.createSearchAnalyzer(any())) thenReturn completedFuture(dummy[SearchAnalyzer])
when(dbMock.createArangoSearch(any(), any())) thenReturn completedFuture(dummy[ViewEntity])
when(dbMock.collection(any())) thenReturn colMock

when(dbVerMgrMock.insertDbVersion(any())) thenReturn Future.successful(dummy[SemanticVersion])

when(colMock.insertDocuments(any())) thenReturn completedFuture(dummy[MultiDocumentEntity[DocumentCreateEntity[Nothing]]])
when(colMock.ensurePersistentIndex(any(), any())) thenReturn completedFuture(dummy[IndexEntity])

when(foxxMgrMock.install(any(), any())) thenReturn Future.successful(())

val manager = newManager(
db = dbMock,
foxxManager = foxxMgrMock,
dbVersionManager = dbVerMgrMock
)

for {
_ <- manager.createDatabase(OnDBExistsAction.Drop, DatabaseCreateOptions())
} yield {
val inOrder = Mockito.inOrder(dbMock)
inOrder.verify(dbMock, atLeastOnce()).exists()
inOrder.verify(dbMock).drop()
inOrder.verify(dbMock).create()
Assertions.succeed
}
}

it should "skip creation of database when database EXISTS and 'Skip' option is used" in {
val dbMock = mock[ArangoDatabaseAsync]
val dbVerMgrMock = mock[DatabaseVersionManager]
val foxxMgrMock = mock[FoxxManager]
val colMock = mock[ArangoCollectionAsync]

when(dbMock.exists()) thenReturn completedFuture(TRUE)

val manager = newManager(
db = dbMock,
foxxManager = foxxMgrMock,
dbVersionManager = dbVerMgrMock
)

for {
_ <- manager.createDatabase(OnDBExistsAction.Skip, DatabaseCreateOptions())
} yield {
verify(dbMock).exists()
verify(dbMock, never()).drop()
verify(dbMock, never()).create()
verify(dbMock, never()).createCollection(any(), any())
verify(dbMock, never()).createSearchAnalyzer(any())
verify(dbMock, never()).createArangoSearch(any(), any())
verify(dbVerMgrMock, never()).insertDbVersion(any())
verify(foxxMgrMock, never()).install(any(), any())
verify(colMock, never()).insertDocuments(any())
verify(colMock, never()).ensurePersistentIndex(any(), any())
Assertions.succeed
}
}


it should "fail to create database when database EXISTS and 'Fail' option is used" in {
val dbMock = mock[ArangoDatabaseAsync]
val dbVerMgrMock = mock[DatabaseVersionManager]
val foxxMgrMock = mock[FoxxManager]
val colMock = mock[ArangoCollectionAsync]

when(dbMock.exists()) thenReturn completedFuture(TRUE)

val manager = newManager(
db = dbMock,
foxxManager = foxxMgrMock,
dbVersionManager = dbVerMgrMock
)

for {
_ <- recoverToSucceededIf[IllegalArgumentException](manager.createDatabase(OnDBExistsAction.Fail, DatabaseCreateOptions()))
} yield {
verify(dbMock, atLeastOnce()).exists()
verify(dbMock, never()).drop()
verify(dbMock, never()).create()
verify(dbMock, never()).createCollection(any(), any())
verify(dbMock, never()).createSearchAnalyzer(any())
verify(dbMock, never()).createArangoSearch(any(), any())
verify(dbVerMgrMock, never()).insertDbVersion(any())
verify(foxxMgrMock, never()).install(any(), any())
verify(colMock, never()).insertDocuments(any())
verify(colMock, never()).ensurePersistentIndex(any(), any())
Assertions.succeed
}
}


behavior of "upgrade()"

it should "call foxx manager and migrator properly in order" in {
it should "call Foxx manager and Migrator properly and in the correct order" in {
val dbVerMgrMock = mock[DatabaseVersionManager]
val migratorMock = mock[Migrator]
val foxxMgrMock = mock[FoxxManager]
Expand All @@ -69,9 +231,9 @@ class ArangoManagerImplSpec
.thenReturn(Future.successful(true))

val manager = newManager(
migratorMock = migratorMock,
dbVersionManagerMock = dbVerMgrMock,
foxxManagerMock = foxxMgrMock,
migrator = migratorMock,
dbVersionManager = dbVerMgrMock,
foxxManager = foxxMgrMock,
appDbVersion = semver"4.5.6")

for {
Expand All @@ -87,7 +249,7 @@ class ArangoManagerImplSpec
}
}

it should "fail when foxx services failed to install" in {
it should "fail when Foxx services failed to install" in {
val dbVerMgrMock = mock[DatabaseVersionManager]
val migratorMock = mock[Migrator]
val foxxMgrMock = mock[FoxxManager]
Expand All @@ -105,9 +267,9 @@ class ArangoManagerImplSpec
.thenReturn(Future.successful(true))

val manager = newManager(
migratorMock = migratorMock,
dbVersionManagerMock = dbVerMgrMock,
foxxManagerMock = foxxMgrMock,
migrator = migratorMock,
dbVersionManager = dbVerMgrMock,
foxxManager = foxxMgrMock,
appDbVersion = semver"4.5.6")

for {
Expand Down Expand Up @@ -162,19 +324,20 @@ class ArangoManagerImplSpec

object ArangoManagerImplSpec {
private def newManager(
drmMock: DataRetentionManager = null, // NOSONAR
clock: Clock = null, // NOSONAR
migratorMock: Migrator = null, // NOSONAR
foxxManagerMock: FoxxManager = null, // NOSONAR
dbVersionManagerMock: DatabaseVersionManager = null, // NOSONAR
appDbVersion: SemanticVersion = null // NOSONAR
db: ArangoDatabaseAsync = mock[ArangoDatabaseAsync],
drmMock: DataRetentionManager = mock[DataRetentionManager],
clock: Clock = mock[Clock],
migrator: Migrator = mock[Migrator],
foxxManager: FoxxManager = mock[FoxxManager],
dbVersionManager: DatabaseVersionManager = mock[DatabaseVersionManager],
appDbVersion: SemanticVersion = mock[SemanticVersion]
)(implicit ec: ExecutionContext): ArangoManagerImpl = {
new ArangoManagerImpl(
mock[ArangoDatabaseAsync],
dbVersionManagerMock,
db,
dbVersionManager,
drmMock,
migratorMock,
foxxManagerMock,
migrator,
foxxManager,
clock,
appDbVersion,
dryRun = false)
Expand Down
30 changes: 30 additions & 0 deletions admin/src/test/scala/za/co/absa/spline/package.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright 2023 ABSA Group Limited
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package za.co.absa

import org.scalatestplus.mockito.MockitoSugar.mock

import scala.reflect.ClassTag

package object spline {
/**
* This is just to add another word for `Mockito.mock` method, for better test readability.
*
* @tparam A type of the dummy
* @return a dummy instance of the given type
*/
def dummy[A <: AnyRef : ClassTag]: A = mock[A]
}
Loading

0 comments on commit 3143bbc

Please sign in to comment.