Disallow full identifiers from ending with a slash

lib/nanoc/base/entities/document.rb

@@ -15,7 +15,7 @@ def attributes
       end
 
       # @return [Nanoc::Identifier]
-      attr_accessor :identifier
+      attr_reader :identifier
 
       # @return [String, nil]
       attr_accessor :checksum_data
@@ -81,6 +81,11 @@ def reference
         raise NotImplementedError
       end
 
+      contract C::Or[Nanoc::Identifier, String] => Nanoc::Identifier
+      def identifier=(new_identifier)
+        @identifier = Nanoc::Identifier.from(new_identifier)
+      end
+
       contract C::None => String
       def inspect
         "<#{self.class} identifier=\"#{identifier}\">"

lib/nanoc/base/entities/identifier.rb

@@ -12,6 +12,13 @@ def initialize(string)
       end
     end
 
+    # @api private
+    class InvalidFullIdentifierError < ::Nanoc::Error
+      def initialize(string)
+        super("Invalid full identifier (ends with a slash): #{string.inspect}")
+      end
+    end
+
     # @api private
     class InvalidTypeError < ::Nanoc::Error
       def initialize(type)
@@ -60,9 +67,9 @@ def initialize(string, type: :full)
       when :legacy
         @string = "/#{string}/".gsub(/^\/+|\/+$/, '/').freeze
       when :full
-        if string !~ /\A\//
-          raise InvalidIdentifierError.new(string)
-        end
+        raise InvalidIdentifierError.new(string) if string !~ /\A\//
+        raise InvalidFullIdentifierError.new(string) if string =~ /\/\z/
+
         @string = string.dup.freeze
       else
         raise InvalidTypeError.new(@type)

spec/nanoc/base/entities/document_spec.rb

@@ -225,4 +225,34 @@
       expect(subject.attributes).to eq(at: 'ribut')
     end
   end
+
+  describe '#identifier=' do
+    let(:document) { described_class.new('stuff', {}, '/foo.md') }
+
+    it 'allows changing to a string that contains a full identifier' do
+      expect { document.identifier = '/thing' }.not_to raise_error
+
+      expect(document.identifier).to eq('/thing')
+      expect(document.identifier).to be_full
+    end
+
+    it 'refuses changing to a string that does not contain a full identifier' do
+      expect { document.identifier = '/thing/' }
+        .to raise_error(Nanoc::Identifier::InvalidFullIdentifierError)
+    end
+
+    it 'allos changing to a full identifier' do
+      document.identifier = Nanoc::Identifier.new('/thing')
+
+      expect(document.identifier.to_s).to eq('/thing')
+      expect(document.identifier).to be_full
+    end
+
+    it 'allos changing to a legacy identifier' do
+      document.identifier = Nanoc::Identifier.new('/thing/', type: :legacy)
+
+      expect(document.identifier).to eq('/thing/')
+      expect(document.identifier).to be_legacy
+    end
+  end
 end

spec/nanoc/base/entities/identifier_spec.rb

@@ -67,6 +67,16 @@
           .to raise_error(Nanoc::Identifier::InvalidIdentifierError)
       end
 
+      it 'refuses string ending with a slash' do
+        expect { described_class.new('/foo/') }
+          .to raise_error(Nanoc::Identifier::InvalidFullIdentifierError)
+      end
+
+      it 'refuses string with only slash' do
+        expect { described_class.new('/') }
+          .to raise_error(Nanoc::Identifier::InvalidFullIdentifierError)
+      end
+
       it 'has proper string representation' do
         expect(described_class.new('/foo').to_s).to eql('/foo')
       end
@@ -422,7 +432,7 @@
     end
 
     context 'full type' do
-      let(:identifier) { described_class.new('/foo/', type: :full) }
+      let(:identifier) { described_class.new('/foo', type: :full) }
       it { is_expected.to eql(false) }
     end
   end
@@ -436,7 +446,7 @@
     end
 
     context 'full type' do
-      let(:identifier) { described_class.new('/foo/', type: :full) }
+      let(:identifier) { described_class.new('/foo', type: :full) }
       it { is_expected.to eql(true) }
     end
   end
@@ -445,7 +455,7 @@
     subject { identifier.components }
 
     context 'no components' do
-      let(:identifier) { described_class.new('/') }
+      let(:identifier) { described_class.new('/', type: :legacy) }
       it { is_expected.to eql([]) }
     end
 

spec/nanoc/base/item_rep_writer_spec.rb

@@ -4,7 +4,7 @@
   describe '#write' do
     let(:raw_path) { 'output/blah.dat' }
 
-    let(:item) { Nanoc::Int::Item.new(orig_content, {}, '/') }
+    let(:item) { Nanoc::Int::Item.new(orig_content, {}, '/foo') }
 
     let(:item_rep) do
       Nanoc::Int::ItemRep.new(item, :default).tap do |ir|

spec/nanoc/base/services/executor_spec.rb

