Refactored Release and Track models, fixed tests

lib/deprecated.rb

@@ -1,12 +1,25 @@
 # -*- encoding: utf-8 -*-
-module MusicBrainz
-  def self.query_interval=(sec)
-    MusicBrainz::Tools::Proxy.query_interval = sec
-    STDOUT.send :puts, "WARNING! MusicBrainz.query_interval is deprecated. Use MusicBrainz::Tools::Proxy.query_interval"
-  end
 
-  def self.cache_path=(path)
+module MusicBrainz
-    MusicBrainz::Tools::Cache.cache_path = path
+  class << self
-    $stdout.send :puts, "WARNING! MusicBrainz.cache_path is deprecated. Use MusicBrainz::Tools::Cache.cache_path"
+    def query_interval
+      $stdout.send :puts, "WARNING! MusicBrainz.query_interval is deprecated. Use MusicBrainz::Tools::Proxy.query_interval"
+      MusicBrainz::Tools::Proxy.query_interval
+    end
+
+    def query_interval=(sec)
+      $stdout.send :puts, "WARNING! MusicBrainz.query_interval= is deprecated. Use MusicBrainz::Tools::Proxy.query_interval"
+      MusicBrainz::Tools::Proxy.query_interval = sec
+    end
+
+    def cache_path
+      $stdout.send :puts, "WARNING! MusicBrainz.cache_path is deprecated. Use MusicBrainz::Tools::Cache.cache_path"
+      MusicBrainz::Tools::Cache.cache_path
+    end
+
+    def cache_path=(path)
+      $stdout.send :puts, "WARNING! MusicBrainz.cache_path= is deprecated. Use MusicBrainz::Tools::Cache.cache_path"
+      MusicBrainz::Tools::Cache.cache_path = path
+    end
   end
 end

lib/musicbrainz.rb

@@ -1,4 +1,5 @@
 # -*- encoding: utf-8 -*-
+
 require "open-uri"
 require "socket"
 require "nokogiri"

lib/musicbrainz/release.rb

@@ -1,6 +1,7 @@
 # -*- encoding: utf-8 -*-
+
 module MusicBrainz
-  class Release < MusicBrainz::Base
+  class Release < Base
 
     field :id, String
     field :title, String
@@ -10,19 +11,32 @@ module MusicBrainz
     field :country, String
 
     def tracks
-      if @tracks.nil? and not self.id.nil?
+      @tracks ||= nil
-        @tracks = []
+      if @tracks.nil? and !id.nil?
-        Nokogiri::XML(self.class.load(:release, :id => self.id, :inc => [:recordings, :media], :limit => 100)).css('medium-list > medium > track-list > track').each do |r|
+        @tracks = self.class.load({
-          @tracks << MusicBrainz::Track.parse_xml(r)
+          :parser => :release_tracks,
-        end
+          :create_models => MusicBrainz::Track
+        }, {
+          :resource => :release,
+          :id => id,
+          :inc => [:recordings, :media],
+          :limit => 100
+        })
+        @tracks.sort{ |a, b| a.position <=> b.position }
       end
-      @tracks.sort{ |a, b| a.position <=> b.position }
+      @tracks
     end
 
     class << self
       def find(mbid)
-        xml = Nokogiri::XML(self.load(:release, :id => mbid, :inc => [:media])).css('release').first
+        load({
-        self.parse_xml(xml) unless xml.nil?
+          :parser => :release_model,
+          :create_model => MusicBrainz::Release
+        }, {
+          :resource => :release,
+          :id => mbid,
+          :inc => [:media]
+        })
       end
     end
   end

lib/musicbrainz/release_group.rb

@@ -1,6 +1,7 @@
 # -*- encoding: utf-8 -*-
+
 module MusicBrainz
-  class ReleaseGroup < MusicBrainz::Base
+  class ReleaseGroup < Base
 
     field :id, String
     field :type, String
@@ -20,8 +21,9 @@ module MusicBrainz
           :inc => [:media],
           :limit => 100
         })
+        @releases.sort!{ |a, b| a.date <=> b.date }
       end
-      @releases.sort{ |a, b| a.date <=> b.date }
+      @releases
     end
 
     class << self

lib/musicbrainz/track.rb

@@ -1,21 +1,23 @@
 # -*- encoding: utf-8 -*-
+
 module MusicBrainz
-  class Track < MusicBrainz::Base
+  class Track < Base
-    attr_accessor :position, :recording_id, :title, :length
 
