Refactoring Artist model

Rakefile

@@ -6,3 +6,8 @@ RSpec::Core::RakeTask.new("spec")
 
 task :default => :spec
 task :test => :spec
+
+desc "Open an irb session preloaded with this library"
+task :console do
+  sh "irb -rubygems -I lib -r musicbrainz.rb"
+end

lib/musicbrainz.rb

@@ -4,18 +4,25 @@ require "socket"
 require "nokogiri"
 require "cgi"
 
+module MusicBrainz
+  module Tools
+  end
+  module Parsers
+  end
+end
+
 require "version"
 require "deprecated"
 
-module MusicBrainz
+require "musicbrainz/base"
-  autoload :Base, "musicbrainz/base"
+require "musicbrainz/artist"
-  autoload :Artist, "musicbrainz/artist"
+require "musicbrainz/release_group"
-  autoload :ReleaseGroup, "musicbrainz/release_group"
+require "musicbrainz/release"
-  autoload :Release, "musicbrainz/release"
+require "musicbrainz/track"
-  autoload :Track, "musicbrainz/track"
 
-  module Tools
+require "tools/cache"
-    autoload :Cache, "tools/cache"
+require "tools/proxy"
-    autoload :Proxy, "tools/proxy"
+
-  end
+require "parsers/base"
-end
+require "parsers/artist"
+require "parsers/release_group"

lib/musicbrainz/artist.rb

@@ -1,81 +1,78 @@
 # -*- encoding: utf-8 -*-
 
 module MusicBrainz
-  class Artist < MusicBrainz::Base
+  class Artist < Base
-    attr_accessor :id, :type, :name, :country, :date_begin, :date_end, :urls
+
-    @release_groups
+    field :id, String
+    field :type, String
+    field :name, String
+    field :country, String
+    field :date_begin, Time
+    field :date_end, Time
+    field :urls, Hash
 
     def release_groups
-      if @release_groups.nil? and not self.id.nil?
+      @release_groups ||= nil
-        @release_groups = []
+      if @release_groups.nil? and !id.nil?
-        Nokogiri::XML(self.class.load(:release_group, :artist => self.id)).css('release-group').each do |rg|
+        @release_groups = self.class.load({
-          @release_groups << MusicBrainz::ReleaseGroup.parse_xml(rg)
+          :parser => :artist_release_groups,
+          :create_models => MusicBrainz::ReleaseGroup
+        }, {
+          :resource => :release_group,
+          :artist => id
+        })
+        @release_groups.sort!{ |a, b| a.first_release_date <=> b.first_release_date }
       end
-      end
+      @release_groups
-      @release_groups.sort{ |a, b| a.first_release_date <=> b.first_release_date }
     end
 
-    def self.find mbid
+    class << self
-      res = self.load :artist, :id => mbid, :inc => [:url_rels]
+      def find(mbid)
-      return nil if res.nil?
+        load({
-      @artist = self.parse_xml(Nokogiri::XML(res))
+          :parser => :artist_model,
+          :create_model => MusicBrainz::Artist
+        }, {
+          :resource => :artist,
+          :id => mbid,
+          :inc => [:url_rels]
+        })
       end
 
