Refactor image hash loading to use PetService.getPet, not CPN redirs

Previously, the way we loaded the image hash for a given pet was to
navigate to `https://pets.neopets.com/cpn/<pet_name>/1/1.png`, but
*not* follow the redirect, and extract the image hash from the URL
where it redirected us to.

In this change, we refactor to use the AMFPHP RPC `PetService.getPet`
instead. I don't think it had this data last time I looked at it, but
now it does! Much prefer to use an actual RPC than our weird hacky
thing!

(We might also be able to use this call for other stuff, like
auto-labeling gender & mood for pet states, maybe?? That's in this data
too! We used to load petlookups for this, long long ago, before the
petlookup captchas got added.)

app/models/pet.rb

@@ -4,10 +4,9 @@ require 'ostruct'
 class Pet < ApplicationRecord
   NEOPETS_URL_ORIGIN = ENV['NEOPETS_URL_ORIGIN'] || 'https://www.neopets.com'
   GATEWAY_URL = NEOPETS_URL_ORIGIN + '/amfphp/gateway.php'
-  PET_VIEWER = RocketAMFExtensions::RemoteGateway.new(GATEWAY_URL).
+  GATEWAY = RocketAMFExtensions::RemoteGateway.new(GATEWAY_URL)
-    service('CustomPetService').action('getViewerData')
+  CUSTOM_PET_SERVICE = GATEWAY.service('CustomPetService')
-  PET_NOT_FOUND_REMOTE_ERROR = 'PHP: Unable to retrieve records from the database.'
+  PET_SERVICE = GATEWAY.service('PetService')
-  WARDROBE_PATH = '/wardrobe'
 
   belongs_to :pet_type
 
@@ -17,20 +16,12 @@ class Pet < ApplicationRecord
     joins(:pet_type).where(PetType.arel_table[:id].in(color_ids))
   }
 
-  def load!(options={})
+  def load!(timeout: nil)
-    options[:locale] ||= I18n.default_locale
+    viewer_data = self.class.fetch_viewer_data(name, timeout:)
-    I18n.with_locale(options.delete(:locale)) do
+    use_viewer_data(viewer_data)
-      use_viewer_data(
-        self.class.fetch_viewer_data(name, options.delete(:timeout)),
-        options,
-      )
-    end
-    true
   end
 
-  def use_viewer_data(viewer_data, options={})
+  def use_viewer_data(viewer_data)
-    options[:item_scope] ||= Item.all
-
     pet_data = viewer_data[:custom_pet]
 
     raise UnexpectedDataFormat unless pet_data[:species_id]
@@ -43,7 +34,9 @@ class Pet < ApplicationRecord
       species_id: pet_data[:species_id].to_i,
       color_id: pet_data[:color_id].to_i
     )
-    self.pet_type.origin_pet = self
+
+    new_image_hash = Pet.fetch_image_hash(self.name)
+    self.pet_type.image_hash = new_image_hash if new_image_hash.present?
 
     # With an alt style, `body_id` in the biology data refers to the body ID of
     # the *alt* style, not the usual pet type. (We have `original_biology` for
@@ -79,8 +72,7 @@ class Pet < ApplicationRecord
     end
 
     @items = Item.collection_from_pet_type_and_registries(self.pet_type,
-      viewer_data[:object_info_registry], viewer_data[:object_asset_registry],
+      viewer_data[:object_info_registry], viewer_data[:object_asset_registry])
-      options[:item_scope])
   end
 
   def wardrobe_query
@@ -122,44 +114,58 @@ class Pet < ApplicationRecord
     end
   end
 
-  def self.load(name, options={})
+  def self.load(name, **options)
     pet = Pet.find_or_initialize_by(name: name)
-    pet.load!(options)
+    pet.load!(**options)
-    pet
-  end
-
-  def self.from_viewer_data(viewer_data, options={})
-    pet = Pet.find_or_initialize_by(name: viewer_data[:custom_pet][:name])
-    pet.use_viewer_data(viewer_data, options)
     pet
   end
 
   # NOTE: Ideally pet requests shouldn't take this long, but Neopets can be
   # slow sometimes! Since we're on the Falcon server, long timeouts shouldn't
   # slow down the rest of the request queue, like it used to be in the past.
-  def self.fetch_viewer_data(name, timeout=10, locale=nil)
+  def self.fetch_viewer_data(name, timeout: 10)
-    locale ||= I18n.default_locale
+    request = CUSTOM_PET_SERVICE.action('getViewerData').request([name])
-    begin
+    send_amfphp_request(request).tap do |data|
-      neopets_language_code = I18n.compatible_neopets_language_code_for(locale)
+      if data[:custom_pet][:name].blank?
-      envelope = PET_VIEWER.request([name, 0]).post(
-        :timeout => timeout,
-        :headers => {
-          'Cookie' => "lang=#{neopets_language_code}"
-        }
-      )
-    rescue RocketAMFExtensions::RemoteGateway::AMFError => e
-      if e.message == PET_NOT_FOUND_REMOTE_ERROR
         raise PetNotFound, "Pet #{name.inspect} does not exist"
       end
