2

I'm again seeking you to share your wisdom with me, the scala padawan!

I'm playing with reactive mongo in scala and while I was writting a test using scalatest, I faced the following issue.

First the code:

"delete" when {
  "passing an existent id" should {
    "succeed" in {
      val testRecord = TestRecord(someString)
      Await.result(persistenceService.persist(testRecord), Duration.Inf)

      Await.result(persistenceService.delete(testRecord.id), Duration.Inf)
      Thread.sleep(1000) // Why do I need that to make the test succeeds?

      val thrownException = intercept[RecordNotFoundException] {
        Await.result(persistenceService.read(testRecord.id), Duration.Inf)
      }
      thrownException.getMessage should include(testRecord._id.toString)
    }
  }
}

And the read and delete methods with the code initializing connection to db (part of the constructor):

class MongoPersistenceService[R](url: String, port: String, databaseName: String, collectionName: String) {
  val driver = MongoDriver()
  val parsedUri: Try[MongoConnection.ParsedURI] = MongoConnection.parseURI("%s:%s".format(url, port))
  val connection: Try[MongoConnection] = parsedUri.map(driver.connection)
  val mongoConnection = Future.fromTry(connection)
  def db: Future[DefaultDB] = mongoConnection.flatMap(_.database(databaseName))

  def collection: Future[BSONCollection] = db.map(_.collection(collectionName))
  def read(id: BSONObjectID): Future[R] = {
      val query = BSONDocument("_id" -> id)

      val readResult: Future[R] = for {
        coll <- collection
        record <- coll.find(query).requireOne[R]
      } yield record

      readResult.recover {
        case NoSuchResultException => throw RecordNotFoundException(id)
      }
  }
  def delete(id: BSONObjectID): Future[Unit] = {
    val query = BSONDocument("_id" -> id)

    // first read then call remove. Read will throw if not present
    read(id).flatMap { (_) => collection.map(coll => coll.remove(query)) }
  }
}

So to make my test pass, I had to had a Thread.sleep right after waiting for the delete to complete. Knowing this is evil usually punished by many whiplash, I want learn and find the proper fix here.

While trying other stuff, I found instead of waiting, entirely closing the connection to the db was also doing the trick...

What am I misunderstanding here? Should a connection to the db be opened and close for each call to it? And not do many actions like adding, removing, updating records with one connection?

Note that everything works fine when I remove the read call in my delete function. Also by closing the connection, I mean call close on the MongoDriver from my test and also stop and start again embed Mongo which I'm using in background.

Thanks for helping guys.

Jeep87c
  • 1,050
  • 16
  • 36

1 Answers1

2

Warning: this is a blind guess, I've no experience with MongoDB on Scala.

You may have forgotten to flatMap

Take a look at this bit:

collection.map(coll => coll.remove(query))

Since collection is Future[BSONCollection] per your code and remove returns Future[WriteResult] per doc, so actual type of this expression is Future[Future[WriteResult]].

Now, you have annotated your function as returning Future[Unit]. Scala often makes Unit as a return value by throwing away possibly meaningful values, which it does in your case:

read(id).flatMap { (_) =>
  collection.map(coll => {
    coll.remove(query) // we didn't wait for removal
    ()                 // before returning unit
  })
}

So your code should probably be

read(id).flatMap(_ => collection.flatMap(_.remove(query).map(_ => ())))

Or a for-comprehension:

for {
  _    <- read(id)
  coll <- collection
  _    <- coll.remove(query)
} yield ()

You can make Scala warn you about discarded values by adding a compiler flag (assuming SBT):

scalacOptions += "-Ywarn-value-discard" 
Oleg Pyzhcov
  • 7,323
  • 1
  • 18
  • 30
  • You just made my day! Thank you it works as expected. Your guess was right on it. I knew it was not waiting for remove but could not understand why. Thank you again, I learned something again about scala. – Jeep87c Jul 19 '17 at 10:01