LIBSYS-8144: Support Code 39 barcode format

Gemfile.lock

@@ -81,19 +81,19 @@ GEM
       securerandom (>= 0.3)
       tzinfo (~> 2.0, >= 2.0.5)
       uri (>= 0.13.1)
-    airbrussh (1.5.3)
+    airbrussh (1.6.1)
       sshkit (>= 1.6.1, != 1.7.0)
     ast (2.4.3)
     base64 (0.3.0)
-    bcrypt (3.1.20)
+    bcrypt (3.1.22)
     bcrypt_pbkdf (1.1.2)
     bcrypt_pbkdf (1.1.2-arm64-darwin)
     bcrypt_pbkdf (1.1.2-x86_64-darwin)
     benchmark (0.5.0)
-    bigdecimal (3.3.1)
-    bootsnap (1.19.0)
+    bigdecimal (4.0.1)
+    bootsnap (1.23.0)
       msgpack (~> 1.2)
-    brakeman (8.0.2)
+    brakeman (8.0.4)
       racc
     builder (3.3.0)
     capistrano (3.19.2)
@@ -120,7 +120,7 @@ GEM
     connection_pool (3.0.2)
     crass (1.0.6)
     date (3.5.1)
-    debug (1.11.0)
+    debug (1.11.1)
       irb (~> 1.10)
       reline (>= 0.3.8)
     devise (4.9.4)
@@ -134,7 +134,7 @@ GEM
     dotenv (3.2.0)
     drb (2.2.3)
     ed25519 (1.4.0)
-    erb (6.0.0)
+    erb (6.0.2)
     erubi (1.13.1)
     faraday (2.14.1)
       faraday-net_http (>= 2.0, < 3.5)
@@ -148,16 +148,18 @@ GEM
       zeitwerk (~> 2.7)
     globalid (1.3.0)
       activesupport (>= 6.1)
-    hashie (5.0.0)
-    i18n (1.14.7)
+    hashie (5.1.0)
+      logger
+    i18n (1.14.8)
       concurrent-ruby (~> 1.0)
-    io-console (0.8.1)
-    irb (1.15.3)
+    io-console (0.8.2)
+    irb (1.17.0)
       pp (>= 0.6.0)
+      prism (>= 1.3.0)
       rdoc (>= 4.0.0)
       reline (>= 0.4.2)
     json (2.19.3)
-    kamal (2.9.0)
+    kamal (2.11.0)
       activesupport (>= 7.0)
       base64 (~> 0.2)
       bcrypt_pbkdf (~> 1.0)
@@ -170,7 +172,7 @@ GEM
       zeitwerk (>= 2.6.18, < 3.0)
     language_server-protocol (3.17.0.5)
     logger (1.7.0)
-    loofah (2.24.1)
+    loofah (2.25.1)
       crass (~> 1.0.2)
       nokogiri (>= 1.12.0)
     mail (2.9.0)
@@ -185,7 +187,9 @@ GEM
     marcel (1.1.0)
     mini_mime (1.1.5)
     mini_portile2 (2.8.9)
-    minitest (5.27.0)
+    minitest (6.0.2)
+      drb (~> 2.0)
+      prism (~> 1.5)
     mono_logger (1.1.2)
     msgpack (1.8.0)
     multi_json (1.17.0)
@@ -208,7 +212,7 @@ GEM
       net-ssh (>= 5.0.0, < 8.0.0)
     net-smtp (0.5.1)
       net-protocol
-    net-ssh (7.3.0)
+    net-ssh (7.3.2)
     nio4r (2.7.5)
     nokogiri (1.19.2)
       mini_portile2 (~> 2.8.2)
@@ -230,14 +234,14 @@ GEM
     pp (0.6.3)
       prettyprint
     prettyprint (0.2.0)
-    prism (1.4.0)
-    psych (5.3.0)
+    prism (1.9.0)
+    psych (5.3.1)
       date
       stringio
     puma (7.1.0)
       nio4r (~> 2.0)
     racc (1.8.1)
-    rack (3.2.4)
+    rack (3.2.5)
     rack-protection (4.2.1)
       base64 (>= 0.1.0)
       logger (>= 1.6.0)
@@ -267,8 +271,8 @@ GEM
       activesupport (>= 5.0.0)
       minitest
       nokogiri (>= 1.6)
-    rails-html-sanitizer (1.6.2)
-      loofah (~> 2.21)
+    rails-html-sanitizer (1.7.0)
+      loofah (~> 2.25)
       nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0)
     railties (8.0.4)
       actionpack (= 8.0.4)
@@ -281,7 +285,7 @@ GEM
       zeitwerk (~> 2.6)
     rainbow (3.1.1)
     rake (13.3.1)
-    rdoc (6.17.0)
+    rdoc (7.2.0)
       erb
       psych (>= 4.0.0)
       tsort
@@ -378,28 +382,28 @@ GEM
       rack-protection (= 4.2.1)
       rack-session (>= 2.0.0, < 3)
       tilt (~> 2.0)
