Merge pull request #2174 from broadinstitute/development

Release 1.86.0

app/controllers/api/v1/concerns/api_caching.rb

@@ -3,7 +3,11 @@ module V1
     module Concerns
       module ApiCaching
         extend ActiveSupport::Concern
+
+        # regexes for blocker types of malicious requests
+        # this prevents the app from being flooded during security scans that can cause runaway viz caching
         XSS_MATCHER = /(xssdetected|script3E)/
+        SCAN_MATCHER = /(\.(git|svn|php)|NULL(%20|\+)OR(%20|\+)1|CODE_POINTS_TO_STRING|UPDATEXML|\$%7Benv)/
 
         # check Rails cache for JSON response based off url/params
         # cache expiration is still handled by CacheRemovalJob
@@ -39,8 +43,8 @@ def check_caching_config
 
         # ignore obvious malicious/bogus requests that can lead to invalid cache path entries
         def validate_cache_request
-          if request.fullpath =~ XSS_MATCHER
-            head 400 and return
+          if request.fullpath =~ XSS_MATCHER || request.fullpath =~ SCAN_MATCHER
+            render json: { error: 'Bad request' }, status: 400 and return
           end
         end
       end

app/controllers/api/v1/study_files_controller.rb

@@ -392,9 +392,14 @@ def perform_update!(study_file)
 
         if ['Expression Matrix', 'MM Coordinate Matrix'].include?(study_file.file_type) && !safe_file_params[:y_axis_label].blank?
           # if user is supplying an expression axis label, update default options hash
-          options = study.default_options.dup
-          options.merge!(expression_label: safe_file_params[:y_axis_label])
-          study.update(default_options: options)
+          study.default_options[:expression_label] = safe_file_params[:y_axis_label]
+          study.save
+        elsif study_file.is_viz_anndata?
+          expression_label = study_file.ann_data_file_info.expression_axis_label
+          if expression_label
+            study.default_options[:expression_label] = expression_label
+            study.save
+          end
         end
 
         if safe_file_params[:upload].present? && !is_chunked || safe_file_params[:remote_location].present?

app/controllers/api/v1/visualization/annotations_controller.rb

@@ -100,6 +100,10 @@ def index
 
         def show
           annotation = self.class.get_selected_annotation(@study, params)
+          if annotation.nil?
+            head :not_found and return
+          end
+
           render json: annotation
         end
 
@@ -148,6 +152,10 @@ def show
 
         def cell_values
           annotation = self.class.get_selected_annotation(@study, params)
+          if annotation.nil?
+            head :not_found and return
+          end
+
           cell_cluster = @study.cluster_groups.by_name(params[:cluster])
           if cell_cluster.nil?
             cell_cluster = @study.default_cluster
@@ -364,9 +372,6 @@ def gene_list
         # parses the url params to identify the selected cluster
         def self.get_selected_annotation(study, params)
           annot_params = get_annotation_params(params)
-          if annot_params[:name] == '_default'
-            annot_params[:name] = nil
-          end
           cluster = nil
           if annot_params[:scope] == 'cluster'
             if params[:cluster].blank?
@@ -384,7 +389,7 @@ def self.get_selected_annotation(study, params)
         def self.get_facet_annotations(study, cluster, annot_param)
           annotations = annot_param.split(',').map { |annot| convert_annotation_param(annot) }
           annotations.map do |annotation|
-            AnnotationVizService.get_selected_annotation(study, cluster:, fallback: false, **annotation)
+            AnnotationVizService.get_selected_annotation(study, cluster:, **annotation)
           end.compact
         end
 

app/controllers/api/v1/visualization/clusters_controller.rb

@@ -89,19 +89,19 @@ def index
               key :type, :string
             end
             parameter do
-              key :name, :annot_name
+              key :name, :annotation_name
               key :in, :query
               key :description, 'Name of the annotation to categorize the cluster data.  Blank for default annotation.'
               key :type, :string
             end
             parameter do
-              key :name, :annot_type
+              key :name, :annotation_type
               key :in, :query
               key :description, 'Type of the annotation to retrieve--numeric or category.  Blank for default annotation.'
               key :type, :string
             end
             parameter do
