API-21503: Remove VBADocuments Checksum Comparison Feature (Part 2) (#…

…11746)

* API-21503: Remove checksum comparison (failed experiment)

* API-21503: Remove supporting code for checksum comparison feature; feature was removed

* API-21503: Fix Tempfile parsing after reverting changes to MultipartParser

* API-21503: Use different fixture file for this spec

modules/vba_documents/app/controllers/vba_documents/v2/uploads_controller.rb

@@ -94,7 +94,7 @@ def submit
           upload_model.metadata['version'] = 2
           upload_model.save!
 
-          parts = VBADocuments::MultipartParser.parse(StringIO.new(request.raw_post))['contents']
+          parts = VBADocuments::MultipartParser.parse(StringIO.new(request.raw_post))
           inspector = VBADocuments::PDFInspector.new(pdf: parts)
           upload_model.update(uploaded_pdf: inspector.pdf_data)
 

modules/vba_documents/app/models/vba_documents/upload_file.rb

@@ -45,7 +45,7 @@ def remove_from_storage
     # Calling parses and uploads the PDFs / metadata.
     def parse_and_upload!
       parsed = multipart.open do |file|
-        VBADocuments::MultipartParser.parse(file.path)['contents']
+        VBADocuments::MultipartParser.parse(file.path)
       end
       parsed_files.attach(io: StringIO.new(parsed['metadata'].to_s), filename: "#{guid}_metadata.json")
       pdf_keys = parsed.keys - ['metadata']

modules/vba_documents/app/workers/vba_documents/upload_processor.rb

@@ -62,16 +62,16 @@ def download_and_process
         @upload.update(metadata: @upload.metadata.merge(original_file_metadata(tempfile)))
 
         parts = VBADocuments::MultipartParser.parse(tempfile.path)
-        inspector = VBADocuments::PDFInspector.new(pdf: parts['contents'])
+        inspector = VBADocuments::PDFInspector.new(pdf: parts)
         @upload.update(uploaded_pdf: inspector.pdf_data)
 
         # Validations
-        validate_parts(@upload, parts['contents'])
-        validate_metadata(parts['contents'][META_PART_NAME], submission_version: @upload.metadata['version'].to_i)
-        validate_documents(parts['contents'])
+        validate_parts(@upload, parts)
+        validate_metadata(parts[META_PART_NAME], submission_version: @upload.metadata['version'].to_i)
+        validate_documents(parts)
 
-        metadata = perfect_metadata(@upload, parts['contents'], timestamp)
-        response = submit(metadata, parts['contents'])
+        metadata = perfect_metadata(@upload, parts, timestamp)
+        response = submit(metadata, parts)
 
         process_response(response)
         log_submission(@upload, metadata)
@@ -82,7 +82,7 @@ def download_and_process
         retry_errors(e, @upload)
       ensure
         tempfile.close
-        close_part_files(parts['contents']) if parts.present? && parts['contents'].present?
+        close_part_files(parts) if parts.present?
       end
       response
     end

modules/vba_documents/lib/vba_documents/multipart_parser.rb

@@ -18,11 +18,7 @@ def self.parse(infile)
 
     # rubocop:disable Metrics/MethodLength
     def self.parse_file(infile)
-      parts = {
-        'boundaries' => {},
-        'headers' => {},
-        'contents' => {}
-      }
+      parts = {}
       begin
         input = if infile.is_a? String
                   File.open(infile, 'rb')
@@ -32,16 +28,13 @@ def self.parse_file(infile)
         validate_size(input)
         lines = input.each_line(LINE_BREAK).lazy.each_with_index
         separator = lines.next[0].chomp(LINE_BREAK)
-        parts['boundaries']['multipart_boundary'] = separator
         loop do
           headers = consume_headers(lines, separator)
           partname = get_partname(headers)
-          parts['headers'][partname] = headers.join(LINE_BREAK)
           content_type = get_content_type(headers)
-          body, closing_boundary = consume_body(lines, separator, content_type)
-          parts['contents'][partname] = body
-          parts['boundaries']['closing_boundary'] = closing_boundary
-          break unless closing_boundary.nil?
+          body, moreparts = consume_body(lines, separator, content_type)
+          parts[partname] = body
+          break unless moreparts
         end
       ensure
         input.close
@@ -151,11 +144,12 @@ def self.consume_body_string(lines, separator)
             raise VBADocuments::UploadError.new(code: 'DOC101',
                                                 detail: 'Unexpected end of payload')
           end
