Overriding extractors at the space level when they are enabled at the global level. (#74)

* Adding support for overriding extractors at the space level when they are enabled at the global level.

* Switched to using radio buttons and icons for the page to enable/disable extractors at the space level.

* Fixed accidental commit.

* Added a default radio button for extractors in space to follow the global default.
Order extractors by name in Space Extractors page.
Moved radio buttons to the end of the form to make it easier to read.

* Changed table header for space extractors to "Enabled by SuperAdmin" to be more clear.

Added some titles text to explain each column.

* Added update-space-extractors-selection Mongo update script.

* Updated CHANGELOG.md.

Author: lmarini

CHANGELOG.md

@@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ## [Unreleased]
 
 ### Added
+- Users have more refined options to set extractors triggers at the space level. They can now follow global settings, 
+  disable and enable triggers. 
 - Ability to set chunksize for clowder when downloading files. This is changed to 1MB from 8KB. This will result in 
   faster downloads and less CPU usage at the cost of slightly more memory use.
 - Add endpoint and view page for the extractor metrics.

app/controllers/Spaces.scala

@@ -96,10 +96,15 @@ class Spaces @Inject() (spaces: SpaceService, users: UserService, events: EventS
         case Some(s) => {
           // get list of registered extractors
           val runningExtractors: List[ExtractorInfo] = extractors.listExtractorsInfo(List.empty)
-          // get list of extractors registered with a specific space
-          val selectedExtractors: List[String] = spaces.getAllExtractors(id)
+          // list of extractors enabled globally
           val globalSelections: List[String] = extractors.getEnabledExtractors()
-          Ok(views.html.spaces.updateExtractors(runningExtractors, selectedExtractors, globalSelections, id, s.name))
+          // get list of extractors registered with a specific space
+          val selectedExtractors: Option[ExtractorsForSpace] = spaces.getAllExtractors(id)
+          val (enabledInSpace, disabledInSpace) = spaces.getAllExtractors(id) match {
+            case Some(extractorsForSpace) => (extractorsForSpace.enabled, extractorsForSpace.disabled)
+            case None => (List.empty[String], List.empty[String])
+          }
+          Ok(views.html.spaces.updateExtractors(runningExtractors, enabledInSpace, disabledInSpace, globalSelections, id, s.name))
         }
         case None => InternalServerError(spaceTitle + " not found")
       }
@@ -111,7 +116,7 @@ class Spaces @Inject() (spaces: SpaceService, users: UserService, events: EventS
   def updateExtractors(id: UUID) = PermissionAction(Permission.EditSpace, Some(ResourceRef(ResourceRef.space, id)))(parse.multipartFormData) {
     implicit request =>
       implicit val user = request.user
-      //form contains space id and list of extractors.
+      // form contains space id and list of extractors.
       var space_id: String = ""
       var extractors: List[String] = Nil
 
@@ -120,17 +125,36 @@ class Spaces @Inject() (spaces: SpaceService, users: UserService, events: EventS
         Logger.error("space id not defined")
         BadRequest(spaceTitle + " id not defined")
       } else {
-        //space id passed as hidden parameter
+        // space id passed as hidden parameter
         space_id = dataParts("space_id").head
         spaces.get(new UUID(space_id)) match {
           case Some(existing_space) => {
-            //1. remove entry with extractors for this space from mongo
+            // FIXME by splitting the operation in two separate queries we run into transaction issues if there is
+            //  a hickup between the two. We should try to do the two db queries in one.
+
+            // 1. remove entry with extractors for this space from mongo
             spaces.deleteAllExtractors(existing_space.id)
-            //2. if extractors are selected, add them
-            if (dataParts.isDefinedAt("extractors")) {
-              extractors = dataParts("extractors").toList
-              extractors.map(spaces.addExtractor(existing_space.id, _))
+            // 2. if extractors are selected, add them
+            val prefix = "extractors-"
+            extractors = dataParts.keysIterator.filter(_.startsWith(prefix)).toList
+            extractors.foreach { extractor =>
+              // get the first entry and ignore all others (there should only be one)
+              val name = extractor.substring(prefix.length)
+              val value = dataParts(extractor)(0)
+              if (value.equals("default")) {
+                spaces.setDefaultExtractor(existing_space.id, name)
+              } else if (value.equals("enabled")) {
+                spaces.enableExtractor(existing_space.id, name)
+               } else if (value.equals("disabled")) {
+                spaces.disableExtractor(existing_space.id, name)
+              } else {
+                Logger.error("Wrong value for update space extractor form")
+              }
             }
+//            if (dataParts.isDefinedAt("extractors-override")) {
+//              extractors = dataParts("extractors-override").toList
+//              extractors.map(spaces.disableExtractor(existing_space.id, _))
+//            }
             Redirect(routes.Spaces.getSpace(new UUID(space_id)))
           }
           case None => {

app/models/ExtractorsForSpace.scala

@@ -1,9 +1,14 @@
 package models
 
 /**
- * Information about extractors assigned to a space.
+ * Information about extractors assigned to a space. This only represents extractors that were customized for a specific
+ * space. For example enabled/disabled. By default we currently show all global extractors on each space and let users
+ * enable/disable them if they so wish.
+ *
+ * TODO: If we wanted to have default parameters for an extractor defined by space, this could be one place to store them.
  */
 case class ExtractorsForSpace(
     spaceId:String,
-    extractors:List[String]
+    enabled: List[String] = List.empty[String],
+    disabled: List[String] = List.empty[String]
  )

app/services/RabbitmqPlugin.scala

@@ -4,6 +4,7 @@ import java.io.IOException
 import java.net.URI
 import java.text.SimpleDateFormat
 import java.net.URLEncoder
+import java.util
 
 import akka.actor.{Actor, ActorRef, PoisonPill, Props}
 
@@ -23,6 +24,7 @@ import play.api.{Application, Logger, Plugin}
 import play.libs.Akka
 import securesocial.core.IdentityId
 
+import scala.collection.mutable.ListBuffer
 import scala.concurrent.{Await, Future}
 import scala.util.Try
 
@@ -314,15 +316,6 @@ class RabbitmqPlugin(application: Application) extends Plugin {
   private def contentTypeToRoutingKey(contentType: String) =
     contentType.replace(".", "_").replace("/", ".")
 
-  /**
-    * Given a dataset, return the union of all extractors registered for each space the dataset is in.
-    * @param dataset
-    * @return list of active extractors
-    */
-  private def getRegisteredExtractors(dataset: Dataset): List[String] = {
-    dataset.spaces.flatMap(s => spacesService.getAllExtractors(s))
-  }
-
   /**
     * Check if given operation matches any existing records cached in ExtractorInfo.
     * Note, dataset operation is in the format of "x.y",
@@ -360,16 +353,26 @@ class RabbitmqPlugin(application: Application) extends Plugin {
   }
 
   /**
-    * Find all extractors enabled for the space the dataset belongs and the matched operation.
+    * Find all extractors enabled/disabled for the space the dataset belongs and the matched operation.
     * @param dataset  The dataset used to find which space to query for registered extractors.
     * @param operation The dataset operation requested.
     * @return A list of extractors IDs.
     */
-  private def getSpaceExtractorsByOperation(dataset: Dataset, operation: String): List[String] = {
-    dataset.spaces.flatMap(s =>
-      spacesService.getAllExtractors(s).flatMap(exId =>
-        extractorsService.getExtractorInfo(exId)).filter(exInfo =>
-        containsOperation(exInfo.process.dataset, operation) || containsOperation(exInfo.process.file, operation)).map(_.name))
+  private def getSpaceExtractorsByOperation(dataset: Dataset, operation: String): (List[String], List[String]) = {
+    var enabledExtractors = new ListBuffer[String]()
+    var disabledExtractors = new ListBuffer[String]()
+    dataset.spaces.map(space => {
+      val extractors = spacesService.getAllExtractors(space)
+      enabledExtractors.appendAll(extractors.get.enabled.flatMap { exId =>
+        extractorsService.getExtractorInfo(exId).filter(exInfo =>
+          containsOperation(exInfo.process.dataset, operation) || containsOperation(exInfo.process.file, operation)).map(_.name)
+      })
+      disabledExtractors.appendAll(extractors.get.disabled.flatMap { exId =>
+        extractorsService.getExtractorInfo(exId).filter(exInfo =>
+          containsOperation(exInfo.process.dataset, operation) || containsOperation(exInfo.process.file, operation)).map(_.name)
+      })
+    })
+    (enabledExtractors.toList, disabledExtractors.toList)
   }
 
   /**
@@ -418,12 +421,14 @@ class RabbitmqPlugin(application: Application) extends Plugin {
       else ""
     // get extractors enabled at the global level
     val globalExtractors = getGlobalExtractorsByOperation(operation)
-    // get extractors enabled at the space level
-    val spaceExtractors = getSpaceExtractorsByOperation(dataset, operation)
+    // get extractors enabled/disabled at the space level
+    val (enabledExtractors, disabledExtractors) = getSpaceExtractorsByOperation(dataset, operation)
     // get queues based on RabbitMQ bindings (old method).
     val queuesFromBindings = getQueuesFromBindings(routingKey)
     // take the union of queues so that we publish to a specific queue only once
-    globalExtractors.toSet union spaceExtractors.toSet union queuesFromBindings.toSet
+    val selected = (globalExtractors.toSet -- disabledExtractors.toSet) union enabledExtractors.toSet union queuesFromBindings.toSet
+    Logger.debug("Extractors selected for submission: " + selected)
+    selected
   }
 
   /**

app/services/SpaceService.scala

@@ -275,12 +275,29 @@ trait SpaceService {
 	 * If entry for spaceId already exists, will update list of extractors.
 	 * Otherwise will create and add a new document to the collection, with spaceId and extractor given.
 	 */
-	def addExtractor(spaceId: UUID, extractor:String)
+	def enableExtractor(spaceId: UUID, extractor: String)
+
+  /**
+   * Disable extractors within the space. This is used to override global selections.
+   * @param spaceId
+   * @param extractor
+   */
+  def disableExtractor(spaceId: UUID, extractor: String)
+
+
+  /**
+   * Follow the global setting for whether to trigger an extractor or not.
+   * @param spaceId
+   * @param extractor
+   */
+  def setDefaultExtractor(spaceId: UUID, extractor: String)
 
 	/**
-	 * Get all extractors for this space id.
+	 * Get all extractors for this space id. This is the union of all enabled and disabled extractors for this space.
+   * If a user never manually enabled or disabled an extractor for a space it will not be returned, but the extractor
+   * might still be enabled/disabled at the instance level.
 	 */
-  def getAllExtractors(spaceId: UUID): List[String]
+  def getAllExtractors(spaceId: UUID): Option[ExtractorsForSpace]
 
   /**
 	 * Delete an entire entry with extractors for this space id.

app/services/mongodb/MongoDBExtractorService.scala

@@ -170,7 +170,7 @@ class MongoDBExtractorService extends ExtractorService {
   def listExtractorsInfo(categories: List[String]): List[ExtractorInfo] = {
     var list_queue = List[ExtractorInfo]()
 
-    val allDocs = ExtractorInfoDAO.findAll()
+    val allDocs = ExtractorInfoDAO.findAll().sort(orderBy = MongoDBObject("name" -> -1))
     for (doc <- allDocs) {
       // If no categories are specified, return all extractor names
       var category_match = categories.isEmpty

app/services/mongodb/MongoDBSpaceService.scala

@@ -4,11 +4,11 @@ package services.mongodb
 import javax.inject.{Inject, Singleton}
 import api.Permission
 import api.Permission.Permission
-import com.mongodb.casbah.Imports._
+import com.mongodb.casbah.Imports.{$and, _}
 import com.mongodb.casbah.WriteConcern
 import com.mongodb.casbah.commons.MongoDBObject
 import com.mongodb.DBObject
-import com.novus.salat.dao.{SalatDAO, ModelCompanion}
+import com.novus.salat.dao.{ModelCompanion, SalatDAO}
 import models._
 import org.bson.types.ObjectId
 import play.api.Logger
@@ -675,24 +675,29 @@ class MongoDBSpaceService @Inject() (
    * If entry for this spaceId already exists, adds extractor to it.
    * Otherwise, creates a new entry with spaceId and extractor.
    */
-  def addExtractor (spaceId: UUID, extractor: String) {
-	  //will add extractor to the list of extractors for this space, only if it's not there.
+  def enableExtractor(spaceId: UUID, extractor: String) {
 	  val query = MongoDBObject("spaceId" -> spaceId.stringify)
-	  ExtractorsForSpaceDAO.update(query, $addToSet("extractors" -> extractor), true, false, WriteConcern.Safe)
+	  ExtractorsForSpaceDAO.update(query, $addToSet("enabled" -> extractor), true, false, WriteConcern.Safe)
+  }
+
+  def disableExtractor(spaceId: UUID, extractor: String) {
+    val query = MongoDBObject("spaceId" -> spaceId.stringify)
+    ExtractorsForSpaceDAO.update(query, $addToSet("disabled" -> extractor), true, false, WriteConcern.Safe)
+  }
+
+  def setDefaultExtractor(spaceId: UUID, extractor: String) {
+    val query = MongoDBObject("spaceId" -> spaceId.stringify)
+    ExtractorsForSpaceDAO.update(query, $pull("disabled" -> extractor,"enabled" -> extractor), true, false, WriteConcern.Safe)
   }
 
   /**
    * Returns a list of extractors associated with this spaceId.
    */
-  def getAllExtractors(spaceId: UUID): List[String] = {
-    //Note: in models.ExtractorsForSpace, spaceId must be a String
+  def getAllExtractors(spaceId: UUID): Option[ExtractorsForSpace] = {
+    // Note: in models.ExtractorsForSpace, spaceId must be a String
     // if space Id is UUID, will compile but throws Box run-time error
     val query = MongoDBObject("spaceId" -> spaceId.stringify)
-
-    val list = (for (extr <- ExtractorsForSpaceDAO.find(query)) yield extr).toList
-    //get extractors' names for given space id
-    val extractorList: List[String] = list.flatMap(_.extractors)
-    extractorList
+    ExtractorsForSpaceDAO.findOne(query)
   }
 
 

app/services/mongodb/MongoSalatPlugin.scala

@@ -445,6 +445,9 @@ class MongoSalatPlugin(app: Application) extends Plugin {
 
     // Updates permissions for the admin Role
     updateMongo("update-admin-role", updateAdminRole)
+
+    // Updates extractors enabled and disabled in a space
+    updateMongo("update-space-extractors-selection", updateSpaceExtractorsSelection)
   }
 
   private def updateMongo(updateKey: String, block: () => Unit): Unit = {
@@ -1668,4 +1671,19 @@ class MongoSalatPlugin(app: Application) extends Plugin {
     }
   }
 
+  private def updateSpaceExtractorsSelection(): Unit = {
+    collection("spaces.extractors").foreach { space =>
+      val enabled = space.getAsOrElse[MongoDBList]("extractors", MongoDBList.empty)
+
+      collection("spaces.extractors").update(
+        MongoDBObject("_id" -> space.get("_id")),
+        MongoDBObject("$set" -> MongoDBObject("enabled" -> enabled)), upsert = false, multi = false)
+
+      collection("spaces.extractors").update(
+        MongoDBObject("_id" -> space.get("_id")),
+        $unset("extractors"))
+    }
+    print("DONE")
+  }
+
 }

app/views/spaces/updateExtractors.scala.html

@@ -1,8 +1,8 @@
-@(runningExtractors: List[ExtractorInfo], selectedExtractors: List[String], globalSelections: List[String], space: UUID, spaceName: String)(implicit user: Option[models.User])
+@(runningExtractors: List[ExtractorInfo], enabledInSpace: List[String], disabledInSpace: List[String], globalSelections: List[String], space: UUID, spaceName: String)(implicit user: Option[models.User])
 @import _root_.util.Formatters._
 @import helper._
 
-@main("Space") {
+@main("Space Extractors") {
 	<div>
 		<ol class="breadcrumb">
 			<li><span class="glyphicon glyphicon-hdd"></span>  <a href="@routes.Spaces.getSpace(space)" title="@spaceName"> @Html(ellipsize(spaceName,18))</a></li>
@@ -16,7 +16,7 @@ <h1> No extractors running</h1>
   </div>
  } else {
      <div class="page-header">
-		 <h1>Extractors</h1>
+		 <h1>Space Extractors</h1>
      </div>
 	 <div class="row top-padding">
 		 <div class="col-xs-12">
@@ -32,20 +32,19 @@ <h1>Extractors</h1>
 		<table class="table">
 			<thead>
 				<tr>
-					<th>Enabled</th>
 					<th>Name</th>
 					<th>Description</th>
 					<th>Process Triggers</th>
+					<th class="text-center" title="Extractors marked for execution in all spaces">Enabled By SuperAdmin</th>
+					<th class="text-center" title="Follow selection in 'Enabled By SuperAdmin' column">Use Default</th>
+					<th class="text-center" title="Always trigger execution in this space">Enable in Space</th>
+					<th class="text-center" title="Never trigger execution in this space">Disable in Space</th>
 				</tr>
 			</thead>
 			<tbody>
 			<!-- Display all running extractors. If extractor already in this space, check the box. -->
 			@runningExtractors.map { extractor =>
 				<tr>
-					<td><input type="checkbox" name="extractors" value="@extractor.name"
-						@if(globalSelections.contains(extractor.name)) {disabled="disabled"}
-						@if(globalSelections.contains(extractor.name) ||
-								selectedExtractors.contains(extractor.name)) {checked} else {unchecked}></td>
 					<td><a href="@routes.Extractors.showExtractorInfo(extractor.name)">@extractor.name</a></td>
 					<td>@extractor.description</td>
 					<td>
@@ -83,6 +82,23 @@ <h1>Extractors</h1>
 						None
 						</p>
 					</td>
+					<td class="text-center">
+						@if(globalSelections.contains(extractor.name)) {
+							<span class="glyphicon glyphicon-ok" aria-hidden="true"></span>
+						}
+					</td>
+					<td class="text-center">
+						<input type="radio" name="[email protected]" value="default"
+						@if(!enabledInSpace.contains(extractor.name) && !disabledInSpace.contains(extractor.name)) {checked}>
+					</td>
+					<td class="text-center">
+						<input type="radio" name="[email protected]" value="enabled"
+						@if(enabledInSpace.contains(extractor.name)) {checked}>
+					</td>
+					<td class="text-center">
+						<input type="radio" name="[email protected]" value="disabled"
+						@if(disabledInSpace.contains(extractor.name)) {checked}>
+					</td>
 				</tr>
 			}
 			</tbody>