Fixed compression on WorkItemState#state

app/controllers/work_item_states_controller.rb

@@ -1,5 +1,4 @@
 class WorkItemStatesController < ApplicationController
-  require 'zlib'
   respond_to :html, :json
   before_filter :authenticate_user!
   before_filter :set_item_and_state, only: [:show, :update]
@@ -42,12 +41,10 @@ def init_from_params
   end
 
   def work_item_state_params
-    params[:work_item_state][:state] = Zlib::Deflate.deflate(params[:work_item_state][:state]) if params[:work_item_state][:state]
     params.require(:work_item_state).permit(:work_item_id, :action, :state)
   end
 
   def params_for_update
-    params[:work_item_state][:state] = Zlib::Deflate.deflate(params[:work_item_state][:state]) if params[:work_item_state][:state]
     params.require(:work_item_state).permit(:action, :state)
   end
 

app/models/work_item_state.rb

@@ -3,35 +3,56 @@ class WorkItemState < ActiveRecord::Base
 
   belongs_to :work_item
   validates :action, presence: true
-  before_save :set_action
+  before_save :set_action, :compress_state
 
   def to_param
     work_item_id
   end
 
   def serializable_hash (options={})
-    #unzipped_state = WorkItemState.inflate(self.state) unless self.state.nil?
-    unzipped_state = Zlib::Inflate.inflate(self.state) unless self.state.nil?
     {
         id: id,
         work_item_id: work_item_id,
         action: action,
-        state: unzipped_state
+        state: self.unzipped_state
     }
   end
 
-  def self.inflate(string)
-    zstream = Zlib::Inflate.new(-Zlib::MAX_WBITS)
-    buf = zstream.inflate(string)
-    zstream.finish
-    zstream.close
-    buf
+  def unzipped_state
+    if state.blank? || state_looks_like_plaintext
+      state
+    else
+      Zlib::Inflate.inflate(state)
+    end
   end
 
   private
 
+  # Returns true if state looks like plaintext.
+  # We only look at the first 50 chars, because the full
+  # string may be a few megabytes, and the gzip header
+  # in the first few bytes will contain binary data.
+  def state_looks_like_plaintext
+    if state.length <= 50
+      sample = state
+    else
+      last_index = [state.length, 50].min
+      sample = state.slice(0, last_index)
+    end
+    # Regex checks for unprintable characters.
+    # If no match, we assume plaintext.
+    !sample.match(/[^[:print:]]/)
+  end
+
   def set_action
     self.action = self.work_item.action if self.action.blank?
   end
 
+  # Compress the state field, if it's not already compressed
+  def compress_state
+    if !state.blank? && state_looks_like_plaintext
+      self.state = Zlib::Deflate.deflate(state)
+    end
+  end
+
 end

db/schema.rb

@@ -78,10 +78,10 @@
     t.integer  "generic_file_id"
     t.datetime "created_at",                     null: false
     t.datetime "updated_at",                     null: false
-    t.integer  "institution_id"
     t.string   "intellectual_object_identifier"
     t.string   "generic_file_identifier"
     t.string   "outcome"
+    t.integer  "institution_id"
   end
 
   add_index "premis_events", ["generic_file_id"], name: "index_premis_events_on_generic_file_id"

spec/controllers/work_item_states_controller_spec.rb

@@ -1,13 +1,12 @@
 require 'spec_helper'
-require 'zlib'
 
 RSpec.describe WorkItemStatesController, type: :controller do
   let!(:institution) { FactoryGirl.create(:institution) }
   let!(:admin_user) { FactoryGirl.create(:user, :admin) }
   let!(:object) { FactoryGirl.create(:intellectual_object, institution: institution, access: 'institution') }
   let!(:item) { FactoryGirl.create(:work_item, institution: institution, intellectual_object: object, object_identifier: object.identifier, action: Pharos::Application::PHAROS_ACTIONS['fixity'], status: Pharos::Application::PHAROS_STATUSES['success']) }
   let!(:other_item) { FactoryGirl.create(:work_item, institution: institution, intellectual_object: object, object_identifier: object.identifier) }
-  let!(:state_item) { FactoryGirl.create(:work_item_state, work_item: item, state: Zlib::Deflate.deflate('{JSON data}')) }
+  let!(:state_item) { FactoryGirl.create(:work_item_state, work_item: item, state: '{JSON data}') }
 
   describe 'POST #create' do
     before do
@@ -18,7 +17,9 @@
       post :create, work_item_state: { state: '{JSON data}', work_item_id: other_item.id, action: 'Success' }, format: :json
       expect(response).to be_success
       assigns(:state_item).action.should eq('Success')
-      assigns(:state_item).state.should eq('{JSON data}')
+      # State '{JSON data}' should be compressed
+      assigns(:state_item).state.bytes.should eq("x\x9C\xAB\xF6\n\xF6\xF7SHI,I\xAC\x05\x00\x16\x90\x03\xED".bytes)
+      assigns(:state_item).unzipped_state.should eq('{JSON data}')
       assigns(:state_item).work_item_id.should eq(other_item.id)
     end
   end
@@ -30,12 +31,13 @@
       end
 
       it 'responds successfully with both the action and the state updated' do
-        put :update, work_item_id: item.id, work_item_state: { action: 'Failed', state: '{NEW JSON data}' }, format: :json
+        put :update, work_item_id: item.id, work_item_state: { action: 'NewAction', state: '{NEW JSON data}' }, format: :json
         expect(response).to be_success
         assigns(:work_item).id.should eq(item.id)
         assigns(:state_item).id.should eq(state_item.id)
-        assigns(:state_item).action.should eq('Failed')
-        assigns(:state_item).state.should eq('{NEW JSON data}')
+        assigns(:state_item).action.should eq('NewAction')
+        assigns(:state_item).state.bytes.should eq("x\x9C\xAB\xF6s\rW\xF0\n\xF6\xF7SHI,I\xAC\x05\x00%\xB9\x04\xF7".bytes)
+        assigns(:state_item).unzipped_state.should eq('{NEW JSON data}')
       end
     end
   end
@@ -51,7 +53,8 @@
         expect(response).to be_success
         assigns(:work_item).id.should eq(item.id)
         assigns(:state_item).id.should eq(state_item.id)
-        assigns(:state_item).state.should eq('{JSON data}')
+        assigns(:state_item).state.bytes.should eq("x\x9C\xAB\xF6\n\xF6\xF7SHI,I\xAC\x05\x00\x16\x90\x03\xED".bytes)
+        assigns(:state_item).unzipped_state.should eq('{JSON data}')
       end
     end
   end