@@ -653,7 +653,7 @@ def run(_content, params = {})
       let(:arg) { '/default' }
 
       let(:layouts) do
-        [Nanoc::Int::Layout.new('head <%= @foo %> foot', {}, '/default/')]
+        [Nanoc::Int::Layout.new('head <%= @foo %> foot', {}, Nanoc::Identifier.new('/default/', type: :legacy))]
       end
 
       it { is_expected.to eq(layouts[0]) }

spec/nanoc/base/views/document_view_spec.rb

@@ -27,7 +27,7 @@
   end
 
   describe '#frozen?' do
-    let(:document) { entity_class.new('content', {}, '/asdf/') }
+    let(:document) { entity_class.new('content', {}, '/asdf') }
 
     subject { view.frozen? }
 
@@ -42,10 +42,10 @@
   end
 
   describe '#== and #eql?' do
-    let(:document) { entity_class.new('content', {}, '/asdf/') }
+    let(:document) { entity_class.new('content', {}, '/asdf') }
 
     context 'comparing with document with same identifier' do
-      let(:other) { entity_class.new('content', {}, '/asdf/') }
+      let(:other) { entity_class.new('content', {}, '/asdf') }
 
       it 'is ==' do
         expect(view).to eq(other)
@@ -57,7 +57,7 @@
     end
 
     context 'comparing with document with different identifier' do
-      let(:other) { entity_class.new('content', {}, '/fdsa/') }
+      let(:other) { entity_class.new('content', {}, '/fdsa') }
 
       it 'is not ==' do
         expect(view).not_to eq(other)
@@ -69,7 +69,7 @@
     end
 
     context 'comparing with document view with same identifier' do
-      let(:other) { other_view_class.new(entity_class.new('content', {}, '/asdf/'), nil) }
+      let(:other) { other_view_class.new(entity_class.new('content', {}, '/asdf'), nil) }
 
       it 'is ==' do
         expect(view).to eq(other)
@@ -81,7 +81,7 @@
     end
 
     context 'comparing with document view with different identifier' do
-      let(:other) { other_view_class.new(entity_class.new('content', {}, '/fdsa/'), nil) }
+      let(:other) { other_view_class.new(entity_class.new('content', {}, '/fdsa'), nil) }
 
       it 'is not ==' do
         expect(view).not_to eq(other)
@@ -106,7 +106,7 @@
   end
 
   describe '#[]' do
-    let(:document) { entity_class.new('stuff', { animal: 'donkey' }, '/foo/') }
+    let(:document) { entity_class.new('stuff', { animal: 'donkey' }, '/foo') }
 
     subject { view[key] }
 
@@ -154,7 +154,7 @@
   end
 
   describe '#attributes' do
-    let(:document) { entity_class.new('stuff', { animal: 'donkey' }, '/foo/') }
+    let(:document) { entity_class.new('stuff', { animal: 'donkey' }, '/foo') }
 
     subject { view.attributes }
 
@@ -179,7 +179,7 @@
   end
 
   describe '#fetch' do
-    let(:document) { entity_class.new('stuff', { animal: 'donkey' }, '/foo/') }
+    let(:document) { entity_class.new('stuff', { animal: 'donkey' }, '/foo') }
 
     context 'with existant key' do
       let(:key) { :animal }
@@ -260,7 +260,7 @@
   end
 
   describe '#key?' do
-    let(:document) { entity_class.new('stuff', { animal: 'donkey' }, '/foo/') }
+    let(:document) { entity_class.new('stuff', { animal: 'donkey' }, '/foo') }
 
     subject { view.key?(key) }
 
@@ -308,15 +308,15 @@
   end
 
   describe '#hash' do
-    let(:document) { double(:document, identifier: '/foo/') }
+    let(:document) { double(:document, identifier: '/foo') }
 
     subject { view.hash }
 
-    it { should == described_class.hash ^ '/foo/'.hash }
+    it { should == described_class.hash ^ '/foo'.hash }
   end
 
   describe '#raw_content' do
-    let(:document) { entity_class.new('stuff', { animal: 'donkey' }, '/foo/') }
+    let(:document) { entity_class.new('stuff', { animal: 'donkey' }, '/foo') }
 
     subject { view.raw_content }
 

spec/nanoc/base/views/item_rep_view_spec.rb

@@ -31,7 +31,7 @@
 
   describe '#frozen?' do
     let(:item_rep) { Nanoc::Int::ItemRep.new(item, :jacques) }
-    let(:item) { Nanoc::Int::Item.new('asdf', {}, '/foo/') }
+    let(:item) { Nanoc::Int::Item.new('asdf', {}, '/foo') }
     let(:view) { described_class.new(item_rep, view_context) }
 
     subject { view.frozen? }
@@ -48,11 +48,11 @@
 
   describe '#== and #eql?' do
     let(:item_rep) { Nanoc::Int::ItemRep.new(item, :jacques) }
