Delegate template parsing to Template.

Fixes for template validation (see the added spec, which failed before): with template '.zed' the noid '111' should be valid, yet the code did not validate it. Takes the work from #3 (thanks to @dbrower) and rebases it the latest commit with changes to match current styles.

Author: mjgiarlo

Gemfile

@@ -3,5 +3,8 @@ source 'https://rubygems.org'
 # Specify your gem's dependencies in test.gemspec
 gemspec
 
-gem 'simplecov'
-gem 'simplecov-rcov'
+group :development, :test do
+  gem 'simplecov'
+  gem 'simplecov-rcov'
+  gem 'byebug', require: false
+end

lib/noid.rb

@@ -5,4 +5,7 @@
 module Noid
   XDIGIT = %w(0 1 2 3 4 5 6 7 8 9 b c d f g h j k m n p q r s t v w x z)
   MAX_COUNTERS = 293
+
+  class TemplateError < StandardError
+  end
 end

lib/noid/minter.rb

@@ -34,20 +34,7 @@ def mint
     # @param [String] id
     # @return bool
     def valid?(id)
-      prefix = @template.prefix.empty? ? '' : id[0..@template.prefix.length - 1]
-      ch = @template.prefix.empty? ? id.split('') : id[@template.prefix.length..-1].split('')
-      check = ch.pop if @template.checkdigit?
-      return false unless prefix == @template.prefix
-
-      return false unless @template.characters.length == ch.length
-      @template.characters.split('').each_with_index do |c, i|
-        return false unless Noid::XDIGIT.include? ch[i]
-        return false if c == 'd' && ch[i] =~ /[^\d]/
-      end
-
-      return false unless check.nil? || check == @template.checkdigit(id[0..-2])
-
-      true
+      template.valid?(id)
     end
 
     ##

lib/noid/template.rb

@@ -1,10 +1,13 @@
 module Noid
   class Template
-    attr_reader :template
+    attr_reader :template, :prefix, :generator, :characters
+
+    VALID_PATTERN = /\A(.*)\.([rsz])([ed]+)(k?)\Z/
 
     # @param [String] template A Template is a coded string of the form Prefix.Mask that governs how identifiers will be minted.
     def initialize(template)
       @template = template
+      parse!
     end
 
     def mint(n)
@@ -15,93 +18,106 @@ def mint(n)
       str
     end
 
+    ##
+    # Is the string valid against this template string and checksum?
+    # @param [String] str
+    # @return bool
     def valid?(str)
-      return false unless str[0..prefix.length] == prefix
-
-      if generator == 'z'
-        str[prefix.length..-1].length > 2
-      else
-        str[prefix.length..-1].length == characters.length
-      end
-
-      characters.split('').each_with_index do |c, i|
-        case c
-        when 'e'
-          return false unless Noid::XDIGIT.include? str[prefix.length + i]
-        when 'd'
-          return false unless str[prefix.length + i] =~ /\d/
-        end
-      end
-
-      return false unless checkdigit(str[0..-2]) == str.split('').last if checkdigit?
-
+      match = validation_regex.match(str)
+      return false if match.nil?
+      return checkdigit(match[1]) == match[3] if checkdigit?
       true
     end
 
     ##
-    # identifier prefix string
-    def prefix
-      @prefix ||= @template.split('.').first
+    # calculate a checkdigit for the str
+    # @param [String] str
+    # @return [String] checkdigit
+    def checkdigit(str)
+      Noid::XDIGIT[str.split('').map { |x| Noid::XDIGIT.index(x).to_i }.each_with_index.map { |n, idx| n * (idx + 1) }.inject { |sum, n| sum + n } % Noid::XDIGIT.length]
     end
 
     ##
-    # identifier mask string
-    def mask
-      @mask ||= @template.split('.').last
+    # minimum sequence value
+    def min
+      @min ||= 0
     end
 
     ##
-    # generator type to use: r, s, z
-    def generator
-      @generator ||= mask[0..0]
+    # maximum sequence value for the template
+    def max
+      @max ||= if generator == 'z'
+                 nil
+               else
+                 size_list.inject(1) { |total, x| total * x }
+               end
     end
 
+    protected
+
     ##
