Merge pull request #345 from barmintor/datastream_helpers

remove copycode in DS model helpers

lib/active_fedora/datastreams.rb

@@ -199,23 +199,16 @@ def datastream_class_for_name(dsid)
       # @option args [Boolean] :versionable Should versioned datastreams be stored
       # @yield block executed by some kinds of datastreams
       def has_metadata(*args, &block)
-
-        if args.first.is_a? String 
-          name = args.first
-          args = args[1] || {}
-          args[:name] = name
-        else
-          args = args.first
-        end
-
-
-        spec = {:autocreate => args.fetch(:autocreate, false), :type => args[:type], :label =>  args.fetch(:label,""), :control_group => args[:control_group], :disseminator => args.fetch(:disseminator,""), :url => args.fetch(:url,""),:block => block}
-        name = args[:name]
-        raise ArgumentError, "You must provide a name (dsid) for the metadata datastream" unless name
-        raise ArgumentError, "You must provide a :type property for the metadata datastream '#{name}'" unless spec[:type]
-        spec[:versionable] = args[:versionable] if args.has_key? :versionable
-        build_datastream_accessor(name)
-        ds_specs[name]= spec
+        @metadata_ds_defaults ||= {
+          :autocreate => false,
+          :type=>nil,
+          :label=>"",
+          :control_group=>nil,
+          :disseminator=>"",
+          :url=>"",
+          :name=>nil
+        }
+        spec_datastream(args, @metadata_ds_defaults, &block)
       end
 
 
@@ -238,19 +231,14 @@ def has_metadata(*args, &block)
       #     @option args [Boolean] :autocreate Always create this datastream on new objects
       #     @option args [Boolean] :versionable Should versioned datastreams be stored
       def has_file_datastream(*args)
-        if args.first.is_a? String 
-          name = args.first
-          args = args[1] || {}
-          args[:name] = name
-        else
-          args = args.first || {}
-        end
-        spec = {:autocreate => args.fetch(:autocreate, false), :type => args.fetch(:type,ActiveFedora::Datastream),
-                :label =>  args.fetch(:label,"File Datastream"), :control_group => args.fetch(:control_group,"M")}
-        spec[:versionable] = args[:versionable] if args.has_key? :versionable
-        name = args.fetch(:name, "content")
-        build_datastream_accessor(name)
-        ds_specs[name]= spec
+        @file_ds_defaults ||= {
+          :autocreate => false,
+          :type=>ActiveFedora::Datastream,
+          :label=>"File Datastream",
+          :control_group=>"M",
+          :name=>"content"
+        }
+        spec_datastream(args, @file_ds_defaults)
       end
 
       def build_datastream_accessor(dsid)
@@ -263,6 +251,28 @@ def build_datastream_accessor(dsid)
 
       private
 
+      # Creates a datastream spec combining the given args and default values
+      # @param args [Array] either [String, Hash] or [Hash]; the latter must .has_key? :name
+      # @param defaults [Hash] the default values for the datastream spec
+      # @yield block executed by some kinds of datastreams
+      def spec_datastream(args, defaults, &block)
+        if args.first.is_a? String 
+          name = args.first
+          args = args[1] || {}
+          args[:name] = name
+        else
+          args = args.first || {}
+        end
+        spec = defaults.merge(args.select {|key, value| defaults.has_key? key})
+        name = spec.delete(:name)
+        raise ArgumentError, "You must provide a name (dsid) for the datastream" unless name
+        raise ArgumentError, "You must provide a :type property for the datastream '#{name}'" unless spec[:type]
+        spec[:versionable] = args[:versionable] if args.has_key? :versionable
+        spec[:block] = block if block
+        build_datastream_accessor(name)
+        ds_specs[name]= spec
+      end
+
         ## Given a dsid return a standard name
         def name_for_dsid(dsid)
           dsid.gsub('-', '_')

spec/unit/datastreams_spec.rb

@@ -25,12 +25,12 @@ class FooHistory < ActiveFedora::Base
 
     it "should raise an error if you don't give a type" do
       expect{ FooHistory.has_metadata "bob" }.to raise_error ArgumentError,
-        "You must provide a :type property for the metadata datastream 'bob'"
+        "You must provide a :type property for the datastream 'bob'"
     end
 
     it "should raise an error if you don't give a dsid" do
       expect{ FooHistory.has_metadata type: ActiveFedora::SimpleDatastream }.to raise_error ArgumentError,
-        "You must provide a name (dsid) for the metadata datastream"
+        "You must provide a name (dsid) for the datastream"
     end
   end