Merge pull request #732 from nicolasbouillon/master

PDF conversion patch + Collect a submission patch [corrected]
MarkUsProject · Apr 1, 2012 · dc2c0f1 · dc2c0f1
2 parents d1e15e8 + feeea45
commit dc2c0f1
Show file tree

Hide file tree

Showing 14 changed files with 135 additions and 109 deletions.
diff --git a/Gemfile b/Gemfile
@@ -70,3 +70,11 @@ end
 group :mongrel do
   gem "mongrel_cluster"
 end
+
+# If you want to be able to view and annotate PDF files, 
+# make sure that this group is included. GhostScript has to be 
+# installed for rghost to work well. You also need to set
+# the PDF_SUPPORT bool to true in the config file(s).
+group :rghost do
+  gem "rghost"
+end
diff --git a/Gemfile.lock b/Gemfile.lock
@@ -94,6 +94,7 @@ GEM
     rbx-require-relative (0.0.5)
     rcov (0.9.10)
     rdoc (3.9.2)
+    rghost (0.8.7.6)
     routing-filter (0.2.4)
       actionpack
     ruby-debug (0.10.4)
@@ -134,6 +135,7 @@ DEPENDENCIES
   rails
   rcov
   rdoc
+  rghost
   routing-filter
   ruby-debug
   rubyzip

diff --git a/app/controllers/results_controller.rb b/app/controllers/results_controller.rb
@@ -198,8 +198,8 @@ def download
     elsif file.is_pdf? && !params[:show_in_browser].nil?
       send_file File.join(MarkusConfigurator.markus_config_pdf_storage,
         file.submission.grouping.group.repository_name, file.path,
-        filename.split('.')[0] + '.jpg'), :type => "image",
-        :disposition => 'inline', :filename => filename
+        filename.split('.')[0] + '_' + sprintf("%04d" % params[:file_index].to_s()) + '.jpg'),
+        :type => "image", :disposition => 'inline', :filename => filename
     else
       send_data file_contents, :filename => filename
     end
@@ -234,6 +234,22 @@ def codeviewer
       return
     end
     @code_type = @file.get_file_type
+
+    # if dealing with a pdf file, get the number of images to display
+    if @file.is_pdf? 
+      i = 1 
+      storage_path = File.join(MarkusConfigurator.markus_config_pdf_storage,
+        @file.submission.grouping.group.repository_name,
+        @file.path)
+      filePathToCheck = File.join(storage_path, @file.filename.split('.')[0] + '_' + sprintf("%04d" % i.to_s()) + '.jpg')
+      while File.exists?(filePathToCheck)
+        i += 1
+        filePathToCheck = File.join(storage_path, @file.filename.split('.')[0] + '_' + sprintf("%04d" % i.to_s()) + '.jpg')
+      end
+      i -= 1
+      @nb_pdf_files_to_download = i
+    end
+
     render :template => 'results/common/codeviewer'
   end
 

diff --git a/app/models/submission_file.rb b/app/models/submission_file.rb
@@ -109,25 +109,27 @@ def convert_pdf_to_jpg
       self.path)
     file_path = File.join(storage_path, self.filename.split('.')[0] + '.jpg')
     self.export_file(storage_path)
+
     # Remove any old copies of this image if they exist
-    FileUtils.remove_file(file_path, true) if File.exists?(file_path)
-    m_logger.log("Starting pdf conversion from #{File.join(storage_path, self.filename)} to #{file_path}")
-    # ImageMagick can only save images with heights not exceeding 65500 pixels.
-    # Larger images result in a conversion failure.
-    begin
-      # This is an ImageMagick command, see http://www.imagemagick.org/script/convert.php for documentation
-      # For some reason, ImageMagick seemed to fail silently when not
-      # redirecting the output to a log file. Let's redirect the output to
-      # "something"
-      `convert -limit memory #{MarkusConfigurator.markus_config_pdf_conv_memory_allowance} -limit map 0 -density 150 -resize 66% #{File.join(storage_path, self.filename)} -append #{file_path} >> #{File.join(RAILS_ROOT, "log", "export-pdf.log")}`
-      # Sometimes, ImageMagick fails silently
-      if File.exists?(file_path)
-        m_logger.log("Successfully converted pdf file to jpg")
-      else
-        m_logger.log("Problem in PDF conversion")
-      end
-    rescue Exception => e
-      m_logger.log("Pdf file couldn't be converted")
+    i = 1
+    filePathToRemove = File.join(storage_path,
+                                 self.filename.split('.')[0] + '_' + sprintf("%04d" % i.to_s()) + '.jpg') 
+    while File.exists?(filePathToRemove)
+      FileUtils.remove_file(filePathToRemove, true)
+      i += 1
+      filePathToRemove = File.join(storage_path,
+                                   self.filename.split('.')[0] + '_' + sprintf("%04d" % i.to_s()) + '.jpg')
+    end
+
+    # Convert a pdf file into a an array of jpg files (one jpg file = one page
+    # of the pdf file)
+    file = RGhost::Convert.new(File.join(storage_path,
+                                  self.filename)).to :jpeg,
+                        :filename => file_path,
+                        :multipage => true,
+                        :resolution => 150
+    if file.error
+      m_logger.log("rghost: Image conversion error")
     end
 
     FileUtils.remove_file(File.join(storage_path, self.filename), true)

