Merge pull request #358 from sul-dlss/symph-bytes

SymphonyReader validates num bytes in resp body

app/controllers/marcxml_controller.rb

@@ -3,6 +3,10 @@
 class MarcxmlController < ApplicationController #:nodoc:
   before_action :set_marcxml_resource
 
+  rescue_from(SymphonyReader::RecordIncompleteError) do |e|
+    render status: :internal_server_error, plain: e.message
+  end
+
   def catkey
     render plain: @marcxml.catkey
   end

app/controllers/metadata_refresh_controller.rb

@@ -4,6 +4,10 @@
 class MetadataRefreshController < ApplicationController
   before_action :load_item
 
+  rescue_from(SymphonyReader::RecordIncompleteError) do |e|
+    render status: :internal_server_error, plain: e.message
+  end
+
   def refresh
     status = RefreshMetadataAction.run(@item)
     @item.save if status

app/controllers/objects_controller.rb

@@ -15,6 +15,10 @@ class ObjectsController < ApplicationController
     render status: :internal_server_error, plain: e.message
   end
 
+  rescue_from(SymphonyReader::RecordIncompleteError) do |e|
+    render status: :internal_server_error, plain: e.message
+  end
+
   # Register new objects in DOR
   def create
     Rails.logger.info(params.inspect)

app/models/symphony_reader.rb

@@ -2,6 +2,8 @@
 
 # Reader from symphony's JSON API to a MARC record
 class SymphonyReader
+  class RecordIncompleteError < RuntimeError; end
+
   attr_reader :catkey
 
   def self.client
@@ -15,7 +17,8 @@ def initialize(catkey:)
   def to_marc
     record = MARC::Record.new
 
-    record.leader = leader if leader
+    # note that new record already has default leader, but we don't want it unless it's from Symphony
+    record.leader = leader
 
     fields.uniq.each do |field|
       record << marc_field(field) unless %w[001 003].include? field['tag'] # explicitly remove all 001 and 003 fields from the record
@@ -36,8 +39,22 @@ def client
     self.class.client
   end
 
+  def symphony_response
+    resp = client.get(format(Settings.catalog.symphony.json_url, catkey: catkey))
+    validate_response(resp)
+  end
+
+  def validate_response(resp)
+    exp_content_length = resp.headers['Content-Length'].to_i
+    actual_content_length = resp.body.length
+    return resp if actual_content_length == exp_content_length
+
+    errmsg = "Incomplete response received from Symphony for #{@catkey} - expected #{exp_content_length} bytes but got #{actual_content_length}"
+    raise RecordIncompleteError, errmsg
+  end
+
   def json
-    @json ||= JSON.parse(client.get(format(Settings.catalog.symphony.json_url, catkey: catkey)).body)
+    @json ||= JSON.parse(symphony_response.body)
   end
 
   def bib_record

spec/controllers/marcxml_controller_spec.rb

@@ -8,6 +8,29 @@
   end
 
   let(:resource) { MarcxmlResource.new(catkey: '12345') }
+  let(:body) do
+    {
+      resource: '/catalog/bib',
+      key: '111',
+      fields: {
+        bib: {
+          standard: 'MARC21',
+          type: 'BIB',
+          leader: '00956cem 2200229Ma 4500',
+          fields: [
+            { tag: '001', subfields: [{ code: '_', data: 'some data' }] },
+            { tag: '001', subfields: [{ code: '_', data: 'some other data' }] },
+            { tag: '009', subfields: [{ code: '_', data: 'whatever' }] },
+            {
+              tag: '245',
+              inds: '41',
+              subfields: [{ code: 'a', data: 'some data' }]
+            }
+          ]
+        }
+      }
+    }
+  end
 
   describe 'GET catkey' do
     it 'returns the provided catkey' do
@@ -24,18 +47,41 @@
 
   describe 'GET marcxml' do
     it 'retrieves MARCXML' do
-      stub_request(:get, format(Settings.catalog.symphony.json_url, catkey: resource.catkey)).to_return(body: '{}')
-
+      stub_request(:get, format(Settings.catalog.symphony.json_url, catkey: resource.catkey)).to_return(body: body.to_json, headers: { 'Content-Length': 394 })
       get :marcxml, params: { catkey: '12345' }
       expect(response.body).to start_with '<record'
     end
+
+    context 'when incomplete response from Symphony' do
+      before do
+        stub_request(:get, format(Settings.catalog.symphony.json_url, catkey: resource.catkey)).to_return(body: '{}', headers: { 'Content-Length': 0 })
+      end
+
+      it 'returns a 500 error' do
+        get :marcxml, params: { catkey: '12345' }
+        expect(response.status).to eq(500)
+        expect(response.body).to eq('Incomplete response received from Symphony for 12345 - expected 0 bytes but got 2')
+      end
+    end
   end
 
   describe 'GET mods' do
     it 'transforms the MARCXML into MODS' do
-      stub_request(:get, format(Settings.catalog.symphony.json_url, catkey: resource.catkey)).to_return(body: '{}')
+      stub_request(:get, format(Settings.catalog.symphony.json_url, catkey: resource.catkey)).to_return(body: body.to_json, headers: { 'Content-Length': 394 })
       get :mods, params: { catkey: '12345' }
       expect(response.body).to match(/mods/)
     end
+
+    context 'when incomplete response from Symphony' do
+      before do
+        stub_request(:get, format(Settings.catalog.symphony.json_url, catkey: resource.catkey)).to_return(body: '{}', headers: { 'Content-Length': 0 })
+      end
+
+      it 'returns a 500 error' do
+        get :mods, params: { catkey: '12345' }
+        expect(response.status).to eq(500)
+        expect(response.body).to eq('Incomplete response received from Symphony for 12345 - expected 0 bytes but got 2')
+      end
+    end
   end
 end

