Permalink
Browse files

Changes to support simple naming scheme

Fix specs
  • Loading branch information...
1 parent 33e0504 commit 30e717b29f3f194ce5b0ed636da522b2a3096811 @kjvarga committed Oct 11, 2012
@@ -2,3 +2,7 @@
require 'sitemap_generator/builder/sitemap_index_file'
require 'sitemap_generator/builder/sitemap_url'
require 'sitemap_generator/builder/sitemap_index_url'
+
+module SitemapGenerator::Builder
+ LinkHolder = Struct.new(:link, :options)
+end
@@ -42,10 +42,19 @@ def initialize(opts={})
@xml_wrapper_start.gsub!(/\s+/, ' ').gsub!(/ *> */, '>').strip!
@xml_wrapper_end = %q[</urlset>]
@filesize = bytesize(@xml_wrapper_start) + bytesize(@xml_wrapper_end)
+ @written = false
+ @reserved_name = nil # holds the name reserved from the namer
+ @frozen = false # rather than actually freeze, use this boolean
end
+ # If a name has been reserved, use the last modified time from the file.
+ # Otherwise return nil. We don't want to prematurely assign a name
+ # for this sitemap if one has not yet been reserved, because we may
+ # mess up the name-assignment sequence.
def lastmod
- File.mtime(location.path) rescue nil
+ File.mtime(location.path) if location.reserved_name?
+ rescue
+ nil
end
def empty?
@@ -102,31 +111,54 @@ def add(link, options={})
@link_count += 1
end
- # Write out the Sitemap file and freeze this object.
- #
- # All the xml content in the instance is cleared, but attributes like
- # <tt>filesize</tt> are still available.
+ # "Freeze" this object. Actually just flags it as frozen.
#
# A SitemapGenerator::SitemapFinalizedError exception is raised if the Sitemap
- # has already been finalized
+ # has already been finalized.
def finalize!
raise SitemapGenerator::SitemapFinalizedError if finalized?
+ @frozen = true
+ end
+ def finalized?
+ @frozen
+ end
+
+ # Write out the sitemap and free up memory.
+ #
+ # All the xml content in the instance is cleared, but attributes like
+ # <tt>filesize</tt> are still available.
+ #
+ # A SitemapGenerator::SitemapError exception is raised if the file has
+ # already been written.
+ def write
+ raise SitemapGenerator::SitemapError.new("Sitemap already written!") if written?
+ finalize! unless finalized?
+ reserve_name
@location.write(@xml_wrapper_start + @xml_content + @xml_wrapper_end)
+ @xml_content = @xml_wrapper_start = @xml_wrapper_end = ''
+ puts summary if @location.verbose?
+ @written = true
+ end
- # Increment the namer (SitemapFile only)
- @location.namer.next if @location.namer
+ # Return true if this file has been written out to disk
+ def written?
+ @written
+ end
- # Cleanup and freeze the object
- @xml_content = @xml_wrapper_start = @xml_wrapper_end = ''
- freeze
+ # Reserve a name from the namer unless one has already been reserved.
+ # Safe to call more than once.
+ def reserve_name
+ @reserved_name ||= @location.reserve_name
end
- def finalized?
- frozen?
+ # Return a boolean indicating whether a name has been reserved
+ def reserved_name?
+ !!@reserved_name
end
- # Return a new instance of the sitemap file with the same options, and the next name in the sequence.
+ # Return a new instance of the sitemap file with the same options,
+ # and the next name in the sequence.
def new
location = @location.dup
location.delete(:filename) if location.namer
@@ -23,15 +23,47 @@ def initialize(opts={})
@xml_wrapper_start.gsub!(/\s+/, ' ').gsub!(/ *> */, '>').strip!
@xml_wrapper_end = %q[</sitemapindex>]
@filesize = bytesize(@xml_wrapper_start) + bytesize(@xml_wrapper_end)
+ @written = false
+ @reserved_name = nil # holds the name reserved from the namer
+ @frozen = false # rather than actually freeze, use this boolean
+ @first_sitemap = nil # reference to the first thing added to this index
end
# Finalize sitemaps as they are added to the index.
+ # If it's the first sitemap, finalize it but don't
+ # write it out, because we don't yet know if we need an index. If it's
+ # the second sitemap, we know we need an index, so reserve a name for the
+ # index, and go and write out the first sitemap. If it's the third or
+ # greater sitemap, just finalize and write it out as usual, nothing more
+ # needs to be done.
+ alias_method :super_add, :add
def add(link, options={})
if file = link.is_a?(SitemapFile) && link
@sitemaps_link_count += file.link_count
file.finalize! unless file.finalized?
+
+ # First link. If it's a SitemapFile store a reference to it and the options
+ # so that we can create a URL from it later. We can't create the URL yet
+ # because doing so fixes the sitemap file's name, and we have to wait to see
+ # if we have more than one link in the index before we can know who gets the
+ # first name (the index, or the sitemap). If the item is not a SitemapFile,
+ # then it has been manually added and we can be sure that the user intends
+ # for there to be an index.
+ if @link_count == 0
+ @first_sitemap = SitemapGenerator::Builder::LinkHolder.new(file, options)
+ @link_count += 1 # pretend it's added
+ elsif @link_count == 1 # adding second link, need an index so reserve names & write out first sitemap
+ reserve_name unless @location.create_index == false # index gets first name
+ write_first_sitemap
+ file.write
+ super(SitemapGenerator::Builder::SitemapIndexUrl.new(file, options))
+ else
+ file.write
+ super(SitemapGenerator::Builder::SitemapIndexUrl.new(file, options))
+ end
+ else
+ super(SitemapGenerator::Builder::SitemapIndexUrl.new(link, options))
end
- super(SitemapGenerator::Builder::SitemapIndexUrl.new(link, options))
end
# Return a boolean indicating whether the sitemap file can fit another link
@@ -59,6 +91,35 @@ def stats_summary(opts={})
str = "Sitemap stats: #{number_with_delimiter(@sitemaps_link_count)} links / #{@link_count} sitemaps"
str += " / %dm%02ds" % opts[:time_taken].divmod(60) if opts[:time_taken]
end
+
+ def finalize!
+ raise SitemapGenerator::SitemapFinalizedError if finalized?
+ reserve_name if create_index?
+ write_first_sitemap
+ @frozen = true
+ end
+
+ # Write out the index if an index is needed
+ def write
+ super if create_index?
+ end
+
+ # Whether or not we need to create an index file.
+ def create_index?
+ @location.create_index == true || @location.create_index == :auto && @link_count > 1
+ end
+
+ protected
+
+ # Make sure the first sitemap has been written out and added to the index
+ def write_first_sitemap
+ if @first_sitemap
+ @first_sitemap.link.write unless @first_sitemap.link.written?
+ super_add(SitemapGenerator::Builder::SitemapIndexUrl.new(@first_sitemap.link, @first_sitemap.options))
+ @link_count -= 1 # we already counted it, don't count it twice
+ @first_sitemap = nil
+ end
+ end
end
end
end
@@ -297,6 +297,7 @@ def sitemap_index_url
sitemap_index.location.url
end
+ # All done. Write out remaining files.
def finalize!
finalize_sitemap!
finalize_sitemap_index!
@@ -417,19 +418,17 @@ def finalize_sitemap!
add_default_links if !@added_default_links && !@created_group
# This will finalize it. We add to the index even if not creating an index because
# the index keeps track of how many links are in our sitemaps and we need this info
- # for the summary line. If not for that problem, I would add the sitemap to
- # the index only if create_index is truthy.
+ # for the summary line. Also the index determines which file gets the first name
+ # so everything has to go via the index.
add_to_index(sitemap)
- output(sitemap.summary)
end
# Finalize a sitemap index and output a summary line. Do nothing if it has already
# been finalized.
def finalize_sitemap_index!
- return if @protect_index || !@create_index || sitemap_index.finalized?
- return if @create_index == :auto && sitemap_index.link_count <= 1
+ return if @protect_index || sitemap_index.finalized?
sitemap_index.finalize!
- output(sitemap_index.summary)
+ sitemap_index.write
end
# Return the interpreter linked to this instance.
@@ -559,7 +558,8 @@ def sitemap_location
:namer => sitemaps_namer,
:public_path => public_path,
:sitemaps_path => @sitemaps_path,
- :adapter => @adapter
+ :adapter => @adapter,
+ :verbose => verbose
)
end
@@ -570,7 +570,9 @@ def sitemap_index_location
:namer => sitemap_index_namer,
:public_path => public_path,
:sitemaps_path => @sitemaps_path,
- :adapter => @adapter
+ :adapter => @adapter,
+ :verbose => verbose,
+ :create_index => @create_index
)
end
@@ -29,11 +29,15 @@ class SitemapLocation < Hash
# directory if running under Rails.
# * <tt>sitemaps_path</tt> - gives the path relative to the <tt>public_path</tt> in which to
# write sitemaps e.g. <tt>sitemaps/</tt>.
+ # * <tt>verbose</tt> - whether to output summary into to STDOUT. Default +false+.
+ # * <tt>create_index</tt> - whether to create a sitemap index. Default +true+. See LinkSet.
+ # Only applies to the SitemapIndexLocation object.
def initialize(opts={})
- SitemapGenerator::Utilities.assert_valid_keys(opts, [:adapter, :public_path, :sitemaps_path, :host, :filename, :namer])
+ SitemapGenerator::Utilities.assert_valid_keys(opts, [:adapter, :public_path, :sitemaps_path, :host, :filename, :namer, :verbose, :create_index])
opts[:adapter] ||= SitemapGenerator::FileAdapter.new
opts[:public_path] ||= SitemapGenerator.app.root + 'public/'
opts[:namer] = SitemapGenerator::SitemapNamer.new(:sitemap) if !opts[:filename] && !opts[:namer]
+ opts[:verbose] = !!opts[:verbose]
self.merge!(opts)
end
@@ -78,10 +82,30 @@ def filename
self[:filename]
end
+ # If a namer is set, reserve the filename and increment the namer.
+ # Returns the reserved name.
+ def reserve_name
+ if self[:namer]
+ filename
+ self[:namer].next
+ end
+ self[:filename]
+ end
+
+ # Return true if this location has a fixed filename. If no name has been
+ # reserved from the namer, for instance, returns false.
+ def reserved_name?
+ !!self[:filename]
+ end
+
def namer
self[:namer]
end
+ def verbose?
+ self[:verbose]
+ end
+
# If you set the filename, clear the namer and vice versa.
def []=(key, value, opts={})
if !opts[:super]
@@ -107,5 +131,11 @@ def initialize(opts={})
end
super(opts)
end
+
+ # Really just a placeholder for an option which should really go into some
+ # kind of options class.
+ def create_index
+ self[:create_index]
+ end
end
end
@@ -32,23 +32,25 @@
@s.finalized?.should be_false
end
- it "should increment the namer after finalizing" do
- @s.finalize!
- @s.location.filename.should_not == @s.location.namer.to_s
- end
-
it "should raise if no default host is set" do
lambda { SitemapGenerator::Builder::SitemapFile.new.location.url }.should raise_error(SitemapGenerator::SitemapError)
end
describe "lastmod" do
it "should be the file last modified time" do
lastmod = (Time.now - 1209600)
+ @s.location.reserve_name
File.expects(:mtime).with(@s.location.path).returns(lastmod)
@s.lastmod.should == lastmod
end
- it "should be nil if the file DNE" do
+ it "should be nil if the location has not reserved a name" do
+ File.expects(:mtime).never
+ @s.lastmod.should be_nil
+ end
+
+ it "should be nil if location has reserved a name and the file DNE" do
+ @s.location.reserve_name
File.expects(:mtime).raises(Errno::ENOENT)
@s.lastmod.should be_nil
end
@@ -190,9 +190,7 @@
let(:ls) { SitemapGenerator::LinkSet.new(:default_host => default_host, :verbose => true) }
it "should output summary lines" do
- ls.sitemap.expects(:finalize!)
ls.sitemap.expects(:summary)
- ls.sitemap_index.expects(:finalize!)
ls.sitemap_index.expects(:summary)
ls.finalize!
end
@@ -725,9 +723,9 @@
describe "when false" do
let(:ls) { SitemapGenerator::LinkSet.new(:default_host => default_host, :create_index => false) }
- it "should not finalize the index" do
+ it "should not write the index" do
ls.send(:finalize_sitemap_index!)
- ls.sitemap_index.finalized?.should be_false
+ ls.sitemap_index.written?.should be_false
end
it "should still add finalized sitemaps to the index (but the index is never finalized)" do
@@ -753,29 +751,29 @@
describe "when :auto" do
let(:ls) { SitemapGenerator::LinkSet.new(:default_host => default_host, :create_index => :auto) }
- it "should not finalize the index when it is empty" do
+ it "should not write the index when it is empty" do
ls.sitemap_index.empty?.should be_true
ls.send(:finalize_sitemap_index!)
- ls.sitemap_index.finalized?.should be_false
+ ls.sitemap_index.written?.should be_false
end
it "should add finalized sitemaps to the index" do
ls.expects(:add_to_index).with(ls.sitemap).once
ls.send(:finalize_sitemap!)
end
- it "should not finalize the index when it has only one link" do
+ it "should not write the index when it has only one link" do
ls.sitemap_index.add '/test', :host => default_host
ls.sitemap_index.empty?.should be_false
ls.send(:finalize_sitemap_index!)
- ls.sitemap_index.finalized?.should be_false
+ ls.sitemap_index.written?.should be_false
end
- it "should finalize the index when it has more than one link" do
+ it "should write the index when it has more than one link" do
ls.sitemap_index.add '/test1', :host => default_host
ls.sitemap_index.add '/test2', :host => default_host
ls.send(:finalize_sitemap_index!)
- ls.sitemap_index.finalized?.should be_true
+ ls.sitemap_index.written?.should be_true
end
end
end

0 comments on commit 30e717b

Please sign in to comment.