-      raise DownloadError, e.message
-    rescue RocketAMFExtensions::RemoteGateway::ConnectionError => e
-      raise DownloadError, e.message, e.backtrace
     end
-    HashWithIndifferentAccess.new(envelope.messages[0].data.body)
+  end
+
+  def self.fetch_metadata(name, timeout: 10)
+    request = PET_SERVICE.action('getPet').request([name])
+    send_amfphp_request(request).tap do |data|
+      if data[:name].blank?
+        raise PetNotFound, "Pet #{name.inspect} does not exist"
+      end
+    end
+  end
+
+  # Given a pet's name, load its image hash, for use in `pets.neopets.com`
+  # image URLs. (This corresponds to its current biology and items.)
+  def self.fetch_image_hash(name, timeout: 10)
+    metadata = fetch_metadata(name, timeout:)
+    metadata[:hash]
   end
 
   class PetNotFound < RuntimeError;end
   class DownloadError < RuntimeError;end
   class UnexpectedDataFormat < RuntimeError;end
+
+  private
+
+  # Send an AMFPHP request, re-raising errors as `Pet::DownloadError`.
+  # Return the response body as a `HashWithIndifferentAccess`.
+  def self.send_amfphp_request(request, timeout: 10)
+    begin
+      response = request.post(timeout: timeout)
+    rescue RocketAMFExtensions::RemoteGateway::AMFError => e
+      raise DownloadError, e.message
+    rescue RocketAMFExtensions::RemoteGateway::ConnectionError => e
+      raise DownloadError, e.message, e.backtrace
+    end
+
+    HashWithIndifferentAccess.new(response.messages[0].data.body)
+  end
 end
 

app/models/pet_type.rb

@@ -1,8 +1,4 @@
 class PetType < ApplicationRecord
-  IMAGE_CPN_FORMAT = 'https://pets.neopets.com/cpn/%s/1/1.png';
-  IMAGE_CP_LOCATION_REGEX = %r{^/cp/(.+?)/[0-9]+/[0-9]+\.png$};
-  IMAGE_CPN_ACCEPTABLE_NAME = /^[a-z0-9_]+$/
-
   # NOTE: While a pet type does always functionally belong to a color and
   # species, sometimes we don't have that record yet, in which case color_id
   # or species_id will refer to a nonexistant record, and the site should
@@ -13,8 +9,6 @@ class PetType < ApplicationRecord
   has_many :pet_states
   has_many :pets
 
-  attr_writer :origin_pet
-
   BasicHashes = YAML::load_file(Rails.root.join('config', 'basic_type_hashes.yml'))
   
   scope :matching_name, ->(color_name, species_name) {
@@ -23,8 +17,6 @@ class PetType < ApplicationRecord
     where(color_id: color.id, species_id: species.id)
   }
 
-  before_save :load_image_hash
-
   def self.special_color_or_basic(special_color)
     color_ids = special_color ? [special_color.id] : Color.basic.select([:id]).map(&:id)
     where(color_id: color_ids)
@@ -41,11 +33,6 @@ class PetType < ApplicationRecord
     random_pet_types
   end
 
-  def self.get_hash_from_cp_path(path)
-    match = path.match(IMAGE_CP_LOCATION_REGEX)
-    match ? match[1] : nil
-  end
-
   def as_json(options={})
     super({
       only: [:id],
@@ -112,38 +99,6 @@ class PetType < ApplicationRecord
     pet_state
   end
 
-  def load_image_hash
-    if @origin_pet && @origin_pet.name =~ IMAGE_CPN_ACCEPTABLE_NAME
-      cpn_uri = URI.parse sprintf(IMAGE_CPN_FORMAT, CGI.escape(@origin_pet.name))
-      begin
-        res = Net::HTTP.get_response(cpn_uri, {
-          'User-Agent' => Rails.configuration.user_agent_for_neopets
-        })
-      rescue Exception => e
-        raise DownloadError, e.message
-      end
-      unless res.is_a? Net::HTTPFound
-        begin
-          res.error!
-        rescue Exception => e
-          Rails.logger.warn "Error loading CPN image at #{cpn_uri}: #{e.message}"
-          return
-        else
-          Rails.logger.warn "Error loading CPN image at #{cpn_uri}. Response: #{res.inspect}"
-          return
-        end
-      end
-      new_url = res['location']
-      new_image_hash = PetType.get_hash_from_cp_path(new_url)
-      if new_image_hash
-        self.image_hash = new_image_hash
-        Rails.logger.info "Successfully loaded #{cpn_uri}, saved image hash #{new_image_hash}"
-      else
-        Rails.logger.warn "CPN image pointed to #{new_url}, which does not match CP image format"
-      end
-    end
-  end
-
   def canonical_pet_state
     # For consistency (randomness is always scary!), we use the PetType ID to
     # determine which gender to prefer. That way, it'll be stable, but we'll