-    let(:item) { Nanoc::Int::Item.new('asdf', {}, '/foo/') }
+    let(:item) { Nanoc::Int::Item.new('asdf', {}, '/foo') }
     let(:view) { described_class.new(item_rep, view_context) }
 
     context 'comparing with item rep with same identifier' do
-      let(:other_item) { double(:other_item, identifier: '/foo/') }
+      let(:other_item) { double(:other_item, identifier: '/foo') }
       let(:other) { double(:other_item_rep, item: other_item, name: :jacques) }
 
       it 'is ==' do
@@ -65,7 +65,7 @@
     end
 
     context 'comparing with item rep with different identifier' do
-      let(:other_item) { double(:other_item, identifier: '/bar/') }
+      let(:other_item) { double(:other_item, identifier: '/bar') }
       let(:other) { double(:other_item_rep, item: other_item, name: :jacques) }
 
       it 'is not ==' do
@@ -78,7 +78,7 @@
     end
 
     context 'comparing with item rep with different name' do
-      let(:other_item) { double(:other_item, identifier: '/foo/') }
+      let(:other_item) { double(:other_item, identifier: '/foo') }
       let(:other) { double(:other_item_rep, item: other_item, name: :marvin) }
 
       it 'is not ==' do
@@ -91,7 +91,7 @@
     end
 
     context 'comparing with item rep with same identifier' do
-      let(:other_item) { double(:other_item, identifier: '/foo/') }
+      let(:other_item) { double(:other_item, identifier: '/foo') }
       let(:other) { described_class.new(double(:other_item_rep, item: other_item, name: :jacques), view_context) }
 
       it 'is ==' do
@@ -104,7 +104,7 @@
     end
 
     context 'comparing with item rep with different identifier' do
-      let(:other_item) { double(:other_item, identifier: '/bar/') }
+      let(:other_item) { double(:other_item, identifier: '/bar') }
       let(:other) { described_class.new(double(:other_item_rep, item: other_item, name: :jacques), view_context) }
 
       it 'is not equal' do
@@ -114,7 +114,7 @@
     end
 
     context 'comparing with item rep with different name' do
-      let(:other_item) { double(:other_item, identifier: '/foo/') }
+      let(:other_item) { double(:other_item, identifier: '/foo') }
       let(:other) { described_class.new(double(:other_item_rep, item: other_item, name: :marvin), view_context) }
 
       it 'is not equal' do
@@ -124,7 +124,7 @@
     end
 
     context 'comparing with something that is not an item rep' do
-      let(:other_item) { double(:other_item, identifier: '/foo/') }
+      let(:other_item) { double(:other_item, identifier: '/foo') }
       let(:other) { :donkey }
 
       it 'is not equal' do
@@ -136,12 +136,12 @@
 
   describe '#hash' do
     let(:item_rep) { Nanoc::Int::ItemRep.new(item, :jacques) }
-    let(:item) { Nanoc::Int::Item.new('asdf', {}, '/foo/') }
+    let(:item) { Nanoc::Int::Item.new('asdf', {}, '/foo') }
     let(:view) { described_class.new(item_rep, view_context) }
 
     subject { view.hash }
 
-    it { should == described_class.hash ^ Nanoc::Identifier.new('/foo/').hash ^ :jacques.hash }
+    it { should == described_class.hash ^ Nanoc::Identifier.new('/foo').hash ^ :jacques.hash }
   end
 
   describe '#snapshot?' do
@@ -349,7 +349,7 @@
 
   describe '#binary?' do
     let(:item_rep) { Nanoc::Int::ItemRep.new(item, :jacques) }
-    let(:item) { Nanoc::Int::Item.new('asdf', {}, '/foo/') }
+    let(:item) { Nanoc::Int::Item.new('asdf', {}, '/foo') }
     let(:view) { described_class.new(item_rep, view_context) }
 
     subject { view.binary? }
@@ -383,7 +383,7 @@
 
   describe '#item' do
     let(:item_rep) { Nanoc::Int::ItemRep.new(item, :jacques) }
-    let(:item) { Nanoc::Int::Item.new('asdf', {}, '/foo/') }
+    let(:item) { Nanoc::Int::Item.new('asdf', {}, '/foo') }
     let(:view) { described_class.new(item_rep, view_context) }
 
     subject { view.item }
@@ -399,11 +399,11 @@
 
   describe '#inspect' do
     let(:item_rep) { Nanoc::Int::ItemRep.new(item, :jacques) }
-    let(:item) { Nanoc::Int::Item.new('asdf', {}, '/foo/') }
+    let(:item) { Nanoc::Int::Item.new('asdf', {}, '/foo') }
     let(:view) { described_class.new(item_rep, view_context) }
 
     subject { view.inspect }
 
-    it { is_expected.to eql('<Nanoc::ItemRepView item.identifier=/foo/ name=jacques>') }
+    it { is_expected.to eql('<Nanoc::ItemRepView item.identifier=/foo name=jacques>') }
   end
 end