Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP : begin working on gridfs implementation #29

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 28 additions & 16 deletions gridfs/src/main/scala/GridFSTransport.scala
Original file line number Diff line number Diff line change
@@ -1,37 +1,49 @@
/*
* Copyright (C) 2018-2019 Zengularity SA (FaberNovel Technologies) <https://www.zengularity.com>
*/
// /*
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

// * Copyright (C) 2018-2019 Zengularity SA (FaberNovel Technologies) <https://www.zengularity.com>
// */
package com.zengularity.benji.gridfs

import reactivemongo.api.MongoConnection
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Il y a un import reactivemongo.api.X plus bas => import reactivemongo.api.{ MongoConnection, X}


import reactivemongo.api.gridfs.GridFS
import reactivemongo.api.BSONSerializationPack
import scala.concurrent.{ ExecutionContext, Future }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sur la forme, J'essaie de me tenir à un ordre pour les imports, en groupant d'abord les importe JVM/Java classique, après les imports scalalib (import scala...), les imports libs externes (ici reactivemongo), les imports internes/cross pkg (com.zengularity.benji...).

Adaptation des règles javastyle, c'est configurable dans Intellij (Order import)

import scala.util.{ Failure, Success, Try }

import com.zengularity.benji.exception.BenjiUnknownError
import scala.util.Success
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double import


final class GridFSTransport(driver: reactivemongo.api.MongoDriver, val gridfsdb: Future[GridFS[BSONSerializationPack.type]]) {
final class GridFSTransport(driver: reactivemongo.api.MongoDriver, connection: MongoConnection, mongoUri: MongoConnection.ParsedURI) {
def close(): Unit = {
driver.close()
}

def gridfs(prefix: String)(implicit ec: ExecutionContext): Future[GridFS[BSONSerializationPack.type]] = {
mongoUri.db match {
case Some(name) =>
for {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for est utile avec plus d'un <- (map), sinon un .map (plus explicite et concis dans ce cas)

db <- connection.database(name)
gridfs = GridFS[BSONSerializationPack.type](db, prefix)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quelle différence entre avoir cette ligne et avoir yield GridFS[BSONSerializationPack.type](db, prefix) ?

} yield gridfs

case None =>
Future.failed(new BenjiUnknownError(s"Couldn't get the db from $mongoUri"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

La formulation actuelle des erreurs est plutôt "Fails to ..."

}
}
}

object GridFSTransport {
def apply(uri: String, prefix: String)(implicit ec: ExecutionContext): GridFSTransport = {
def apply(uri: String)(implicit ec: ExecutionContext): Try[GridFSTransport] = {
val driver = new reactivemongo.api.MongoDriver

val gridfsdb = for {
mongoUri <- Future.fromTry(MongoConnection.parseURI(uri))
val res = for {
mongoUri <- MongoConnection.parseURI(uri)
con = driver.connection(mongoUri)
gridfs <- mongoUri.db match {
case Some(name) =>
con.database(name).map(db =>
GridFS[BSONSerializationPack.type](db, prefix))
case None => Future.failed(new BenjiUnknownError(s"Couldn't get the db from $mongoUri"))
}
} yield gridfs

new GridFSTransport(driver, gridfsdb)
} yield (mongoUri, con)
val (mongoUri, connection) = (res.map(_._1), res.map(_._2))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pourquoi sortir du for à partir de là ?

connection match {
case Success(connection) => new Success(GridFSTransport(driver, connection, mongoUri))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Success est une case class, pas de new (par ailleurs même pour les class on tend à fournir une factory/apply dans companion pour découpler l'init)

case Failure(_) => Failure[GridFSTransport](new BenjiUnknownError(s"Couldn't create the connection to $uri"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formulation message "Fails to ..."

}
}
}