-    def self.find mbid
+    field :position, Integer
-      xml = Nokogiri::XML(self.load(:recording, :id => mbid))
+    field :recording_id, String
-      self.parse_xml(xml) unless xml.nil?
+    field :title, String
-    end
+    field :length, Integer
 
-    def self.parse_xml xml
+    class << self
-      @track = MusicBrainz::Track.new
+      def find(mbid)
-      @track.position = self.safe_get_value(xml, 'position').to_i
+        load({
-      @track.recording_id = self.safe_get_attr(xml, 'recording', 'id')
+          :parser => :track_model,
-      @track.title = self.safe_get_value(xml, 'recording > title')
+          :create_model => MusicBrainz::Track
-      @track.length = self.safe_get_value(xml, 'length').to_i
+        }, {
-      @track.length = self.safe_get_value(xml, 'recording > length').to_i
+          :resource => :recording,
-      @track
+          :id => mbid
+        })
+      end
     end
   end
 end

lib/parsers/release.rb

@@ -6,7 +6,7 @@ module MusicBrainz
       class << self
         def model(xml)
           {
-            :id => safe_get_attr(xml, nil, "id") || safe_get_attr(xml, "release-group", "id"),
+            :id => safe_get_attr(xml, nil, "id") || safe_get_attr(xml, "release", "id"),
             :title => safe_get_value(xml, "title"),
             :status => safe_get_value(xml, "status"),
             :country => safe_get_value(xml, "country"),
@@ -14,6 +14,14 @@ module MusicBrainz
             :date => safe_get_value(xml, "date")
           }
         end
+
+        def tracks(xml)
+          tracks = []
+          xml.css("medium-list > medium > track-list > track").each do |r|
+            tracks << MusicBrainz::Parsers::Track.model(r)
+          end
+          tracks
+        end
       end
     end
   end

lib/parsers/track.rb

@@ -4,7 +4,14 @@ module MusicBrainz
   module Parsers
     class Track < Base
       class << self
-
+        def model(xml)
+          {
+            :position => safe_get_value(xml, "position"),
+            :recording_id => safe_get_attr(xml, "recording", "id"),
+            :title => safe_get_value(xml, "recording > title"),
+            :length => safe_get_value(xml, "length") || safe_get_value(xml, "recording > length")
+          }
+        end
       end
     end
   end

lib/tools/proxy.rb

@@ -23,6 +23,7 @@ module MusicBrainz
 
       def self.load(params = {})
         url = WEB_SERVICE_URL + params[:resource].to_s.gsub('_', '-') + '/' + (params[:id].to_s || '')
