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

make bool2Fox always explicit #2989

Merged
merged 3 commits into from Aug 2, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 2 additions & 3 deletions app/controllers/AnnotationController.scala
Expand Up @@ -88,7 +88,7 @@ class AnnotationController @Inject()(val messagesApi: MessagesApi)
annotation <- provideAnnotation(typ, id)(securedRequestToUserAwareRequest)
restrictions <- restrictionsFor(typ, id)(securedRequestToUserAwareRequest)
_ <- restrictions.allowUpdate(request.identity) ?~> Messages("notAllowed")
_ <- annotation.isRevertPossible ?~> Messages("annotation.revert.toOld")
_ <- bool2Fox(annotation.isRevertPossible) ?~> Messages("annotation.revert.toOld")
dataSet <- annotation.dataSet
dataStoreHandler <- dataSet.dataStoreHandler
newTracingReference <- dataStoreHandler.duplicateSkeletonTracing(annotation.tracing, Some(version.toString))
Expand Down Expand Up @@ -118,8 +118,7 @@ class AnnotationController @Inject()(val messagesApi: MessagesApi)

for {
annotation <- provideAnnotation(typ, id)(securedRequestToUserAwareRequest)
isAllowed <- isReopenAllowed(request.identity, annotation)
_ <- isAllowed.toFox ?~> "reopen.notAllowed"
_ <- Fox.assertTrue(isReopenAllowed(request.identity, annotation)) ?~> "reopen.notAllowed"
_ <- annotation.muta.reopen ?~> "annotation.invalid"
updatedAnnotation <- provideAnnotation(typ, id)(securedRequestToUserAwareRequest)
json <- updatedAnnotation.publicWrites(Some(request.identity))
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/Authentication.scala
Expand Up @@ -204,7 +204,7 @@ class Authentication @Inject()(

def autoLogin = Action.async { implicit request =>
for {
_ <- Play.configuration.getBoolean("application.authentication.enableDevAutoLogin").get ?~> Messages("error.notInDev")
_ <- bool2Fox(Play.configuration.getBoolean("application.authentication.enableDevAutoLogin").get) ?~> Messages("error.notInDev")
user <- UserService.defaultUser
authenticator <- env.authenticatorService.create(user.loginInfo)
value <- env.authenticatorService.init(authenticator)
Expand Down Expand Up @@ -408,8 +408,8 @@ class Authentication @Inject()(

private def creatingOrganizationsIsAllowed(requestingUser: Option[User]) = {
val noOrganizationPresent = InitialDataService.assertNoOrganizationsPresent
val configurationFlagSet = Play.configuration.getBoolean("application.allowOrganzationCreation").getOrElse(false) ?~> "allowOrganzationCreation.notEnabled"
val userIsSuperUser = requestingUser.exists(_.isSuperUser).toFox
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it correct that .toFox and bool2Fox have the same behavior? The only difference is that Boolean does not have a method called .toFox thats why it is implicitly converted to a Fox and then returns itself, while bool2Fox converts it explicitly?

Copy link
Member Author

Choose a reason for hiding this comment

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

.toFox is actually a method of Fox. This way, if called on any type that has an implicit conversion to Fox, that conversion will be triggered. This will now no longer work for booleans (as is intended).

val configurationFlagSet = bool2Fox(Play.configuration.getBoolean("application.allowOrganzationCreation").getOrElse(false)) ?~> "allowOrganzationCreation.notEnabled"
val userIsSuperUser = bool2Fox(requestingUser.exists(_.isSuperUser))

Fox.sequenceOfFulls(List(noOrganizationPresent, configurationFlagSet, userIsSuperUser)).map(_.headOption).toFox
}
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/DataSetController.scala
Expand Up @@ -126,7 +126,7 @@ class DataSetController @Inject()(val messagesApi: MessagesApi) extends Controll

def importDataSet(dataSetName: String) = SecuredAction.async { implicit request =>
for {
_ <- DataSetService.isProperDataSetName(dataSetName) ?~> Messages("dataSet.import.impossible.name")
_ <- bool2Fox(DataSetService.isProperDataSetName(dataSetName)) ?~> Messages("dataSet.import.impossible.name")
dataSet <- DataSetDAO.findOneByName(dataSetName) ?~> Messages("dataSet.notFound", dataSetName)
result <- DataSetService.importDataSet(dataSet)
} yield {
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/InitialDataController.scala
Expand Up @@ -80,13 +80,13 @@ Samplecountry

def assertInitialDataEnabled =
for {
_ <- Play.configuration.getBoolean("application.insertInitialData").getOrElse(false) ?~> "initialData.notEnabled"
_ <- bool2Fox(Play.configuration.getBoolean("application.insertInitialData").getOrElse(false)) ?~> "initialData.notEnabled"
} yield ()

def assertNoOrganizationsPresent =
for {
organizations <- OrganizationDAO.findAll
_ <- organizations.isEmpty ?~> "initialData.organizationsNotEmpty"
_ <- bool2Fox(organizations.isEmpty) ?~> "initialData.organizationsNotEmpty"
} yield ()

def insertDefaultUser = {
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/ProjectController.scala
Expand Up @@ -57,7 +57,7 @@ class ProjectController @Inject()(val messagesApi: MessagesApi) extends Controll
implicit request =>
for {
project <- ProjectDAO.findOneByName(projectName) ?~> Messages("project.notFound", projectName)
_ <- project.isDeletableBy(request.identity) ?~> Messages("project.remove.notAllowed")
_ <- bool2Fox(project.isDeletableBy(request.identity)) ?~> Messages("project.remove.notAllowed")
_ <- ProjectService.deleteOne(project._id) ?~> Messages("project.remove.failure")
} yield {
JsonOk(Messages("project.remove.success"))
Expand Down Expand Up @@ -127,7 +127,7 @@ class ProjectController @Inject()(val messagesApi: MessagesApi) extends Controll
def incrementEachTasksInstances(projectName: String, delta: Option[Long]) = SecuredAction.async {
implicit request =>
for {
_ <- (delta.getOrElse(1L) >= 0) ?~> Messages("project.increaseTaskInstances.negative")
_ <- bool2Fox(delta.getOrElse(1L) >= 0) ?~> Messages("project.increaseTaskInstances.negative")
project <- ProjectDAO.findOneByName(projectName) ?~> Messages("project.notFound", projectName)
_ <- TaskDAO.incrementTotalInstancesOfAllWithProject(project._id, delta.getOrElse(1L))
openInstanceCount <- TaskDAO.countOpenInstancesForProject(project._id)
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/ScriptController.scala
Expand Up @@ -22,7 +22,7 @@ class ScriptController @Inject()(val messagesApi: MessagesApi) extends Controlle
def create = SecuredAction.async(parse.json) { implicit request =>
withJsonBodyUsing(scriptPublicReads) { script =>
for {
_ <- request.identity.isAdmin ?~> Messages("notAllowed")
_ <- bool2Fox(request.identity.isAdmin) ?~> Messages("notAllowed")
_ <- ScriptDAO.insertOne(script)
js <- script.publicWrites
} yield {
Expand Down Expand Up @@ -55,7 +55,7 @@ class ScriptController @Inject()(val messagesApi: MessagesApi) extends Controlle
for {
scriptIdValidated <- ObjectId.parse(scriptId)
oldScript <- ScriptDAO.findOne(scriptIdValidated) ?~> Messages("script.notFound")
_ <- (oldScript._owner == request.identity._id) ?~> Messages("script.notOwner")
_ <- bool2Fox(oldScript._owner == request.identity._id) ?~> Messages("script.notOwner")
updatedScript = scriptFromForm.copy(_id = oldScript._id)
_ <- ScriptDAO.updateOne(updatedScript)
js <- updatedScript.publicWrites
Expand All @@ -69,7 +69,7 @@ class ScriptController @Inject()(val messagesApi: MessagesApi) extends Controlle
for {
scriptIdValidated <- ObjectId.parse(scriptId)
oldScript <- ScriptDAO.findOne(scriptIdValidated) ?~> Messages("script.notFound")
_ <- (oldScript._owner == request.identity._id) ?~> Messages("script.notOwner")
_ <- bool2Fox(oldScript._owner == request.identity._id) ?~> Messages("script.notOwner")
_ <- ScriptDAO.deleteOne(scriptIdValidated) ?~> Messages("script.removalFailed")
_ <- TaskDAO.removeScriptFromAllTasks(scriptIdValidated) ?~> Messages("script.removalFailed")
} yield {
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/UserController.scala
Expand Up @@ -194,7 +194,7 @@ class UserController @Inject()(val messagesApi: MessagesApi)
userIdValidated <- ObjectId.parse(userId)
user <- UserDAO.findOne(userIdValidated) ?~> Messages("user.notFound")
_ <- Fox.assertTrue(user.isEditableBy(request.identity)) ?~> Messages("notAllowed")
_ <- checkAdminOnlyUpdates(user, isActive, isAdmin, email)(issuingUser) ?~> Messages("notAllowed")
_ <- bool2Fox(checkAdminOnlyUpdates(user, isActive, isAdmin, email)(issuingUser)) ?~> Messages("notAllowed")
teams <- Fox.combined(assignedMemberships.map(t => TeamDAO.findOne(t.teamId)(GlobalAccessContext) ?~> Messages("team.notFound")))
oldTeamMemberships <- user.teamMemberships
teamsWithoutUpdate <- Fox.filterNot(oldTeamMemberships)(t => issuingUser.isTeamManagerOrAdminOf(t.teamId))
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/WKDataStoreController.scala
Expand Up @@ -29,9 +29,9 @@ class WKDataStoreController @Inject()(val messagesApi: MessagesApi)
def validateDataSetUpload(name: String) = DataStoreAction(name).async(parse.json) { implicit request =>
for {
uploadInfo <- request.body.validate[DataSourceId].asOpt.toFox ?~> Messages("dataStore.upload.invalid")
_ <- DataSetService.isProperDataSetName(uploadInfo.name) ?~> Messages("dataSet.name.invalid")
_ <- bool2Fox(DataSetService.isProperDataSetName(uploadInfo.name)) ?~> Messages("dataSet.name.invalid")
_ <- DataSetService.assertNewDataSetName(uploadInfo.name)(GlobalAccessContext) ?~> Messages("dataSet.name.alreadyTaken")
_ <- uploadInfo.team.nonEmpty ?~> Messages("team.invalid")
_ <- bool2Fox(uploadInfo.team.nonEmpty) ?~> Messages("team.invalid")
} yield Ok
}

Expand Down
Expand Up @@ -68,7 +68,7 @@ class WebknossosBearerTokenAuthenticatorService(settings: BearerTokenAuthenticat
def userForToken(tokenValue: String)(implicit ctx: DBAccessContext): Fox[User] =
for {
tokenAuthenticator <- dao.findOneByValue(tokenValue) ?~> Messages("auth.invalidToken")
_ <- (tokenAuthenticator.isValid) ?~> Messages("auth.invalidToken")
_ <- bool2Fox(tokenAuthenticator.isValid) ?~> Messages("auth.invalidToken")
user <- UserService.findOneByEmail(tokenAuthenticator.loginInfo.providerKey)
} yield user

Expand Down
9 changes: 5 additions & 4 deletions util/src/main/scala/com/scalableminds/util/tools/Fox.scala
Expand Up @@ -8,10 +8,6 @@ import scala.reflect.ClassTag
import scala.util.{Success, Try}

trait FoxImplicits {
implicit def bool2Fox(b: Boolean)(implicit ec: ExecutionContext): Fox[Boolean] =
if(b) Fox.successful(b)
else Fox.empty

implicit def futureBox2Fox[T](f: Future[Box[T]])(implicit ec: ExecutionContext): Fox[T] =
new Fox(f)

Expand Down Expand Up @@ -42,6 +38,11 @@ trait FoxImplicits {

implicit def fox2FutureBox[T](f: Fox[T])(implicit ec: ExecutionContext): Future[Box[T]] =
f.futureBox

// This one is no longer implicit since that has lead to confusion. Should always be used explicitly.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it wanted that it still belongs to FoxImplicits?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that way it is available to all the classes inheriting from that Trait

def bool2Fox(b: Boolean)(implicit ec: ExecutionContext): Fox[Boolean] =
if(b) Fox.successful(b)
else Fox.empty
}

object Fox extends FoxImplicits {
Expand Down