-              key :name, :annot_scope
+              key :name, :annotation_scope
               key :in, :query
               key :description, 'Scope of the annotation to retrieve--study or cluster.  Blank for default annotation.'
               key :type, :string
@@ -156,7 +156,8 @@ def self.get_cluster_viz_data(study, cluster, url_params)
                                                        annot_type: annot_params[:type],
                                                        annot_scope: annot_params[:scope])
           if !annotation
-            raise ArgumentError, "Annotation \"#{annot_params[:annot_name]}\" could not be found"
+            annot_identifier = annot_params.values.compact.join('--')
+            raise ArgumentError, "Annotation \"#{annot_identifier}\" could not be found for \"#{cluster.name}\""
           end
 
           subsample = get_selected_subsample_threshold(url_params[:subsample], cluster)

app/controllers/api/v1/visualization/expression_controller.rb

@@ -105,6 +105,10 @@ def show
             }, status: :bad_request and return
           end
 
+          if @annotation.nil?
+            render json: { error: 'Requested annotation not found' }, status: :not_found and return
+          end
+
           data_type = params[:data_type]
           case data_type
           when 'violin'
@@ -163,10 +167,6 @@ def render_morpheus_json
             render json: { error: 'Requested cluster not found' }, status: :not_found and return
           end
 
-          if @annotation.nil?
-            render json: { error: 'Requested annotation not found' }, status: :not_found and return
-          end
-
           expression_data = ExpressionVizService.get_morpheus_json_data(
             study: @study, genes: @genes, cluster: @cluster, annotation: @annotation, subsample_threshold: @subsample
           )

app/javascript/components/upload/FileUploadControl.jsx