-          case line
-          when /^#{separator}--/
-            return tf.string, line
-          when /^#{separator}/
-            return tf.string, nil
+          linechomp = line.chomp(LINE_BREAK)
+          case linechomp
+          when "#{separator}--", "#{separator}--#{CARRIAGE_RETURN}"
+            return tf.string, false
+          when separator
+            return tf.string, true
           else
             tf.write(line)
           end
@@ -171,14 +165,14 @@ def self.consume_body_tempfile(lines, separator)
         rescue StopIteration
           raise VBADocuments::UploadError.new(code: 'DOC101', detail: 'Unexpected end of payload')
         end
-
-        case line
-        when /^#{separator}--/
+        linechomp = line.chomp(LINE_BREAK)
+        case linechomp
+        when "#{separator}--", "#{separator}--#{CARRIAGE_RETURN}"
           tf.rewind
-          return tf, line
-        when /^#{separator}/
+          return tf, false
+        when separator
           tf.rewind
-          return tf, nil
+          return tf, true
         else
           tf.write(line)
         end

modules/vba_documents/lib/vba_documents/payload_manager.rb

@@ -10,7 +10,7 @@ def self.zip(submission)
       raw_file, = download_raw_file(submission.guid)
 
       begin
-        parsed = VBADocuments::MultipartParser.parse(raw_file.path)['contents']
+        parsed = VBADocuments::MultipartParser.parse(raw_file.path)
         files = [
           { name: 'content.pdf', path: parsed['content'].path },
           { name: 'metadata.json', path: write_json(submission.guid, parsed).path }

modules/vba_documents/lib/vba_documents/pdf_inspector.rb

@@ -18,13 +18,13 @@ module Constants
 
     # If add_file_key is true the file is added to the returned hash as the parent key.
     # Useful for the rake task vba_documents:inspect_pdf
-    # pdf can be a String file path or the result of 'VBADocuments::MultipartParser.parse(tempfile.path)['contents']'
+    # pdf can be a String file path or the parts result of 'VBADocuments::MultipartParser.parse(tempfile.path)'
     def initialize(pdf:, add_file_key: false)
       if pdf.is_a?(String)
         raise ArgumentError, "Invalid file #{pdf}, does not exist!" unless File.exist? pdf
 
         @file = pdf
-        @parts = VBADocuments::MultipartParser.parse(@file)['contents']
+        @parts = VBADocuments::MultipartParser.parse(@file)
       else
         @parts = pdf
       end

modules/vba_documents/spec/lib/multipart_parser_spec.rb

@@ -24,55 +24,29 @@ def fetch(fixture, type)
     end
 
     context 'multipart_data_type' do
-      let(:expected_high_level_keys) { %w[boundaries headers contents] }
-      let(:metadata_header) do
-        "Content-Disposition: form-data; name=\"metadata\"\r\nContent-Type: application/json"
-      end
-      let(:content_header) do
-        "Content-Disposition: form-data; name=\"content\"; filename=\"valid_doc.pdf\"\r\nContent-Type: application/pdf"
-      end
-      let(:attachment1_header) do
-        "Content-Disposition: form-data; name=\"attachment1\"; filename=\"valid_doc.pdf\"\r\nContent-Type: " \
-          'application/pdf'
-      end
-
       FixtureHelper.data_type.each do |file_or_stringio|
         it "parses a valid multipart payload #{file_or_stringio}" do
           valid_doc = FixtureHelper.fetch(get_fixture('valid_multipart_pdf.blob'), file_or_stringio)
-          multipart_boundary = '----------------------------023987515673673391235769'
-          closing_boundary = "----------------------------023987515673673391235769--\r\n"
           result = described_class.parse(valid_doc)
 
-          expect(result.keys.sort).to eq(expected_high_level_keys.sort)
-
-          expect(result['boundaries']['multipart_boundary']).to eq(multipart_boundary)
-          expect(result['boundaries']['closing_boundary']).to eq(closing_boundary)
-
-          expect(result['headers']['metadata']).to eq(metadata_header)
-          expect(result['headers']['content']).to eq(content_header)
-
-          expect(result['contents']['metadata']).to be_a(String)
-          expect(result['contents']['content']).to be_a(Tempfile)
+          expect(result.size).to eq(2)
+          expect(result).to have_key('metadata')
+          expect(result['metadata']).to be_a(String)
+          expect(result).to have_key('content')
+          expect(result['content']).to be_a(Tempfile)
         end
 
         it "parses a valid multipart payload with attachments #{file_or_stringio}" do
           valid_doc = FixtureHelper.fetch(get_fixture('valid_multipart_pdf_attachments.blob'), file_or_stringio)
-          multipart_boundary = '----------------------------226100779484386681498651'
-          closing_boundary = "----------------------------226100779484386681498651--\r\n"
           result = described_class.parse(valid_doc)
 
-          expect(result.keys.sort).to eq(expected_high_level_keys.sort)
-
-          expect(result['boundaries']['multipart_boundary']).to eq(multipart_boundary)
-          expect(result['boundaries']['closing_boundary']).to eq(closing_boundary)
-
-          expect(result['headers']['metadata']).to eq(metadata_header)
-          expect(result['headers']['content']).to eq(content_header)
-          expect(result['headers']['attachment1']).to eq(attachment1_header)
-
-          expect(result['contents']['metadata']).to be_a(String)
-          expect(result['contents']['content']).to be_a(Tempfile)
-          expect(result['contents']['attachment1']).to be_a(Tempfile)
+          expect(result.size).to eq(3)
+          expect(result).to have_key('metadata')
+          expect(result['metadata']).to be_a(String)
+          expect(result).to have_key('content')
+          expect(result['content']).to be_a(Tempfile)
+          expect(result).to have_key('attachment1')
+          expect(result['attachment1']).to be_a(Tempfile)
         end
 
         it "raises on a malformed multipart payload #{file_or_stringio}" do
@@ -138,22 +112,15 @@ def fetch(fixture, type)
 
         it "handles a base64 payload #{file_or_stringio}" do
           valid_doc = FixtureHelper.fetch(get_fixture('base_64_with_attachment'), file_or_stringio)
-          multipart_boundary = '----------------------------226100779484386681498651'
-          closing_boundary = "----------------------------226100779484386681498651--\r\n"
           result = described_class.parse(valid_doc)
 
-          expect(result.keys.sort).to eq(expected_high_level_keys.sort)
-
-          expect(result['boundaries']['multipart_boundary']).to eq(multipart_boundary)
-          expect(result['boundaries']['closing_boundary']).to eq(closing_boundary)
-
-          expect(result['headers']['metadata']).to eq(metadata_header)
-          expect(result['headers']['content']).to eq(content_header)
-          expect(result['headers']['attachment1']).to eq(attachment1_header)
-
-          expect(result['contents']['metadata']).to be_a(String)
-          expect(result['contents']['content']).to be_a(Tempfile)
-          expect(result['contents']['attachment1']).to be_a(Tempfile)
+          expect(result.size).to eq(3)
+          expect(result).to have_key('metadata')
+          expect(result['metadata']).to be_a(String)
+          expect(result).to have_key('content')
+          expect(result['content']).to be_a(Tempfile)
+          expect(result).to have_key('attachment1')
+          expect(result['attachment1']).to be_a(Tempfile)
         end
 
         it "logs base64 decoding progress when handling a base64 payload #{file_or_stringio}" do

modules/vba_documents/spec/request/v1/uploads_request_spec.rb

@@ -137,7 +137,7 @@
         @upload_submission.update(status: 'uploaded')
         allow_any_instance_of(VBADocuments::UploadProcessor).to receive(:cancelled?).and_return(false)
         allow(VBADocuments::MultipartParser).to receive(:parse) {
-          { 'contents' => { 'metadata' => @md.to_json, 'content' => valid_doc } }
+          { 'metadata' => @md.to_json, 'content' => valid_doc }
         }
         allow(CentralMail::Service).to receive(:new) { client_stub }
         allow(faraday_response).to receive(:status).and_return(200)
@@ -225,10 +225,8 @@
 
     let(:valid_parts) do
       {
-        'contents' => {
-          'metadata' => valid_metadata,
-          'content' => valid_doc
-        }
+        'metadata' => valid_metadata,
+        'content' => valid_doc
       }
     end
 

modules/vba_documents/spec/request/v2/uploads_request_spec.rb

@@ -205,7 +205,7 @@
         @upload_submission.update(status: 'uploaded')
         allow_any_instance_of(VBADocuments::UploadProcessor).to receive(:cancelled?).and_return(false)
         allow(VBADocuments::MultipartParser).to receive(:parse) {
-          { 'contents' => { 'metadata' => @md.to_json, 'content' => valid_doc } }
+          { 'metadata' => @md.to_json, 'content' => valid_doc }
         }
         allow(CentralMail::Service).to receive(:new) { client_stub }
         allow(faraday_response).to receive(:status).and_return(200)
@@ -291,10 +291,8 @@
 
     let(:valid_parts) do
       {
-        'contents' => {
-          'metadata' => valid_metadata,
-          'content' => valid_doc
-        }
+        'metadata' => valid_metadata,
+        'content' => valid_doc
       }
     end