diff --git a/app/views/results/common/_pdf_codeviewer.html.erb b/app/views/results/common/_pdf_codeviewer.html.erb
@@ -0,0 +1,33 @@
+<div id="image_container" class="image_container">
+  <div id="sel_box"></div>
+    <%# Create a box for each image annotation %>
+    <% @annots.each do |annot| %>
+      <div id="annotation_holder_<%=annot.annotation_text.id%>" class="annotation_holder"></div>
+    <%end%>
+   <div id="image_preview">
+       <%# First image displayed with a different alt message, in case of boolean PDF_SUPPORT set to false %>
+       <li>
+         <%= image_tag(url_for(:action => 'download', :controller => 'results', :select_file_id => submission_file_id, :show_in_browser => true, :file_index => 1), :alt => I18n.t("common.cant_display_image"), :width => "950") %>
+       </li>
+       <% (2..nb_files).each do|i| %>
+         <li>      
+	   <%= image_tag(url_for(:action => 'download', :controller => 'results', :select_file_id => submission_file_id, :show_in_browser => true, :file_index => i), :alt => I18n.t("common.image_downloading"), :width => "950") %>
+         </li>
+       <% end %>
+   </div>
+
+  <%# Keep track of all annotations associated with file  %>
+  <input id="annotation_grid"type="hidden" value="<%=h(@file.get_annotation_grid.to_json)%>"></input>
+  <input id="enable_annotations?"type="hidden" value=<%=(@current_user.ta? || @current_user.admin?) && !@result.released_to_students%>></input>
+  <script type="text/javascript">
+  //<![CDATA[
+    source_code_ready_for_image();
+    <% if (defined? annots) %>
+      <% annots.each do |annot| %>
+          add_annotation_text(<%=annot.annotation_text.id%>,
+          '<%=simple_format(h(escape_javascript(annot.annotation_text.content.to_s)))%>');
+      <% end %>
+    <% end %>
+  //]]>
+  </script>
+</div>
diff --git a/app/views/results/common/codeviewer.rjs b/app/views/results/common/codeviewer.rjs
@@ -1,20 +1,27 @@
 #Render the source code for syntax highlighting...
   begin
     page['code_pane'].onmousemove = nil
-    #Non-binary files
+    # Non-binary files
     if !SubmissionFile.is_binary?(@file_contents)
       page.replace_html 'codeviewer',
         :partial => 'results/common/text_codeviewer',
         :locals => { :uid => params[:uid], :file_contents => @file_contents,
         :annots => @annots, :code_type => @code_type}
-    #Supported image files and pdfs
-    elsif @file.is_supported_image? || @file.is_pdf?
+    # Supported image files
+    elsif @file.is_supported_image?
       page.replace_html 'codeviewer',
         :partial => 'results/common/image_codeviewer',
         :locals => { :uid => params[:uid], :file_contents => @file_contents,
         :annots => @annots, :code_type => @code_type,
         :submission_file_id => @submission_file_id}
-    #Rest of the files
+    # Pdf files    
+    elsif @file.is_pdf?
+      page.replace_html 'codeviewer',
+        :partial => 'results/common/pdf_codeviewer',
+        :locals => { :uid => params[:uid], :file_contents => @file_contents,
+        :annots => @annots, :code_type => @code_type,
+        :nb_files => @nb_pdf_files_to_download,
+        :submission_file_id => @submission_file_id}
     else
       page.replace_html 'codeviewer',
         :partial => 'results/common/text_codeviewer',
@@ -23,7 +30,7 @@
         :annots => @annots, :code_type => @code_type}
     end
 
-    #Also update the annotation_summary_list
+    # Also update the annotation_summary_list
     if @current_user.ta? || @current_user.admin?
       page.replace_html 'annotation_summary_list',
         :partial => 'results/marker/annotation_summary',