@@ -72,7 +72,7 @@ export default function FileUploadControl({
     }
 
     setFileValidation({ validating: true, issues: {}, fileName: selectedFile.name })
-    const [issues, notes] = await ValidateFile.validateLocalFile(selectedFile, file, allFiles, allowedFileExts)
+    const [issues, notes] = await ValidateFile.validateLocalFile(selectedFile, file, allFiles, allowedFileExts, isAnnDataExperience)
     setFileValidation({ validating: false, issues, fileName: selectedFile.name, notes })
     if (issues.errors.length === 0) {
       updateFile(file._id, {
@@ -133,7 +133,7 @@ export default function FileUploadControl({
     setFileValidation({ validating: true, issues: {}, fileName: trimmedPath })
     try {
       const issues = await ValidateFile.validateRemoteFile(
-        bucketName, trimmedPath, fileType, fileOptions
+        bucketName, trimmedPath, fileType, fileOptions, isAnnDataExperience
       )
       setFileValidation({ validating: false, issues, fileName: trimmedPath })
 

app/javascript/components/visualization/ScatterPlot.jsx

@@ -175,8 +175,11 @@ function RawScatterPlot({
   /** Update layout, without recomputing traces */
   function resizePlot() {
     const scatter = updateScatterLayout()
-    Plotly.relayout(graphElementId, scatter.layout)
-    setScatterData(scatter)
+    // in cases where an annotation wasn't found, skip calling Plotly.relayout before plot is instantiated
+    if (document.getElementById(graphElementId).data) {
+      Plotly.relayout(graphElementId, scatter.layout)
+      setScatterData(scatter)
+    }
   }
 
   /** Update legend counts and recompute traces, without recomputing layout */
@@ -604,7 +607,7 @@ function RawScatterPlot({
         correlation={bulkCorrelation}/>
       { hasMissingAnnot &&
         <div className="alert-warning text-center error-boundary">
-          "{cluster}" does not have the requested annotation "{loadedAnnotation}"
+          "{cluster}" does not have the requested annotation {loadedAnnotation !== '----' && loadedAnnotation}
         </div>
       }
       <div

app/javascript/lib/pathway-expression.js

@@ -145,31 +145,49 @@ export async function renderBackgroundDotPlot(
 /** Get unique genes in pathway diagram, ranked by global interest */
 export function getPathwayGenes(ranks) {
   const dataNodes = Array.from(document.querySelectorAll('#_ideogramPathwayContainer g.DataNode'))
-  const geneNodes = dataNodes.filter(
-    dataNode => Array.from(dataNode.classList).some(cls => cls.startsWith('Ensembl_ENS'))
-  )
+  const geneNodes = []
+  for (let i = 0; i < dataNodes.length; i++) {
+    const dataNode = dataNodes[i]
+    const classes = dataNode.classList
+
+    for (let j = 0; j < classes.length; j++) {
+      const cls = classes[j]
+      const isGene = ['geneproduct', 'rna', 'protein'].includes(cls.toLowerCase())
+      if (isGene) {
+        geneNodes.push(dataNode)
+        break
+      }
+    }
+  }
+
   const genes = geneNodes.map(
     node => {return { domId: node.id, name: node.querySelector('text').textContent }}
   )
+
   const rankedGenes = genes
     .filter(gene => ranks.includes(gene.name))
     .sort((a, b) => ranks.indexOf(a.name) - ranks.indexOf(b.name))
+
   return rankedGenes
 }
 
-/** Get up to 50 genes from pathway, including searched gene and interacting gene */
-function getDotPlotGenes(searchedGene, interactingGene, pathwayGenes) {
+/** Slice array into batches of a given size */
+function sliceIntoBatches(arr, batchSize) {
+  const result = []
+  for (let i = 0; i < arr.length; i += batchSize) {
+    result.push(arr.slice(i, i + batchSize))
+  }
+  return result
+}
+
+/** Get genes from pathway, in batches of up to 50 genes, eliminating duplicates */
+export function getDotPlotGeneBatches(pathwayGenes) {
   const genes = pathwayGenes.map(g => g.name)
   const uniqueGenes = Array.from(new Set(genes))
-  const dotPlotGenes = uniqueGenes.slice(0, 50)
-  if (!dotPlotGenes.includes(searchedGene)) {
-    dotPlotGenes[dotPlotGenes.length - 2] = searchedGene
-  }
-  if (!dotPlotGenes.includes(interactingGene)) {
-    dotPlotGenes[dotPlotGenes.length - 1] = interactingGene
-  }
 
-  return dotPlotGenes
+  const dotPlotGeneBatches = sliceIntoBatches(uniqueGenes, 50)
+
+  return dotPlotGeneBatches
 }
 
 /**
@@ -307,18 +325,33 @@ function writeLoadingIndicator(loadingCls) {
   headerLink.insertAdjacentHTML('afterend', loading)
 }
 
+/** Merge new and old dot plots metrics */
+function mergeDotPlotMetrics(newMetrics, oldMetrics) {
+  Object.entries(oldMetrics).map(([label, oldGeneMetrics]) => {
+    const newGeneMetrics = newMetrics[label]
+    if (!newGeneMetrics) {
+      return
+    }
+    newMetrics[label] = Object.assign(newGeneMetrics, oldGeneMetrics)
+  })
+
+  return newMetrics
+}
+
 /** Color pathway gene nodes by expression */
-function renderPathwayExpression(
+async function renderPathwayExpression(
   searchedGene, interactingGene,
   ideogram, dotPlotParams
 ) {
+  let allDotPlotMetrics = {}
+
   const ranks = ideogram.geneCache.interestingNames
   const pathwayGenes = getPathwayGenes(ranks)
-  const dotPlotGenes = getDotPlotGenes(searchedGene, interactingGene, pathwayGenes, ideogram)
-
+  const dotPlotGeneBatches = getDotPlotGeneBatches(pathwayGenes)
   const { studyAccession, cluster, annotation } = dotPlotParams
 
   let numDraws = 0
+  let numRenders = 0
 
   const annotationLabels = getEligibleLabels()
 
@@ -329,63 +362,88 @@ function renderPathwayExpression(
   function backgroundDotPlotDrawCallback(dotPlot) {
     // The first render is for uncollapsed cell-x-gene metrics (heatmap),
     // the second render is for collapsed label-x-gene metrics (dotplot)
+
     numDraws += 1
     if (numDraws === 1) {return}
 
     const dotPlotMetrics = getDotPlotMetrics(dotPlot)
+
     if (!dotPlotMetrics) {
       // Occurs upon resizing window, artifact of internal Morpheus handling
       // of pre-dot-plot heatmap matrix.  No user-facing impact.
       return
     }
-    writePathwayExpressionHeader(loadingCls, dotPlotMetrics, annotationLabels, pathwayGenes)
+
+    if (!annotationLabels.includes(Object.keys(dotPlotMetrics)[0])) {
+      // Another protection for computing only for dot plots, not heatmaps
+      return
+    }
+
+    allDotPlotMetrics = mergeDotPlotMetrics(dotPlotMetrics, allDotPlotMetrics)
+
+    writePathwayExpressionHeader(loadingCls, allDotPlotMetrics, annotationLabels, pathwayGenes)
 
     const annotationLabel = annotationLabels[0]
-    colorPathwayGenesByExpression(pathwayGenes, dotPlotMetrics, annotationLabel)
+    colorPathwayGenesByExpression(pathwayGenes, allDotPlotMetrics, annotationLabel)
+
+    if (numRenders <= dotPlotGeneBatches.length) {
+      numRenders += 1
+      // Future optimization: render background dot plot one annotation at a time.  This would
+      // speed up initial pathway expression overlay rendering, and increase the practical limit
+      // on number of genes that could be retrieved via SCP API Morpheus endpoint.
+      renderBackgroundDotPlot(
+        studyAccession, dotPlotGeneBatches[numRenders], cluster, annotation,
+        'All', annotationLabels, backgroundDotPlotDrawCallback,
+        '#related-genes-ideogram-container'
+      )
+    }
   }
 
   // Future optimization: render background dot plot one annotation at a time.  This would
   // speed up initial pathway expression overlay rendering, and increase the practical limit
   // on number of genes that could be retrieved via SCP API Morpheus endpoint.
   renderBackgroundDotPlot(
-    studyAccession, dotPlotGenes, cluster, annotation,
+    studyAccession, dotPlotGeneBatches[0], cluster, annotation,
     'All', annotationLabels, backgroundDotPlotDrawCallback,
     '#related-genes-ideogram-container'
   )
 }
 
+/** Draw pathway diagram */
+function drawPathway(event, dotPlotParams, ideogram) {
+  // Hide popover instantly upon drawing pathway; don't wait ~2 seconds
+  const ideoTooltip = document.querySelector('._ideogramTooltip')
+  ideoTooltip.style.opacity = 0
+  ideoTooltip.style.pointerEvents = 'none'
+
+  // Ensure popover for pathway diagram doesn't appear over gene search autocomplete,
+  // while still appearing over default visualizations.
+  const container = document.querySelector('#_ideogramPathwayContainer')
+  container.style.zIndex = 2
+
+  const details = event.detail
+  const searchedGene = details.sourceGene
+  const interactingGene = details.destGene
+  renderPathwayExpression(
+    searchedGene, interactingGene, ideogram,
+    dotPlotParams
+  )
+}
+
 /**
  * Add and remove event listeners for Ideogram's `ideogramDrawPathway` event
  *
  * This sets up the pathway expression overlay
  */
 export function manageDrawPathway(studyAccession, cluster, annotation, ideogram) {
-
   const flags = getFeatureFlagsWithDefaults()
   if (!flags?.show_pathway_expression) {return}
 
   const dotPlotParams = { studyAccession, cluster, annotation }
   if (annotation.type === 'group') {
-    document.removeEventListener('ideogramDrawPathway')
+    document.removeEventListener('ideogramDrawPathway', drawPathway)
     document.addEventListener('ideogramDrawPathway', event => {
-
-      // Hide popover instantly upon drawing pathway; don't wait ~2 seconds
-      const ideoTooltip = document.querySelector('._ideogramTooltip')
-      ideoTooltip.style.opacity = 0
-      ideoTooltip.style.pointerEvents = 'none'
-
-      // Ensure popover for pathway diagram doesn't appear over gene search autocomplete,
-      // while still appearing over default visualizations.
-      const container = document.querySelector('#_ideogramPathwayContainer')
-      container.style.zIndex = 2
-
-      const details = event.detail
-      const searchedGene = details.sourceGene
-      const interactingGene = details.destGene
-      renderPathwayExpression(
-        searchedGene, interactingGene, ideogram,
-        dotPlotParams
-      )
+      drawPathway(event, dotPlotParams, ideogram)
     })
   }
 }