-    sqlite3 (2.8.1-aarch64-linux-gnu)
-    sqlite3 (2.8.1-aarch64-linux-musl)
-    sqlite3 (2.8.1-arm-linux-gnu)
-    sqlite3 (2.8.1-arm-linux-musl)
-    sqlite3 (2.8.1-arm64-darwin)
-    sqlite3 (2.8.1-x86_64-darwin)
-    sqlite3 (2.8.1-x86_64-linux-gnu)
-    sqlite3 (2.8.1-x86_64-linux-musl)
-    sshkit (1.24.0)
+    sqlite3 (2.9.2-aarch64-linux-gnu)
+    sqlite3 (2.9.2-aarch64-linux-musl)
+    sqlite3 (2.9.2-arm-linux-gnu)
+    sqlite3 (2.9.2-arm-linux-musl)
+    sqlite3 (2.9.2-arm64-darwin)
+    sqlite3 (2.9.2-x86_64-darwin)
+    sqlite3 (2.9.2-x86_64-linux-gnu)
+    sqlite3 (2.9.2-x86_64-linux-musl)
+    sshkit (1.25.0)
       base64
       logger
       net-scp (>= 1.1.2)
       net-sftp (>= 2.1.2)
       net-ssh (>= 2.8.0)
       ostruct
-    stringio (3.1.9)
-    thor (1.4.0)
-    thruster (0.1.16)
-    thruster (0.1.16-aarch64-linux)
-    thruster (0.1.16-arm64-darwin)
-    thruster (0.1.16-x86_64-darwin)
-    thruster (0.1.16-x86_64-linux)
+    stringio (3.2.0)
+    thor (1.5.0)
+    thruster (0.1.20)
+    thruster (0.1.20-aarch64-linux)
+    thruster (0.1.20-arm64-darwin)
+    thruster (0.1.20-x86_64-darwin)
+    thruster (0.1.20-x86_64-linux)
     tilt (2.6.1)
     timeout (0.5.0)
     tsort (0.2.0)

app/controllers/barcode_controller.rb

@@ -2,14 +2,24 @@
 
 class BarcodeController < ApplicationController
   before_action :authenticate, only: [:update]
+  before_action :validate_barcode, only: [:query, :update]
+
   skip_before_action :verify_authenticity_token, only: [:update] # No need for CSRF token for API token auth endpoint
 
   rescue_from Faraday::Error, with: :handle_faraday_error
   rescue_from Bibdata::Exceptions::UnresolvableHoldingsPermanentLocationError,
               with: :unresolvable_holdings_location_error
 
+  # See https://en.wikipedia.org/wiki/Code_39 for more information about allowed barcode characters.
+  # Note: Even though Code 39 supports the characters "/" and " ", we expect those characters to be supplied in the URL
+  # as HTML entities.  "/" becomes "%2F".  " " becomes "%20".
+  # We only expect a possible special character to be at the end of the string. In some cases like "-" or "." the final
+  # character can be expressed as the original character values, but some characters must be expressed as html entitites
+  # (like "%20") for them to be considered a valid URL.
+  VALID_BARCODE_REGEX = /[a-zA-Z0-9]+([-.$+]|%[a-zA-Z0-9]{2})?/
+
   def query
-    barcode = params[:barcode] # Example: 'CU23392169'
+    barcode = query_or_update_params[:barcode] # Example: 'CU23392169'
     marc_record = nil
     duration = Benchmark.measure do
       marc_record = Bibdata::Scsb.merged_marc_record_for_barcode(barcode, flip_location: false)
@@ -24,7 +34,7 @@ def query
   end
 
   def update
-    barcode = params[:barcode] # Example: 'CU23392169'
+    barcode = query_or_update_params[:barcode] # Example: 'CU23392169'
     marc_record = nil
     duration = Benchmark.measure do
       marc_record = Bibdata::Scsb.merged_marc_record_for_barcode(barcode, flip_location: true)
@@ -40,12 +50,20 @@ def update
 
   private
 
+  def query_or_update_params
+    params.permit(:barcode)
+  end
+
   def authenticate
     authenticate_or_request_with_http_token do |token, _options|
       ActiveSupport::SecurityUtils.secure_compare(token, Rails.application.config.bibdata['barcode_update_api_token'])
     end
   end
 
+  def validate_barcode
+    render_not_found unless VALID_BARCODE_REGEX.match?(query_or_update_params[:barcode])
+  end
+
   def render_not_found(barcode)
     render plain: "Barcode #{barcode} was not found.", status: :not_found
   end

config/routes.rb

@@ -2,15 +2,17 @@
 
 require 'resque/server'
 
+BARCODE_CONSTRAINT = %r{[^/]+}
+
 Rails.application.routes.draw do
   devise_for :users, controllers: { sessions: 'users/sessions', omniauth_callbacks: 'users/omniauth_callbacks' }
   devise_scope :user do
     get '/users/sign_in', to: 'users/sessions#new'
     delete '/users/sign_out', to: 'users/sessions#destroy'
   end
 