diff --git a/app/views/submissions/update_converted_pdfs.rjs b/app/views/submissions/update_converted_pdfs.rjs
@@ -7,12 +7,16 @@ if submission_collector.safely_stop_child_exited
     submission_collector.safely_stop_child_exited = false
     submission_collector.save
       page.redirect_to :controller => 'results', :action => 'edit',
-        :id => @grouping.current_submission_used.result.id
+      :assignment_id => @grouping.assignment_id,
+      :submission_id => @grouping.current_submission_used.id,
+      :id => @grouping.current_submission_used.result.id
     end
   else
     submission_collector.safely_stop_child_exited = false
     submission_collector.save
     page.redirect_to :controller => 'results', :action => 'edit',
+      :assignment_id => @grouping.assignment_id,
+      :submission_id => @grouping.current_submission_used.id,
       :id => @grouping.current_submission_used.result.id
   end
 end
diff --git a/config/environments/development.rb b/config/environments/development.rb
@@ -108,6 +108,7 @@
   ###################################################################
   # Directory where converted PDF files will be stored as JPEGs. Make sure MarkUs
   # is allowed to write to this directory
+
   PDF_STORAGE = "#{::Rails.root.to_s}/data/dev/pdfs"
 
   ###################################################################
@@ -118,32 +119,9 @@
   ###################################################################
   # Set this to true or false if you want to be able to display and annotate
   # PDF documents within the browser.
-  PDF_SUPPORT = false
-
-  ###################################################################
-  # In order for markus to display pdfs, it converts them to jpg format via
-  # ImageMagick first. The conversion process is very expensive and can quickly
-  # eat up all available RAM and swap memory available to the server causing it
-  # to crash. The solution to this is to set a limit on how much RAM ImageMagick
-  # is allowed to use, forcing it to use the hard-disk for all the needs exceeding
-  # the allowance.
-  #
-  # Using hard-disk memory for conversion is significantly slower than using RAM.
-  # Increasing the memory allowance will help speed up conversion speeds.
-  #
-  # Caution: setting the allowance too high will result in ImageMagick using all
-  # the server RAM and will crash it. Be sure that you can afford the memory you
-  # allow.
-  #
-  # The maximum amount of megabytes that the ImageMagick pdf conversion process
-  # may use. This setting is NOT a limit on MarkUs's total memory use. It only
-  # limits the ImageMagick conversion process.
-  #
-  # 100 mbs should be enough to quickly convert most submissions around 10 pages
-  # long.
-  #
-  # This setting doesn't matter unless PDF_SUPPORT is set to true
-  PDF_CONV_MEMORY_ALLOWANCE = 100
+  # When collecting pdfs files, it converts them to jpg format via RGhost.
+  # RGhost is ghostscript dependent. Be sure ghostscript is installed.
+  PDF_SUPPORT = false 
 
   ###################################################################
   # Change this to 'REPOSITORY_EXTERNAL_SUBMITS_ONLY = true' if you

diff --git a/config/environments/production.rb b/config/environments/production.rb
@@ -142,32 +142,9 @@
   ###################################################################
   # Set this to true or false if you want to be able to display and annotate
   # PDF documents within the browser.
-  PDF_SUPPORT = true
-
-  ###################################################################
-  # In order for markus to display pdfs, it converts them to jpg format via
-  # ImageMagick first. The conversion process is very expensive and can quickly
-  # eat up all available RAM and swap memory available to the server causing it
-  # to crash. The solution to this is to set a limit on how much RAM ImageMagick
-  # is allowed to use, forcing it to use the hard-disk for all the needs exceeding
-  # the allowance.
-  #
-  # Using hard-disk memory for conversion is significantly slower than using RAM.
-  # Increasing the memory allowance will help speed up conversion speeds.
-  #
-  # Caution: setting the allowance too high will result in ImageMagick using all
-  # the server RAM and will crash it. Be sure that you can afford the memory you
-  # allow.
-  #
-  # The maximum amount of megabytes that the ImageMagick pdf conversion process
-  # may use. This setting is NOT a limit on MarkUs's total memory use. It only
-  # limits the ImageMagick conversion process.
-  #
-  # 100 mbs should be enough to quickly convert most submissions around 10 pages
-  # long.
-  #
-  # This setting doesn't matter unless PDF_SUPPORT is set to true
-  PDF_CONV_MEMORY_ALLOWANCE = 100
+  # When collecting pdfs files, it converts them to jpg format via RGhost.
+  # RGhost is ghostscript dependent. Be sure ghostscript is installed.
+  PDF_SUPPORT = false
 
   ###################################################################
   # Change this to 'REPOSITORY_EXTERNAL_SUBMITS_ONLY = true' if you