+        params.delete(:resource)
         params.delete(:id) unless params[:id].nil?
         url << '?' + params.map{ |k, v|
           k = k.to_s.gsub('_', '-')

spec/fixtures/kasabian.xml

@@ -0,0 +1 @@
+<?xml version="1.0" encoding="UTF-8"?><metadata xmlns="http://musicbrainz.org/ns/mmd-2.0#"><artist type="Group" id="69b39eab-6577-46a4-a9f5-817839092033"><name>Kasabian</name><sort-name>Kasabian</sort-name><country>GB</country><life-span><begin>1999</begin></life-span><relation-list target-type="url"><relation type="allmusic"><target>http://allmusic.com/artist/p678134</target></relation><relation type="wikipedia"><target>http://en.wikipedia.org/wiki/Kasabian</target></relation><relation type="microblog"><target>http://twitter.com/kasabianhq</target></relation><relation type="BBC Music page"><target>http://www.bbc.co.uk/music/artists/69b39eab-6577-46a4-a9f5-817839092033</target></relation><relation type="discogs"><target>http://www.discogs.com/artist/Kasabian</target></relation><relation type="social network"><target>http://www.facebook.com/kasabian</target></relation><relation type="IMDb"><target>http://www.imdb.com/name/nm1868442/</target></relation><relation type="official homepage"><target>http://www.kasabian.co.uk/</target></relation><relation type="myspace"><target>http://www.myspace.com/kasabian</target></relation><relation type="youtube"><target>http://www.youtube.com/kasabianvevo</target></relation><relation type="youtube"><target>http://www.youtube.com/user/KasabianTour</target></relation></relation-list></artist></metadata>

spec/misc/deprecated_spec.rb

@@ -1,20 +1,35 @@
 require "spec_helper"
 
 describe MusicBrainz do
+  before(:all) {
+    @old_cache_path = MusicBrainz::Tools::Cache.cache_path
+  }
+
   before(:each) {
-    STDOUT.stub!(:puts)
+    $stdout.stub!(:puts)
+    MusicBrainz::Tools::Cache.cache_path = nil
   }
 
   after(:all) {
-    MusicBrainz.cache_path.nil
+    MusicBrainz::Tools::Cache.cache_path = @old_cache_path
   }
 
   it "allows deprecated use of cache_path" do
+    MusicBrainz::Tools::Cache.cache_path = "some/path"
+    MusicBrainz::cache_path.should == "some/path"
+  end
+
+  it "allows deprecated use of cache_path=" do
     MusicBrainz.cache_path = "some/path"
     MusicBrainz::Tools::Cache.cache_path.should == "some/path"
   end
 
   it "allows deprecated use of query_interval" do
+    MusicBrainz::Tools::Proxy.query_interval = 2
+    MusicBrainz::query_interval.should == 2
+  end
+
+  it "allows deprecated use of query_interval=" do
     MusicBrainz.query_interval = 2
     MusicBrainz::Tools::Proxy.query_interval.should == 2
   end

spec/tools/cache_spec.rb

@@ -11,32 +11,17 @@ describe MusicBrainz::Tools::Cache do
   end
 
   before(:each) do
-    @test_response = ::StringIO.new('<?xml version="1.0" encoding="UTF-8"?>'+
+    file_path = File.join(File.dirname(__FILE__), "../fixtures/kasabian.xml")
-      '<metadata xmlns="http://musicbrainz.org/ns/mmd-2.0#">'+
+    @test_response = ::StringIO.new(File.open(file_path).gets)
-      '<artist type="Group" id="69b39eab-6577-46a4-a9f5-817839092033">'+
-      '<name>Kasabian</name><sort-name>Kasabian</sort-name><country>GB</country>'+
-      '<life-span><begin>1999</begin></life-span><relation-list target-type="url">'+
-      '<relation type="allmusic"><target>http://allmusic.com/artist/p678134</target>'+
-      '</relation><relation type="wikipedia"><target>http://en.wikipedia.org/wiki/Kasabian</target>'+
-      '</relation><relation type="microblog"><target>http://twitter.com/kasabianhq</target>'+
-      '</relation><relation type="BBC Music page"><target>'+
-      'http://www.bbc.co.uk/music/artists/69b39eab-6577-46a4-a9f5-817839092033</target></relation>'+
-      '<relation type="discogs"><target>http://www.discogs.com/artist/Kasabian</target></relation>'+
-      '<relation type="social network"><target>http://www.facebook.com/kasabian</target></relation>'+
-      '<relation type="IMDb"><target>http://www.imdb.com/name/nm1868442/</target></relation>'+
-      '<relation type="official homepage"><target>http://www.kasabian.co.uk/</target></relation>'+
-      '<relation type="myspace"><target>http://www.myspace.com/kasabian</target></relation>'+
-      '<relation type="youtube"><target>http://www.youtube.com/kasabianvevo</target></relation>'+
-      '<relation type="youtube"><target>http://www.youtube.com/user/KasabianTour</target></relation>'+
-      '</relation-list></artist></metadata>')
   end
 
   context "with cache enabled" do
     it "calls get contents only once when requesting the resource twice" do
       MusicBrainz::Tools::Cache.cache_path = @tmp_cache_path
+      mbid = "69b39eab-6577-46a4-a9f5-817839092033"
+
       MusicBrainz::Tools::Proxy.stub(:get_contents).and_return(@test_response)
       MusicBrainz::Tools::Proxy.should_receive(:get_contents).once
-      mbid = "69b39eab-6577-46a4-a9f5-817839092033"
 
       File.exist?("#{@tmp_cache_path}/artist/#{mbid}?inc=url-rels").should be_false
       artist = MusicBrainz::Artist.find(mbid)
@@ -53,9 +38,10 @@ describe MusicBrainz::Tools::Cache do
   context "with cache disabled" do
     it "calls get contents twice when requesting the resource twice" do
       MusicBrainz::Tools::Cache.cache_path = nil
+      mbid = "69b39eab-6577-46a4-a9f5-817839092033"
+
       MusicBrainz::Tools::Proxy.stub(:get_contents).and_return(@test_response)
       MusicBrainz::Tools::Proxy.should_receive(:get_contents).twice
-      mbid = "69b39eab-6577-46a4-a9f5-817839092033"
 
       File.exist?("#{@tmp_cache_path}/artist/#{mbid}?inc=url-rels").should be_false
       artist = MusicBrainz::Artist.find(mbid)