-  get 'barcode/:barcode/query' => 'barcode#query'
-  post 'barcode/:barcode/update' => 'barcode#update'
+  get 'barcode/:barcode/query' => 'barcode#query', constraints: { barcode: BARCODE_CONSTRAINT }
+  post 'barcode/:barcode/update' => 'barcode#update', constraints: { barcode: BARCODE_CONSTRAINT }
   # Define your application routes per the DSL in https://guides.rubyonrails.org/routing.html
 
   # Reveal health status on /up that returns 200 if the app boots with no exceptions, otherwise 500.

spec/requests/barcode_spec.rb

@@ -2,7 +2,8 @@
 
 RSpec.describe "Barcodes", type: :request do
   let(:valid_barcode) { "CU12345678" }
-  let(:invalid_barcode) { "not-valid" }
+  let(:invalid_format_barcode) { "!not-valid!" }
+  let(:valid_format_but_unresolvable_barcode) { "0000000000" }
   let(:marc_record) { double(MARC::Record) }
   let(:xml_response) do
     %{
@@ -18,7 +19,7 @@
   end
 
   before do
-    # merged_marc_record_for_barcode will return nil by default
+    # merged_marc_record_for_barcode will return nil by default (as if the barcode cannot be resolved to an existing record)
     allow(Bibdata::Scsb).to receive(:merged_marc_record_for_barcode).and_return(nil)
     # merged_marc_record_for_barcode will return a marc record for a valid barcode
     allow(Bibdata::Scsb).to receive(:merged_marc_record_for_barcode).with(valid_barcode, flip_location: flip_location).and_return(marc_record)
@@ -38,10 +39,16 @@
       expect(response.body).to eq(xml_response)
     end
 
-    it "returns a 404 Not Found status, plus informative message, for an invalid barcode" do
-      get "/barcode/#{invalid_barcode}/query"
+    it "returns a 404 Not Found status, plus informative message, for an invalid format barcode" do
+      get "/barcode/#{invalid_format_barcode}/query"
       expect(response).to have_http_status(:not_found)
-      expect(response.body).to eq("Barcode #{invalid_barcode} was not found.")
+      expect(response.body).to eq("Barcode #{invalid_format_barcode} was not found.")
+    end
+
+    it "returns a 404 Not Found status, plus informative message, for a valid format barcode that cannot be resolved to a record" do
+      get "/barcode/#{valid_format_but_unresolvable_barcode}/query"
+      expect(response).to have_http_status(:not_found)
+      expect(response.body).to eq("Barcode #{valid_format_but_unresolvable_barcode} was not found.")
     end
 
     [Faraday::Error, Faraday::UnprocessableEntityError, Faraday::UnauthorizedError].each do |faraday_error_class|
@@ -99,10 +106,16 @@
         expect(response.body).to eq(xml_response)
       end
 
-      it "returns a 404 Not Found status, plus informative message, for an invalid barcode" do
-        post "/barcode/#{invalid_barcode}/update", params: {}, headers: headers_with_valid_authorization_token
+      it "returns a 404 Not Found status, plus informative message, for an invalid format barcode" do
+        post "/barcode/#{invalid_format_barcode}/update", params: {}, headers: headers_with_valid_authorization_token
+        expect(response).to have_http_status(:not_found)
+        expect(response.body).to eq("Barcode #{invalid_format_barcode} was not found.")
+      end
+
+      it "returns a 404 Not Found status, plus informative message, for a valid format barcode that cannot be resolved to a record" do
+        post "/barcode/#{valid_format_but_unresolvable_barcode}/update", params: {}, headers: headers_with_valid_authorization_token
         expect(response).to have_http_status(:not_found)
-        expect(response.body).to eq("Barcode #{invalid_barcode} was not found.")
+        expect(response.body).to eq("Barcode #{valid_format_but_unresolvable_barcode} was not found.")
       end
 
       [Faraday::Error, Faraday::UnprocessableEntityError, Faraday::UnauthorizedError].each do |faraday_error_class|

spec/routing/barcode_spec.rb

@@ -0,0 +1,29 @@
+require 'rails_helper'
+
+RSpec.describe "/barcode routes", type: :routing do
+  describe "/barcode/:barcode/query" do
+    it "routes as expected for an alphanumeric barcode" do
+      expect(:get => "/barcode/AR03426424/query").to route_to(
+        controller: "barcode",
+        action: "query",
+        barcode: "AR03426424"
+      )
+    end
+
+    it "routes as expected for a Code 39 barcode that ends with a non-alphanumeric character" do
+      expect(:get => "/barcode/3500500741637+/query").to route_to(
+        controller: "barcode",
+        action: "query",
+        barcode: "3500500741637+"
+      )
+    end
+
+    it "routes as expected for a Code 39 barcode that ends with a url-encoded non-alphanumeric character" do
+      expect(:get => "/barcode/3500500741637%20/query").to route_to(
+        controller: "barcode",
+        action: "query",
+        barcode: "3500500741637 "
+      )
+    end
+  end
+end