-    # sequence pattern: e (extended), d (digit)
-    def characters
-      @characters ||= begin
-                        if checkdigit?
-                          mask[1..-2]
-                        else
-                          mask[1..-1]
-                        end
-                      end
+    # A noid has the structure (prefix)(code)(checkdigit)
+    # the regexp has the following captures
+    #  1 - the prefix and the code
+    #  2 - the changing id characters (not the prefix and not the checkdigit)
+    #  3 - the checkdigit, if there is one. This field is missing if there is no checkdigit
+    def validation_regex
+      @validation_regex ||= begin
+                              character_pattern = ''
+                              # the first character in the mask after the type character is the most significant
+                              # acc. to the Noid spec (p.9):
+                              # https://wiki.ucop.edu/display/Curation/NOID?preview=/16744482/16973835/noid.pdf
+                              character_pattern += character_to_pattern(character_list.first) + "*" if generator == 'z'
+                              character_pattern += character_list.map { |c| character_to_pattern(c) }.join
+
+                              %r{\A(#{Regexp.escape(prefix)}(#{character_pattern}))(#{character_to_pattern('k') if checkdigit?})\Z}
+                            end
     end
 
     ##
-    # should generated identifiers have a checkdigit?
-    def checkdigit?
-      mask.split('').last == 'k'
+    # parse template and put the results into instance variables
+    # raise an exception if there is a parse error
+    def parse!
+      match = VALID_PATTERN.match(template)
+      raise Noid::TemplateError, "Malformed noid template '#{template}'" unless match
+      @prefix = match[1]
+      @generator = match[2]
+      @characters = match[3]
+      @checkdigit = (match[4] == 'k')
     end
 
-    ##
-    # calculate a checkdigit for the str
-    # @param [String] str
-    # @return [String] checkdigit
-    def checkdigit(str)
-      Noid::XDIGIT[str.split('').map { |x| Noid::XDIGIT.index(x).to_i }.each_with_index.map { |n, idx| n * (idx + 1) }.inject { |sum, n| sum + n } % Noid::XDIGIT.length]
+    def xdigit_pattern
+      @xdigit_pattern ||= '[' + Noid::XDIGIT.join('') + ']'
     end
 
-    ##
-    # minimum sequence value
-    def min
-      @min ||= 0
+    def character_to_pattern(c)
+      case c
+      when 'e', 'k'
+        xdigit_pattern
+      when 'd'
+        '\d'
+      else
+        ''
+      end
     end
 
     ##
-    # maximum sequence value for the template
-    def max
-      @max ||= begin
-        case generator
-        when 'z'
-          nil
-        else
-          characters.split('').map { |x| character_space(x) }.compact.inject(1) { |total, x| total * x }
-        end
-      end
+    # Return a list giving the number of possible characters at each position
+    def size_list
+      @size_list ||= character_list.map { |c| character_space(c) }
     end
 
-    protected
+    def character_list
+      characters.split('')
+    end
+
+    def mask
+      generator + characters
+    end
+
+    def checkdigit?
+      @checkdigit
+    end
 
     ##
     # total size of a given template character value
@@ -120,17 +136,17 @@ def character_space(c)
     # @param [Integer] n
     # @return [String]
     def n2xdig(n)
-      xdig = characters.reverse.split('').map do |c|
-        value = n % character_space(c)
-        n /= character_space(c)
+      xdig = size_list.reverse.map { |size|
+        value = n % size
+        n /= size
         Noid::XDIGIT[value]
-      end.compact.join('')
+      }.compact.join('')
 
       if generator == 'z'
-        c = characters.split('').last
+        size = size_list.last
         while n > 0
-          value = n % character_space(c)
-          n /= character_space(c)
+          value = n % size
+          n /= size
           xdig += Noid::XDIGIT[value]
         end
       end

spec/lib/minter_spec.rb

@@ -112,6 +112,12 @@
       minter2 = described_class.new(template: '.redek')
       expect(minter2.valid?(id)).to eq(true)
     end
+    it 'validates an unlimited sequence with mixed digits' do
+      minter = described_class.new(template: '.zed')
+      1000.times { minter.mint }
+      id = minter.mint
+      expect(minter.valid?(id)).to eq(true)
+    end
   end
 
   describe 'seed' do

spec/lib/template_spec.rb

@@ -0,0 +1,16 @@
+require 'spec_helper'
+
+describe Noid::Template do
+  context 'with a valid template' do
+    let(:template) { '.redek' }
+    it 'initializes without raising' do
+      expect { described_class.new(template) }.not_to raise_error
+    end
+  end
+  context 'with a bogus template' do
+    let(:template) { 'foobar' }
+    it 'raises Noid::TemplateError' do
+      expect { described_class.new(template) }.to raise_error(Noid::TemplateError)
+    end
+  end
+end