From 57dcc88b2775821fe638e391b1f841a41eef5fb6 Mon Sep 17 00:00:00 2001 From: Emi Matchu Date: Sat, 6 Apr 2024 02:25:22 -0700 Subject: [PATCH] Refactor pet image hash loading into the Pet model, not PetType Just a small thing, I guess when I was a kid I did a weird thing where I attached `origin_pet` to `PetType`, then upon saving `PetType` I loaded the image hash for the pet to save as the pet type's new image hash. I guess this does have the nice property of not bothering to load that stuff until we need it? But whatever, I'm moving this into `Pet` both to simplify the relationship between the models, and to prepare for another potential refactor: using `PetService.getPet` for this instead! --- app/models/pet.rb | 46 +++++++++++++++++++++++++++++++++++++++++- app/models/pet_type.rb | 45 ----------------------------------------- 2 files changed, 45 insertions(+), 46 deletions(-) diff --git a/app/models/pet.rb b/app/models/pet.rb index ca4637a7..d852bf2a 100644 --- a/app/models/pet.rb +++ b/app/models/pet.rb @@ -43,7 +43,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 @@ -158,6 +160,48 @@ class Pet < ApplicationRecord HashWithIndifferentAccess.new(envelope.messages[0].data.body) 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.) + 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-Za-z0-9_]+$/ + def self.fetch_image_hash(name) + return nil unless name.match?(IMAGE_CPN_ACCEPTABLE_NAME) + + cpn_uri = URI.parse sprintf(IMAGE_CPN_FORMAT, CGI.escape(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 = get_hash_from_cp_path(new_url) + if new_image_hash + Rails.logger.info "Successfully loaded #{cpn_uri}, with image hash #{new_image_hash}" + new_image_hash + else + Rails.logger.warn "CPN image pointed to #{new_url}, which does not match CP image format" + end + end + + def self.get_hash_from_cp_path(path) + match = path.match(IMAGE_CP_LOCATION_REGEX) + match ? match[1] : nil + end + class PetNotFound < RuntimeError;end class DownloadError < RuntimeError;end class UnexpectedDataFormat < RuntimeError;end diff --git a/app/models/pet_type.rb b/app/models/pet_type.rb index 5138d267..b34d2910 100644 --- a/app/models/pet_type.rb +++ b/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