Refactor file viewer components (#7270)

* Return extra file information for submission files
* Remove get_file route
* Move HEIC/HEIF file handling to the image viewer
* Refactor text viewer to fetch content from URL
* Cancel promises and state updates after component unmount
* Add `max_content_size` param to `/download_file`
* Handle (413 Content Too Large) when fetching file content
* Add binary file extensions

Changelog.md

@@ -14,6 +14,7 @@
 
 - Reorganize Javascript files to separate Sprockets and Webpack compilation (#7345)
 - Replace moment.js dependency with much smaller dayjs (#7346)
+- Refactor `FileViewer`, `TextViewer`, `BinaryViewer` and `ImageViewer` components (#7270)
 
 ## [v2.6.1]
 

app/controllers/results_controller.rb

@@ -80,7 +80,10 @@ def show
         data[:can_release] = allowed_to?(:manage_assessments?, current_role)
 
         # Submission files
-        file_data = submission.submission_files.order(:path, :filename).pluck_to_hash(:id, :filename, :path)
+        file_data = submission.submission_files.order(:path, :filename).pluck_to_hash(:id, :filename, :path) do |hash|
+          hash[:type] = FileHelper.get_file_type(hash[:filename])
+          hash
+        end
         file_data.reject! { |f| Repository.get_class.internal_file_names.include? f[:filename] }
         data[:submission_files] = file_data
 

app/controllers/submissions_controller.rb

@@ -472,64 +472,6 @@ def update_files
     head :bad_request
   end
 
-  def get_file
-    submission = record
-    grouping = submission.grouping
-    assignment = grouping.assignment
-
-    if !current_role.is_a_reviewer?(assignment.pr_assignment) && current_role.student? &&
-      current_role.accepted_grouping_for(assignment.id).id != grouping.id
-      flash_message(:error,
-                    t('submission_file.error.no_access',
-                      submission_file_id: params[:submission_file_id]))
-      redirect_back(fallback_location: root_path)
-      return
-    end
-
-    file = SubmissionFile.find(params[:submission_file_id])
-    file_size = begin
-      file.retrieve_file.size
-    rescue StandardError
-      0
-    end
-    if file.is_supported_image?
-      render json: { type: 'image', size: file_size }
-    elsif file.is_pdf?
-      render json: { type: 'pdf', size: file_size }
-    elsif file.is_pynb?
-      render json: { type: 'jupyter-notebook', size: file_size }
-    else
-      grouping.access_repo do |repo|
-        revision = repo.get_revision(submission.revision_identifier)
-        raw_file = revision.files_at_path(file.path)[file.filename]
-        file_type = FileHelper.get_file_type(file.filename)
-        if raw_file.nil?
-          file_contents = I18n.t('student.submission.missing_file', file_name: file.filename)
-          file_type = 'unknown'
-        else
-          file_contents = repo.download_as_string(raw_file)
-          file_contents.encode!('UTF-8', invalid: :replace, undef: :replace, replace: '�')
-
-          file_type = 'unknown' unless file_type != 'markusurl' || assignment.url_submit
-
-          if params[:force_text] != 'true' && SubmissionFile.is_binary?(file_contents)
-            # If the file appears to be binary, display a warning
-            file_contents = I18n.t('submissions.cannot_display')
-            file_type = 'binary'
-          end
-        end
-
-        max_content_size = params[:max_content_size].blank? ? -1 : params[:max_content_size].to_i
-        # Omit content if it exceeds the maximum size requests by the client
-        render json: {
-          content: file_size <= max_content_size || max_content_size == -1 ? file_contents.to_json : '',
-          type: file_type,
-          size: file_size
-        }
-      end
-    end
-  end
-
   def download_file
     if params[:download_zip_button]
       download_file_zip
@@ -558,6 +500,13 @@ def download_file
       head :internal_server_error
       return
     end
+
+    max_content_size = params[:max_content_size].blank? ? -1 : params[:max_content_size].to_i
+    if max_content_size != -1 && file_contents.size > max_content_size
+      head :payload_too_large
+      return
+    end
+
     filename = file.filename
     # Display the file in the page if it is an image/pdf, and download button
     # was not explicitly pressed

app/javascript/Components/Result/binary_viewer.jsx

@@ -1,11 +1,81 @@
 import React from "react";
 
 export class BinaryViewer extends React.PureComponent {
+  constructor(props) {
+    super(props);
+    this.state = {
+      content: null,
+      getAnyway: false,
+    };
+    this.abortController = null;
+  }
+
+  componentDidMount() {
+    // Notify the parent component that the file content is loading.
+    this.props.setLoadingCallback(true);
+    // The URL has updated, so the content needs to be fetched using the new URL.
+    this.fetchContent(this.props.url)
+      .then(content =>
+        this.setState({content: content}, () => this.props.setLoadingCallback(false))
+      )
+      .catch(error => {
+        this.props.setLoadingCallback(false);
+        if (error instanceof DOMException) return;
+        console.error(error);
+      });
+  }
+
+  componentDidUpdate(prevProps, prevState) {
+    if (this.props.url && this.props.url !== prevProps.url) {
+      this.setState({getAnyway: false});
+      this.props.setLoadingCallback(true);
+      this.fetchContent(this.props.url)
+        .then(content =>
+          this.setState({content: content}, () => this.props.setLoadingCallback(false))
+        )
+        .catch(error => {
+          this.props.setLoadingCallback(false);
+          if (error instanceof DOMException) return;
+          console.error(error);
+        });
+    }
+  }
+
+  fetchContent(url) {
+    if (this.abortController) {
+      // Stops ongoing fetch requests. It's ok to call .abort() after the fetch has already completed,
+      // fetch simply ignores it.
+      this.abortController.abort();
+    }
+    // Reinitialize the controller, because the signal can't be reused after the request has been aborted.
+    this.abortController = new AbortController();
+
+    return fetch(url, {signal: this.abortController.signal})
+      .then(response => {
+        if (response.status === 413) {
+          const errorMessage = I18n.t("submissions.oversize_submission_file");
+          this.props.setErrorMessageCallback(errorMessage);
+          throw new Error(errorMessage);
+        } else {
+          return response.text();
+        }
+      })
+      .then(content => content.replace(/\r?\n/gm, "\n"));
+  }
+
+  componentWillUnmount() {
+    if (this.abortController) {
+      this.abortController.abort();
+    }
+  }
+
   render() {
     return (
       <div>
-        <p>{this.props.content}</p>
-        <a onClick={this.props.getAnyway}>{I18n.t("submissions.get_anyway")}</a>
+        {!this.state.getAnyway && (
+          <a onClick={() => this.setState({getAnyway: true})}>{I18n.t("submissions.get_anyway")}</a>
+        )}
+        {this.state.getAnyway && <p>{this.state.content}</p>}
       </div>
     );
   }