spec/controllers/objects_controller_spec.rb

@@ -26,6 +26,8 @@
     render_views
 
     context 'error handling' do
+      let(:errmsg) { 'my unique snowflake error message' }
+
       it 'returns a 409 error with location header when an object already exists' do
         allow(RegistrationService).to receive(:register_object).and_raise(Dor::DuplicateIdError.new('druid:existing123obj'))
         post :create
@@ -34,9 +36,17 @@
       end
 
       it 'returns a 422 error when an object has a bad name' do
-        allow(RegistrationService).to receive(:register_object).and_raise(ArgumentError)
+        allow(RegistrationService).to receive(:register_object).and_raise(ArgumentError.new(errmsg))
         post :create
         expect(response.status).to eq(422)
+        expect(response.body).to eq(errmsg)
+      end
+
+      it 'returns a 500 error when the SymphonyReader gets an incomplete response' do
+        allow(RegistrationService).to receive(:register_object).and_raise(SymphonyReader::RecordIncompleteError.new(errmsg))
+        post :create
+        expect(response.status).to eq(500)
+        expect(response.body).to eq(errmsg)
       end
     end
 

spec/models/symphony_reader_spec.rb

@@ -8,17 +8,22 @@
 
   describe '#to_marc' do
     before do
-      stub_request(:get, format(Settings.catalog.symphony.json_url, catkey: catkey)).to_return(body: body.to_json)
+      stub_request(:get, format(Settings.catalog.symphony.json_url, catkey: catkey)).to_return(body: body.to_json, headers: headers)
     end
 
     let(:body) do
       {
+        resource: '/catalog/bib',
+        key: '111',
         fields: {
           bib: {
+            standard: 'MARC21',
+            type: 'BIB',
             leader: '00956cem 2200229Ma 4500',
             fields: [
               { tag: '001', subfields: [{ code: '_', data: 'some data' }] },
-              { tag: '001', subfields: [{ code: '_', data: 'some data' }] },
+              { tag: '001', subfields: [{ code: '_', data: 'some other data' }] },
+              { tag: '009', subfields: [{ code: '_', data: 'whatever' }] },
               {
                 tag: '245',
                 inds: '41',
@@ -29,6 +34,8 @@
         }
       }
     end
+    let(:headers) { { 'Content-Length': 394 } }
+
     it 'converts symphony json to marc records' do
       expect(reader.to_marc).to be_a_kind_of MARC::Record
     end
@@ -38,11 +45,12 @@
     end
 
     it 'parses control fields' do
-      expect(reader.to_marc.fields('001').first.value).to eq 'acatkey'
+      expect(reader.to_marc.fields('009').first.value).to eq 'whatever'
     end
 
-    it 'removes duplicate fields' do
+    it 'removes original 001 fields and puts catkey in 001 field' do
       expect(reader.to_marc.fields('001').length).to eq 1
+      expect(reader.to_marc.fields('001').first.value).to eq 'acatkey'
     end
 
     it 'parses data fields' do
@@ -52,5 +60,12 @@
       expect(field.subfields.first.code).to eq 'a'
       expect(field.subfields.first.value).to eq 'some data'
     end
+
+    context 'when wrong number of bytes received from Symphony' do
+      let(:headers) { { 'Content-Length': 268 } }
+      it 'raises RecordIncompleteError' do
+        expect { reader.to_marc }.to raise_error(SymphonyReader::RecordIncompleteError, 'Incomplete response received from Symphony for catkey - expected 268 bytes but got 394')
+      end
+    end
   end
 end

spec/requests/metadata_refresh_spec.rb

@@ -9,15 +9,34 @@
 
   before do
     allow(Dor).to receive(:find).and_return(object)
-    allow(RefreshMetadataAction).to receive(:run).and_return('<xml />')
     allow(object).to receive(:save)
   end
 
-  it 'updates the metadata and saves the changes' do
-    post '/v1/objects/druid:mk420bs7601/refresh_metadata',
-         headers: { 'X-Auth' => "Bearer #{jwt}" }
-    expect(response).to be_successful
-    expect(RefreshMetadataAction).to have_received(:run).with(object)
-    expect(object).to have_received(:save)
+  context 'when happy path' do
+    before do
+      allow(RefreshMetadataAction).to receive(:run).and_return('<xml />')
+    end
+
+    it 'updates the metadata and saves the changes' do
+      post '/v1/objects/druid:mk420bs7601/refresh_metadata',
+           headers: { 'X-Auth' => "Bearer #{jwt}" }
+      expect(response).to be_successful
+      expect(RefreshMetadataAction).to have_received(:run).with(object)
+      expect(object).to have_received(:save)
+    end
+  end
+
+  context 'when incomplete response from Symphony' do
+    before do
+      allow(object.identityMetadata).to receive(:otherId).and_return(['catkey:666'])
+      stub_request(:get, format(Settings.catalog.symphony.json_url, catkey: '666')).to_return(body: '{}', headers: { 'Content-Length': 0 })
+    end
+
+    it 'returns a 500 error' do
+      post '/v1/objects/druid:mk420bs7601/refresh_metadata',
+           headers: { 'X-Auth' => "Bearer #{jwt}" }
+      expect(response.status).to eq(500)
+      expect(response.body).to eq('Incomplete response received from Symphony for 666 - expected 0 bytes but got 2')
+    end
   end
 end