-    def self.parse_xml xml
+      def search(name)
-      @artist = MusicBrainz::Artist.new
+        artists = load({
-      @artist.id = self.safe_get_attr(xml, 'artist', 'id')
+          :parser => :artist_search
-      @artist.type = self.safe_get_attr(xml, 'artist', 'type')
+        }, {
-      @artist.name = self.safe_get_value(xml, 'artist > name').gsub(/[`’]/, "'")
+          :resource => :artist,
-      @artist.country = self.safe_get_value(xml, 'artist > country')
+          :query => CGI.escape(name).gsub(/\!/, '\!') + '~',
-      @artist.date_begin = self.safe_get_value(xml, 'artist > life-span > begin')
+          :limit => 50
-      @artist.date_end = self.safe_get_value(xml, 'artist > life-span > end')
+        })
-      @artist.urls = {}
+
-      xml.css('relation-list[target-type="url"] > relation').each do |rel|
+        artists.each { |artist|
-        @artist.urls[rel.attr('type').downcase.split(' ').join('_').to_sym] = rel.css('target').text
+          if artist[:name].downcase == name.downcase
+            artist[:weight] += 80
+          elsif artist[:name].downcase.gsub(/\s/, "") == name.downcase.gsub(/\s/, "")
+            artist[:weight] += 25
+          elsif artist[:aliases].include? name
+            artist[:weight] += 20
+          elsif artist[:aliases].map { |item| item.downcase }.include?(name.downcase)
+            artist[:weight] += 10
+          elsif artist[:aliases].map { |item| item.downcase.gsub(/\s/, "") }.include?(name.downcase.gsub(/\s/, ""))
+            artist[:weight] += 5
           end
-      @artist
+        }
+        artists.sort{ |a, b| b[:weight] <=> a[:weight] }.take(10)
       end
 
-    def self.discography mbid
+      def discography(mbid)
-      artist = self.find(mbid)
+        artist = find(mbid)
-      artist.release_groups.each {|rg| rg.releases.each {|r| r.tracks } }
+        artist.release_groups.each { |rg| rg.releases.each { |r| r.tracks } }
         artist
       end
 
-    def self.find_by_name name
+      def find_by_name(name)
-      matches = self.search name
+        matches = search(name)
-      matches.length.zero? ? nil : self.find(matches.first[:mbid])
+        matches.length.zero? ? nil : find(matches.first[:mbid])
       end
-
-    def self.search name
-      artists = []
-      xml = Nokogiri::XML(self.load(:artist, :query => CGI.escape(name).gsub(/\!/, '\!') + '~', :limit => 50))
-      xml.css('artist-list > artist').each do |a|
-        artist = {
-          :name => a.first_element_child.text.gsub(/[`’]/, "'"),
-          :sort_name => self.safe_get_value(a, 'sort-name').gsub(/[`’]/, "'"),
-          :weight => 0,
-          :desc => self.safe_get_value(a, 'disambiguation'),
-          :type => self.safe_get_attr(a, nil, 'type'),
-          :mbid => self.safe_get_attr(a, nil, 'id')
-        }
-        aliases = a.css('alias-list > alias').map{ |item| item.text }
-        if artist[:name] == name
-          artist[:weight] += 100
-        elsif artist[:name].downcase == name.downcase
-          artist[:weight] += 50
-        elsif artist[:name].downcase.gsub(/\s/, '') == name.downcase.gsub(/\s/, '')
-          artist[:weight] += 25
-        elsif aliases.include? name
-          artist[:weight] += 20
-        elsif aliases.map{ |item| item.downcase }.include? name.downcase
-          artist[:weight] += 10
-        elsif aliases.map{ |item| item.downcase.gsub(/\s/, '') }.include? name.downcase.gsub(/\s/, '')
-          artist[:weight] += 5
-        end
-        artists << artist
-      end
-      artists.sort{ |a, b| b[:weight] <=> a[:weight] }.take(10)
     end
   end
 end

lib/musicbrainz/base.rb

@@ -1,17 +1,71 @@
 # -*- encoding: utf-8 -*-
+
 module MusicBrainz
   class Base
-    def self.safe_get_attr(xml, path, name)
+    class << self
-      node = path.nil? ? xml : (xml.css(path).first unless xml.css(path).empty?)
+      def field(name, type)
-      node.attr(name) unless node.nil? or node.attr(name).nil?
+        @fields ||= {}
+        @fields[name] = type
+
+        define_method(name) {
+          instance_variable_get("@#{name}")
+        }
+        define_method("#{name}=") { |val|
+          instance_variable_set("@#{name}", validate_type(val, type))
+        }
       end
 
-    def self.safe_get_value(xml, path)
+      def load(params, query)
-      xml.css(path).first.text unless xml.css(path).empty?
+        parser = MusicBrainz::Parsers.get_by_name(params[:parser])
+        xml = MusicBrainz::Tools::Proxy.load(query)
+        result = parser[:const].send(parser[:method], Nokogiri::XML(xml))
+        if params[:create_model]
+          result_model = params[:create_model].new
+          result.each { |field, value|
+            result_model.send("#{field}=".to_sym, value)
+          }
+          result_model
+        elsif params[:create_models]
+          result_models = []
+          result.each { |item|
+            result_model = params[:create_models].new
+            item.each { |field, value|
+              result_model.send("#{field}=".to_sym, value)
+            }
+            result_models << result_model
+          }
+          result_models
+        else
+          result
+        end
+      end
     end
 
-    def self.load(*args)
+    def initialize
-      MusicBrainz::Tools::Proxy.load(*args)
+      self.class.instance_variable_get("@fields").each { |name, type|
+        instance_variable_set("@#{name}", nil)
+      }
+    end
+
+    def validate_type(val, type)
+      if type == Integer
+        val.to_i
+      elsif type == Float
+        val.to_f
+      elsif type == String
+        val.to_s
+      elsif type == Time
+        if val.nil? or val == ""
+          val = "2030-12-31"
+        elsif val.split("-").length == 1
+          val << "-12-31"
+        elsif val.split("-").length == 2
+          val << "-31"
+        end
+        Time.utc(*val.split("-"))
+      else
+        val
+      end
     end
   end
 end

lib/musicbrainz/release_group.rb

@@ -1,8 +1,12 @@
 # -*- encoding: utf-8 -*-
 module MusicBrainz
   class ReleaseGroup < MusicBrainz::Base
-    attr_accessor :id, :type, :title, :disambiguation, :first_release_date
+
-    @releases
+    field :id, String
+    field :type, String
+    field :title, String
+    field :disambiguation, String
+    field :first_release_date, Time
 
     def releases
       if @releases.nil? and not self.id.nil?

lib/parsers/artist.rb

@@ -0,0 +1,49 @@
+# -*- encoding: utf-8 -*-
+
+module MusicBrainz
+  module Parsers
+    class Artist < Base
+      class << self
+        def model(xml)
+          res = {
+            :id => safe_get_attr(xml, "artist", "id"),
+            :type => safe_get_attr(xml, "artist", "type"),
+            :name => safe_get_value(xml, "artist > name").gsub(/[`’]/, "'"),
+            :country => safe_get_value(xml, "artist > country"),
+            :date_begin => safe_get_value(xml, "artist > life-span > begin"),
+            :date_end => safe_get_value(xml, "artist > life-span > end"),
+            :urls => {}
+          }
+          xml.css("relation-list[target-type='url'] > relation").each { |rel|
+            res[:urls][rel.attr("type").downcase.split(" ").join("_").to_sym] = rel.css("target").text
+          }
+          res
+        end
+
+        def search(xml)
+          artists = []
+          xml.css("artist-list > artist").each do |a|
+            artists << {
+              :name => a.first_element_child.text.gsub(/[`’]/, "'"),
+              :sort_name => self.safe_get_value(a, "sort-name").gsub(/[`’]/, "'"),
+              :weight => 0,
+              :desc => self.safe_get_value(a, "disambiguation"),
+              :type => self.safe_get_attr(a, nil, "type"),
+              :mbid => self.safe_get_attr(a, nil, "id"),
+              :aliases => a.css("alias-list > alias").map { |item| item.text }
+            }
+          end
+          artists
+        end
+
+        def release_groups(xml)
+          release_groups = []
+          xml.css("release-group").each do |rg|
+            release_groups << MusicBrainz::Parsers::ReleaseGroup.model(rg)
+          end
+          release_groups
+        end
+      end
+    end
+  end
+end

lib/parsers/base.rb

@@ -0,0 +1,30 @@
+# -*- encoding: utf-8 -*-
+module MusicBrainz
+  module Parsers
+    class << self
+      def get_by_name(name)
+        case name
+        when :artist_model
+          return { :const => MusicBrainz::Parsers::Artist, :method => :model }
+        when :artist_search
+          return { :const => MusicBrainz::Parsers::Artist, :method => :search }
+        when :artist_release_groups
+          return { :const => MusicBrainz::Parsers::Artist, :method => :release_groups }
+        end
+      end
+    end
+
+    class Base
+      class << self
+        def safe_get_attr(xml, path, name)
+          node = path.nil? ? xml : (xml.css(path).first unless xml.css(path).empty?)
+          node.attr(name) unless node.nil? or node.attr(name).nil?
+        end
+
+        def safe_get_value(xml, path)
+          xml.css(path).first.text unless xml.css(path).empty?
+        end
+      end
+    end
+  end
+end

lib/parsers/release_group.rb

@@ -0,0 +1,19 @@
+# -*- encoding: utf-8 -*-
+
+module MusicBrainz
+  module Parsers
+    class ReleaseGroup < Base
+      class << self
+        def model(xml)
+          {
+            :id => safe_get_attr(xml, nil, "id"),
+            :type => safe_get_attr(xml, nil, "type"),
+            :title => safe_get_value(xml, "title"),
+            :disambiguation => safe_get_value(xml, "disambiguation"),
+            :first_release_date => safe_get_value(xml, "first-release-date")
+          }
+        end
+      end
+    end
+  end
+end

lib/tools/proxy.rb

@@ -21,8 +21,8 @@ module MusicBrainz
         @@tries_limit = num.to_i
       end
 
-      def self.load(resourse, params = {})
+      def self.load(params = {})
-        url = WEB_SERVICE_URL + resourse.to_s.gsub('_', '-') + '/' + (params[:id].to_s || '')
+        url = WEB_SERVICE_URL + params[:resource].to_s.gsub('_', '-') + '/' + (params[:id].to_s || '')
         params.delete(:id) unless params[:id].nil?
         url << '?' + params.map{ |k, v|
           k = k.to_s.gsub('_', '-')

spec/requests/artist_spec.rb

@@ -21,7 +21,7 @@ describe MusicBrainz::Artist do
   it "finds name first than alias" do
     matches = MusicBrainz::Artist.search('Chris Martin')
     matches.length.should be > 0
-    matches.first[:name].should == "Chris Martin"
+    matches.first[:mbid].should == "af2ab893-3212-4226-9e73-73a1660b6952"
   end
 
   it "gets correct result by name" do
@@ -35,7 +35,7 @@ describe MusicBrainz::Artist do
     artist.type.should == "Group"
     artist.name.should == "Kasabian"
     artist.country.should == "GB"
-    artist.date_begin.should == "1999"
+    artist.date_begin.year.should == 1999
   end
 
   it "gets correct artist's release groups" do