Merge pull request #1359 in CATS/clowder from bugfix/CATS-999-page-loading-is-very-slow-for-datasets-with-lots-of-files to develop

* commit 'de7a8cded27c73f7add8ca30812e77a935776d65':
  Fixed slow load times in dataset page by removing queries for comments  and tags on files within a dataset. As a result dataset page does not  show comments on files within the dataset anymore.

Author: lmarini

CHANGELOG.md

@@ -12,11 +12,17 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - About page should no longer show "0 Bytes", counts should be more accurate.
   [CATS-779] (https://opensource.ncsa.illinois.edu/jira/browse/CATS-779)
 - Fixed creation of standard vocabularies in a single space case.
+- Slow load times in dataset page by removing queries for comments and tags on files within a dataset.
+  [CATS-999] (https://opensource.ncsa.illinois.edu/jira/browse/CATS-999)
 
 ### Changed
 - Changed the HTTP return codes for the generic error handlers in Clowder.
 - Adjusted display of Advanced Search matching options to include (AND) / (OR).
   [CATS-998](https://opensource.ncsa.illinois.edu/jira/browse/CATS-998)
+- Dataset page does not show comments on files within the dataset anymore.
+- dataset-image previewer turned off by default since it is expensive for datasets with many files but does not much
+  information to the dataset page.
+- Removed unused queries for comments throughout the application.
 
 ### Added
 - When a folder has been deleted, clowder will traverse each file (directly/indirectly) under this folder and send file deletion event to Rabbitmq.

app/controllers/Application.scala

@@ -119,16 +119,6 @@ class Application @Inject() (files: FileService, collections: CollectionService,
         if( play.Play.application().configuration().getBoolean("showCommentOnHomepage")) newsfeedEvents = newsfeedEvents :::events.getCommentEvent(clowderUser, Some(20))
         newsfeedEvents = newsfeedEvents.sorted(Ordering.by((_: Event).created).reverse).distinct.take(20)
         val datasetsUser = datasets.listUser(12, Some(clowderUser), request.user.fold(false)(_.superAdminMode), clowderUser)
-        val datasetcommentMap = datasetsUser.map { dataset =>
-          var allComments = comments.findCommentsByDatasetId(dataset.id)
-          dataset.files.map { file =>
-            allComments ++= comments.findCommentsByFileId(file)
-            sections.findByFileId(file).map { section =>
-              allComments ++= comments.findCommentsBySectionId(section.id)
-            }
-          }
-          dataset.id -> allComments.size
-        }.toMap
         val collectionList = collections.listUser(12, Some(clowderUser), request.user.fold(false)(_.superAdminMode), clowderUser)
         val collectionsWithThumbnails = collectionList.map {c =>
           if (c.thumbnail_id.isDefined) {
@@ -211,8 +201,9 @@ class Application @Inject() (files: FileService, collections: CollectionService,
           if(user.isDefined) selections.get(user.get.identityId.userId).map(_.id.stringify)
           else List.empty[String]
         Logger.debug("User selection " + userSelections)
-        Ok(views.html.home(AppConfiguration.getDisplayName, newsfeedEvents, clowderUser, datasetsUser, datasetcommentMap, decodedCollections.toList, spacesUser, true, followers, followedUsers.take(12),
-       followedFiles.take(8), followedDatasets.take(8), followedCollections.take(8),followedSpaces.take(8), Some(true), userSelections))
+        Ok(views.html.home(AppConfiguration.getDisplayName, newsfeedEvents, clowderUser, datasetsUser,
+          decodedCollections.toList, spacesUser, true, followers, followedUsers.take(12), followedFiles.take(8),
+          followedDatasets.take(8), followedCollections.take(8),followedSpaces.take(8), Some(true), userSelections))
       }
       case _ => {
         // Set bytes from appConfig

app/controllers/Collections.scala

@@ -542,17 +542,6 @@ class Collections @Inject() (datasets: DatasetService, collections: CollectionSe
             decodedDatasetsInside += dDataset
           }
 
-          val commentMap = datasetsInside.map { dataset =>
-            var allComments = comments.findCommentsByDatasetId(dataset.id)
-            dataset.files.map { file =>
-              allComments ++= comments.findCommentsByFileId(file)
-              sections.findByFileId(file).map { section =>
-                allComments ++= comments.findCommentsBySectionId(section.id)
-              }
-            }
-            dataset.id -> allComments.size
-          }.toMap
-
           val child_collections = if(play.Play.application().configuration().getBoolean("sortInMemory")) {
             SortingUtils.sortCollections(dCollection.child_collection_ids.flatMap(c => collections.get(c)), sortOrder).slice(0, limit)
           } else {
@@ -630,7 +619,7 @@ class Collections @Inject() (datasets: DatasetService, collections: CollectionSe
           val (view_count, view_date) = collections.incrementViews(id, user)
 
           Ok(views.html.collectionofdatasets(decodedDatasetsInside.toList, decodedChildCollections.toList,
-            Some(decodedParentCollections.toList),dCollection, filteredPreviewers.toList,commentMap,Some(collectionSpaces_canRemove),
+            Some(decodedParentCollections.toList),dCollection, filteredPreviewers, Some(collectionSpaces_canRemove),
             prevd,nextd, prevcc, nextcc, limit, canAddToParent, userSelections, view_count, view_date))
 
         }
@@ -654,16 +643,6 @@ class Collections @Inject() (datasets: DatasetService, collections: CollectionSe
           decodedDatasetsInside += dDataset
         }
 
-        val commentMap = datasetsInside.map { dataset =>
-          var allComments = comments.findCommentsByDatasetId(dataset.id)
-          dataset.files.map { file =>
-            allComments ++= comments.findCommentsByFileId(file)
-            sections.findByFileId(file).map { section =>
-              allComments ++= comments.findCommentsBySectionId(section.id)
-            }
-          }
-          dataset.id -> allComments.size
-        }.toMap
         val prev = index-1
         val next = if(datasetsInside.length > (index+1) * limit) {
           index + 1
@@ -675,9 +654,9 @@ class Collections @Inject() (datasets: DatasetService, collections: CollectionSe
             val selectIds = selections.get(u.email.get).map(d => {
               d.id.toString
             })
-            Ok(views.html.collections.datasetsInCollection(decodedDatasetsInside.toList, commentMap, id, prev, next, selectIds))
+            Ok(views.html.collections.datasetsInCollection(decodedDatasetsInside.toList, id, prev, next, selectIds))
           }
-          case None => Ok(views.html.collections.datasetsInCollection(decodedDatasetsInside.toList, commentMap, id, prev, next, List.empty))
+          case None => Ok(views.html.collections.datasetsInCollection(decodedDatasetsInside.toList, id, prev, next, List.empty))
         }
 
 

app/controllers/Datasets.scala

@@ -168,17 +168,6 @@ class Datasets @Inject() (
           }
         }
 
-        val commentMap = datasetList.map { dataset =>
-          var allComments = comments.findCommentsByDatasetId(dataset.id)
-          dataset.files.map { file =>
-            allComments ++= comments.findCommentsByFileId(file)
-            sections.findByFileId(file).map { section =>
-              allComments ++= comments.findCommentsBySectionId(section.id)
-            }
-          }
-          dataset.id -> allComments.size
-        }.toMap
-
         //Modifications to decode HTML entities that were stored in an encoded fashion as part
         //of the datasets names or descriptions
         val decodedDatasetList = ListBuffer.empty[models.Dataset]
@@ -206,7 +195,7 @@ class Datasets @Inject() (
         Logger.debug("User selection " + userSelections)
 
         //Pass the viewMode into the view
-        Ok(views.html.users.followingDatasets(decodedDatasetList.toList, commentMap, prev, next, limit, viewMode, None, title, None, userSelections))
+        Ok(views.html.users.followingDatasets(decodedDatasetList.toList, prev, next, limit, viewMode, None, title, None, userSelections))
       }
       case None => InternalServerError("No User found")
     }
@@ -356,17 +345,6 @@ class Datasets @Inject() (
       ""
     }
 
-    val commentMap = datasetList.map { dataset =>
-      var allComments = comments.findCommentsByDatasetId(dataset.id)
-      dataset.files.map { file =>
-        allComments ++= comments.findCommentsByFileId(file)
-        sections.findByFileId(file).map { section =>
-          allComments ++= comments.findCommentsBySectionId(section.id)
-        }
-      }
-      dataset.id -> allComments.size
-    }.toMap
-
     //Modifications to decode HTML entities that were stored in an encoded fashion as part
     //of the datasets names or descriptions
     val decodedDatasetList = ListBuffer.empty[models.Dataset]
@@ -397,7 +375,9 @@ class Datasets @Inject() (
       case Some(s) if !Permission.checkPermission(Permission.ViewSpace, ResourceRef(ResourceRef.space, UUID(s))) => {
         BadRequest(views.html.notAuthorized("You are not authorized to access the " + spaceTitle + ".", s, "space"))
       }
-      case _ => Ok(views.html.datasetList(decodedDatasetList.toList, commentMap, prev, next, limit, viewMode, space, spaceName, status, title, owner, ownerName, when, date, userSelections, showTrash))
+      case _ =>
+        Ok(views.html.datasetList(decodedDatasetList.toList, prev, next, limit, viewMode, space, spaceName,
+        status, title, owner, ownerName, when, date, userSelections, showTrash))
     }
   }
 
@@ -512,7 +492,7 @@ class Datasets @Inject() (
           ""
         }
         val date = ""
-        Ok(views.html.datasetList(decodedDatasetList.toList, commentMap, prev, next, limit, viewMode, Some(space), spaceName, None, title, None, None, "a", date, userSelections))
+        Ok(views.html.datasetList(decodedDatasetList.toList, prev, next, limit, viewMode, Some(space), spaceName, None, title, None, None, "a", date, userSelections))
       }
     }
   }
@@ -526,54 +506,30 @@ class Datasets @Inject() (
     Previewers.findDatasetPreviewers().foreach(p => Logger.debug("Previewer found " + p.id))
     datasets.get(id) match {
       case Some(dataset) => {
-
-        // get files info sorted by date
-        val filesInDataset = dataset.files.flatMap(f => files.get(f) match {
-          case Some(file) => Some(file)
-          case None => Logger.debug(s"Unable to find file $f"); None
-        }).asInstanceOf[List[File]].sortBy(_.uploadDate)
-
-        var datasetWithFiles = dataset.copy(files = filesInDataset.map(_.id))
-        datasetWithFiles = Utils.decodeDatasetElements(datasetWithFiles)
-
+        // previewers
         val filteredPreviewers = Previewers.findDatasetPreviewers
 
+        // metadata
         val m = metadata.getMetadataByAttachTo(ResourceRef(ResourceRef.dataset, dataset.id))
 
+        // collections
         val collectionsInside = collections.listInsideDataset(id, request.user, request.user.fold(false)(_.superAdminMode)).sortBy(_.name)
         var decodedCollectionsInside = new ListBuffer[models.Collection]()
-        var filesTags = TreeSet.empty[String]
-
         for (aCollection <- collectionsInside) {
           val dCollection = Utils.decodeCollectionElements(aCollection)
           decodedCollectionsInside += dCollection
         }
 
-        var commentsByDataset = comments.findCommentsByDatasetId(id)
-        filesInDataset.map {
-          file =>
-
-            commentsByDataset ++= comments.findCommentsByFileId(file.id)
-            sections.findByFileId(UUID(file.id.toString)).map { section =>
-              commentsByDataset ++= comments.findCommentsBySectionId(section.id)
-            }
-        }
-        commentsByDataset = commentsByDataset.sortBy(_.posted)
-
-        //Decode the comments so that their free text will display correctly in the view
+        // comments
+        var commentsByDataset = comments.findCommentsByDatasetId(id).sortBy(_.posted)
+        // decode the comments so that their free text will display correctly in the view
         var decodedCommentsByDataset = ListBuffer.empty[Comment]
         for (aComment <- commentsByDataset) {
           val dComment = Utils.decodeCommentElements(aComment)
           decodedCommentsByDataset += dComment
         }
 
-        filesInDataset.map { file =>
-          file.tags.map {
-            tag => filesTags += tag.name
-          }
-        }
-
-        // associated sensors
+        // sensors
         val sensors: List[(String, String, String)] = current.plugin[PostgresPlugin] match {
           case Some(db) if db.isEnabled => {
             // findRelationships will return a "Relation" model with all information about the relationship
@@ -591,8 +547,8 @@ class Datasets @Inject() (
           case _ => List.empty[(String, String, String)]
         }
 
+        // spaces
         var datasetSpaces: List[ProjectSpace] = List.empty[ProjectSpace]
-
         var decodedSpaces_canRemove: Map[ProjectSpace, Boolean] = Map.empty
         var isInPublicSpace = false
         dataset.spaces.map {
@@ -607,26 +563,27 @@ class Datasets @Inject() (
               case None => Logger.error(s"space with id $sp on $Messages('dataset.title') $id doesn't exist.")
             }
         }
-
-        val fileList: List[File] = dataset.files.reverse.map(f => files.get(f)).flatten
-
-        //dataset is in at least one space with editstagingarea permission, or if the user is the owner of dataset.
-        val stagingarea = datasetSpaces filter (space => Permission.checkPermission(Permission.EditStagingArea, ResourceRef(ResourceRef.space, space.id)))
+        // dataset is in at least one space with editstagingarea permission, or if the user is the owner of dataset
+        val stagingarea = datasetSpaces filter (space => Permission.checkPermission(Permission.EditStagingArea,
+          ResourceRef(ResourceRef.space, space.id)))
         val toPublish = !stagingarea.isEmpty
-
-        val curObjectsPublished: List[CurationObject] = curationService.getCurationObjectByDatasetId(dataset.id).filter(_.status == 'Published)
-        val curObjectsPermission: List[CurationObject] = curationService.getCurationObjectByDatasetId(dataset.id).filter(curation => Permission.checkPermission(Permission.EditStagingArea, ResourceRef(ResourceRef.curationObject, curation.id)))
+        val curObjectsPublished: List[CurationObject] = curationService.getCurationObjectByDatasetId(dataset.id).filter(
+          _.status == 'Published)
+        val curObjectsPermission: List[CurationObject] = curationService.getCurationObjectByDatasetId(dataset.id).filter(curation =>
+          Permission.checkPermission(Permission.EditStagingArea, ResourceRef(ResourceRef.curationObject, curation.id)))
         val curPubObjects: List[CurationObject] = curObjectsPublished ::: curObjectsPermission
 
+        // download button
         var showDownload: Boolean = dataset.files.length > 0
         if (!showDownload) {
           val foldersList = folders.findByParentDatasetId(dataset.id)
           foldersList.map { folder =>
             if (folder.files.length > 0) { showDownload = true }
           }
         }
-        var showAccess = false
 
+        // access control based on config flags `verifySpaces`, `enablePublic`
+        var showAccess = false
         if (play.Play.application().configuration().getBoolean("verifySpaces")) {
           showAccess = !dataset.isTRIAL
         } else {
@@ -649,11 +606,13 @@ class Datasets @Inject() (
         } else {
           accessOptions.append(spaceTitle + " Default (Private)")
         }
-        accessOptions.append(DatasetStatus.PRIVATE.toString.substring(0, 1).toUpperCase() + DatasetStatus.PRIVATE.toString.substring(1).toLowerCase())
-        accessOptions.append(DatasetStatus.PUBLIC.toString.substring(0, 1).toUpperCase() + DatasetStatus.PUBLIC.toString.substring(1).toLowerCase())
-
+        accessOptions.append(DatasetStatus.PRIVATE.toString.substring(0, 1).toUpperCase() +
+          DatasetStatus.PRIVATE.toString.substring(1).toLowerCase())
+        accessOptions.append(DatasetStatus.PUBLIC.toString.substring(0, 1).toUpperCase() +
+          DatasetStatus.PUBLIC.toString.substring(1).toLowerCase())
         val accessData = new models.DatasetAccess(showAccess, access, accessOptions.toList)
 
+        // add to collection permissions
         var canAddDatasetToCollection = Permission.checkOwner(user, ResourceRef(ResourceRef.dataset, dataset.id))
         if (!canAddDatasetToCollection) {
           datasetSpaces.map(space =>
@@ -662,18 +621,21 @@ class Datasets @Inject() (
            }
           )
         }
+
+        // staging area
         val stagingAreaDefined = play.api.Play.current.plugin[services.StagingAreaPlugin].isDefined
 
+        // extraction logs
         val extractionsByDataset = extractions.findById(new ResourceRef('dataset, id))
         val extractionGroups = extractions.groupByType(extractionsByDataset)
 
-        // Increment view count for dataset
+        // increment view count for dataset
         val view_data = datasets.incrementViews(id, user)
 
         // view_data is passed as tuple in dataset case only, because template is at limit of 22 parameters
-        Ok(views.html.dataset(datasetWithFiles, commentsByDataset, filteredPreviewers.toList, m,
-          decodedCollectionsInside.toList, sensors, Some(decodedSpaces_canRemove), fileList,
-          filesTags, toPublish, curPubObjects, currentSpace, limit, showDownload, accessData, canAddDatasetToCollection,
+        Ok(views.html.dataset(dataset, commentsByDataset, filteredPreviewers.toList, m,
+          decodedCollectionsInside.toList, sensors, Some(decodedSpaces_canRemove), toPublish, curPubObjects,
+          currentSpace, limit, showDownload, accessData, canAddDatasetToCollection,
           stagingAreaDefined, view_data, extractionGroups))
       }
       case None => {
@@ -698,14 +660,14 @@ class Datasets @Inject() (
           case Some(fId) => {
             folders.get(UUID(fId)) match {
               case Some(folder) => {
-                val (foldersList: List[Folder], limitFileList: List[File]) = if(play.Play.application().configuration().getBoolean("sortInMemory")) {
-                  (SortingUtils.sortFolders(folder.folders.flatMap(f => folders.get(f)), sortOrder).slice(limit * filepageUpdate, limit * (filepageUpdate + 1)),
-                   SortingUtils.sortFiles(folder.files.flatMap(f => files.get(f)), sortOrder).slice(limit * filepageUpdate - folder.folders.length, limit * (filepageUpdate + 1) - folder.folders.length))
-                } else {
-                  (folder.folders.reverse.slice(limit * filepageUpdate, limit * (filepageUpdate+1)).flatMap(f => folders.get(f)),
-                   folder.files.reverse.slice(limit * filepageUpdate - folder.folders.length, limit * (filepageUpdate+1) - folder.folders.length).flatMap(f => files.get(f)))
-                }
-
+                val (foldersList: List[Folder], limitFileList: List[File]) =
+                  if(play.Play.application().configuration().getBoolean("sortInMemory")) {
+                    (SortingUtils.sortFolders(folder.folders.flatMap(f => folders.get(f)), sortOrder).slice(limit * filepageUpdate, limit * (filepageUpdate + 1)),
+                     SortingUtils.sortFiles(folder.files.flatMap(f => files.get(f)), sortOrder).slice(limit * filepageUpdate - folder.folders.length, limit * (filepageUpdate + 1) - folder.folders.length))
+                  } else {
+                    (folder.folders.reverse.slice(limit * filepageUpdate, limit * (filepageUpdate+1)).flatMap(f => folders.get(f)),
+                     folder.files.reverse.slice(limit * filepageUpdate - folder.folders.length, limit * (filepageUpdate+1) - folder.folders.length).flatMap(f => files.get(f)))
+                  }
                 var folderHierarchy = new ListBuffer[Folder]()
                 folderHierarchy += folder
                 var f1: Folder = folder