diff --git a/config/environments/test.rb b/config/environments/test.rb
@@ -117,33 +117,9 @@
   ###################################################################
   # Set this to true or false if you want to be able to display and annotate
   # PDF documents within the browser.
-  PDF_SUPPORT = true
-
-  ###################################################################
-  # In order for markus to display pdfs, it converts them to jpg format via
-  # ImageMagick first. The conversion process is very expensive and can quickly
-  # eat up all available RAM and swap memory available to the server causing it
-  # to crash. The solution to this is to set a limit on how much RAM ImageMagick
-  # is allowed to use, forcing it to use the hard-disk for all the needs exceeding
-  # the allowance.
-  #
-  # Using hard-disk memory for conversion is significantly slower than using RAM.
-  # Increasing the memory allowance will help speed up conversion speeds.
-  #
-  # Caution: setting the allowance too high will result in ImageMagick using all
-  # the server RAM and will crash it. Be sure that you can afford the memory you
-  # allow.
-  #
-  # The maximum amount of megabytes that the ImageMagick pdf conversion process
-  # may use. This setting is NOT a limit on MarkUs's total memory use. It only
-  # limits the ImageMagick conversion process.
-  #
-  # 100 mbs should be enough to quickly convert most submissions around 10 pages
-  # long.
-  #
-  # This setting doesn't matter unless PDF_SUPPORT is set to true
-  PDF_CONV_MEMORY_ALLOWANCE = 100
-
+  # When collecting pdfs files, it converts them to jpg format via RGhost.
+  # RGhost is ghostscript dependent. Be sure ghostscript is installed.
+  PDF_SUPPORT = false
 
   ###################################################################
   # Change this to 'REPOSITORY_EXTERNAL_SUBMITS_ONLY = true' if you

diff --git a/config/locales/en.yml b/config/locales/en.yml
@@ -295,7 +295,8 @@ en:
       submission_file: "Submission File:"
       test_results: "Test Results:"
       no_file_in_repository: "[No files in repository]"
-      cant_display_image: "Image cannot be displayed. If you are trying to view a pdf file its probably too big to display or you have turned off pdf support. Click on the Download button to download the file instead"
+      cant_display_image: "Image cannot be displayed. If you are trying to view a pdf file, please check if you have turned on pdf support. Click on the Download button to download the file instead"
+      image_downloading: "Pdf page downloading. Please wait."
       test_code: "Test Code"
       criterion_incomplete_error: "Mark unfilled but marking state was set to complete"
 

diff --git a/config/locales/fr.yml b/config/locales/fr.yml
@@ -295,7 +295,8 @@ fr:
       submission_file: "Fichier envoyé : "
       test_results: "Résultat des tests : "
       no_file_in_repository: "[Pas de fichiers dans le dépôt]"
-      cant_display_image: "L'image ne peut pas être affichée. Si vous essayez d'afficher un PDF, ou le fichier est trop grand pour être affiché, ou l'option permettant d'afficher les PDF est désactivée. Vous pouvez télécharger manuellement le fichier."
+      cant_display_image: "L'image ne peut pas être affichée. Si vous essayez d'afficher un PDF, vérifiez que l'option permettant d'afficher les PDF est activée. Vous pouvez télécharger manuellement le fichier."
+      image_downloading: "Page pdf en cours de téléchargement. Veuillez patienter."
       test_code: "Code pour les tests"
       criterion_incomplete_error: "Note non remplie, mais état fixé comme terminé"
 

diff --git a/config/routes.rb b/config/routes.rb
@@ -136,8 +136,12 @@
 
       resources :results do
         collection do
-          post 'update_mark'
-          post 'expand_criteria'
+          get 'update_mark'
+          get 'expand_criteria'
+          get 'collapse_criteria'
+          get 'expand_unmarked_criteria'
+          get 'edit'
+          get 'download'
         end
 
         member do

diff --git a/test/functional/results_controller_test.rb b/test/functional/results_controller_test.rb
@@ -16,6 +16,23 @@ def teardown
 
   SAMPLE_ERR_MSG = "sample error message"
 
+  should "recognize routes" do
+    assert_recognizes({:controller => "results",
+                       :action => "update_mark",
+                       :assignment_id => '1',
+                       :submission_id => '1'},
+                      {:path => 'assignments/1/submissions/1/results/update_mark',
+                       :method => :get})
+
+    assert_recognizes({:controller => "results",
+                       :action => "expand_criteria",
+                       :assignment_id => '1',
+                       :submission_id => '1'},
+                      {:path => 'assignments/1/submissions/1/results/expand_criteria',
+                       :method => :get})
+
+  end
+
   context "A user" do
 
     # Since we are not authenticated and authorized, we should be redirected