app/models/item.rb

@@ -18,6 +18,9 @@ class Item < ApplicationRecord
   SPECIAL_COLOR_DESCRIPTION_REGEX =
     /This item is only wearable by [a-zA-Z]+ painted ([a-zA-Z]+)\.|WARNING: This [a-zA-Z]+ can be worn by ([a-zA-Z]+) [a-zA-Z]+ ONLY!|If your Neopet is not painted ([a-zA-Z]+), it will not be able to wear this item\./
 
+  cattr_reader :per_page
+  @@per_page = 30
+
   scope :newest, -> {
     order(arel_table[:created_at].desc) if arel_table[:created_at]
   }

app/models/swf_asset.rb

@@ -51,65 +51,18 @@ class SwfAsset < ApplicationRecord
   end
 
   def manifest
-    @manifest ||= load_manifest
+    raise "manifest_url is blank" if manifest_url.blank?
+    @manifest ||= NeopetsMediaArchive.load_json(manifest_url)
   end
 
-  def preload_manifest(save_changes: true)
-    load_manifest(return_content: false, save_changes:)
-  end
-
-  def load_manifest(return_content: true, save_changes: true)
-    return nil if manifest_url.blank?
-
-    # If we recently tried loading the manifest and got a 4xx HTTP status code
-    # (e.g. a 404, there's a surprising amount of these!), don't try again. But
-    # after enough time passes, if this is called again, we will!
-    #
-    # (We always retry 5xx errors, on the assumption that they probably
-    # represent intermittent failures, whereas 4xx errors are not likely to
-    # succeed just by retrying.)
-    if manifest_loaded_at.present?
-      last_try_was_4xx =(400...500).include?(manifest_status_code)
-      last_try_was_recent = (Time.now - manifest_loaded_at) <= 1.day
-      if last_try_was_4xx and last_try_was_recent
-        Rails.logger.debug "Skipping loading manifest for asset #{id}: " +
-          "last try was status #{manifest_status_code} at #{manifest_loaded_at}"
-        return nil
-      end
-    end
-
-    begin
-      NeopetsMediaArchive.load_file(manifest_url, return_content:) =>
-        {content:, source:}
-    rescue NeopetsMediaArchive::ResponseNotOK => error
-      Rails.logger.warn "Failed to load manifest for asset #{id}: " +
-        error.message
-      self.manifest_loaded_at = Time.now
-      self.manifest_status_code = error.status
-      save! if save_changes
-      return nil
-    end
-
-    if source == "network" || manifest_loaded_at.blank?
-      self.manifest_loaded_at = Time.now
-      self.manifest_status_code = 200
-      save! if save_changes
-    end
-
-    return nil unless return_content
-
-    begin
-      JSON.parse(content)
-    rescue JSON::ParserError => error
-      Rails.logger.warn "Failed to parse manifest for asset #{id}: " +
-        error.message
-      return nil
-    end
+  def preload_manifest
+    raise "manifest_url is blank" if manifest_url.blank?
+    NeopetsMediaArchive.preload_file(manifest_url)
   end
 
   MANIFEST_BASE_URL = Addressable::URI.parse("https://images.neopets.com")
   def manifest_asset_urls
-    return {} unless manifest.present?
+    return {} if manifest_url.nil?
 
     begin
       # Organize the asset URLs by file extension, convert them from paths to
@@ -135,10 +88,7 @@ class SwfAsset < ApplicationRecord
         # (There's probably only one of each! I'm just going by the same logic
         # we've seen in the JS library case, that later entries are more likely
         # to be correct.)
-        {
-          png: assets_by_ext.fetch(:png, []).last,
-          svg: assets_by_ext.fetch(:svg, []).last,
-        }
+        { png: assets_by_ext[:png].last, svg: assets_by_ext[:svg].last }
       end
     rescue StandardError => error
       Rails.logger.error "Could not read URLs from manifest: #{error.full_message}"
@@ -309,9 +259,7 @@ class SwfAsset < ApplicationRecord
       swf_assets.map do |swf_asset|
         semaphore.async do
           begin
-            # Don't save changes in this big async situation; we'll do it all
-            # in one batch after, to avoid too much database concurrency!
-            swf_asset.preload_manifest(save_changes: false)
+            swf_asset.preload_manifest
           rescue StandardError => error
             Rails.logger.error "Could not preload manifest for asset " + 
               "#{swf_asset.id} (#{swf_asset.manifest_url}): #{error.message}"
@@ -324,10 +272,6 @@ class SwfAsset < ApplicationRecord
     ensure
       barrier.stop # If something goes wrong, clean up all tasks.
     end
-
-    SwfAsset.transaction do
-      swf_assets.each(&:save!)
-    end
   end
 
   before_save do

app/services/neopets_media_archive.rb

@@ -17,6 +17,11 @@ module NeopetsMediaArchive
 
   ROOT_PATH = Pathname.new(Rails.configuration.neopets_media_archive_root)
 
+  # Load the file from the given `images.neopets.com` URI, as JSON.
+  def self.load_json(uri)
+    JSON.parse(load_file(uri))
+  end
+
   # Load the file from the given `images.neopets.com` URI.
   def self.load_file(uri, return_content: true)
     local_path = local_file_path(uri)
@@ -26,7 +31,7 @@ module NeopetsMediaArchive
       begin
         content = File.read(local_path)
         debug "Loaded source file from filesystem: #{local_path}"
-        return {content: content, source: "filesystem"}
+        return content
       rescue Errno::ENOENT
         # If it doesn't exist, that's fine: just move on and download it.
       end
@@ -37,7 +42,7 @@ module NeopetsMediaArchive
       # we're not ready to use yet.)
       if File.exist?(local_path)
         debug "Source file is already loaded, skipping: #{local_path}"
-        return {content: nil, source: "filesystem"}
+        return
       end
     end
 
@@ -48,7 +53,7 @@ module NeopetsMediaArchive
     File.write(local_path, content)
     info "Wrote source file to filesystem: #{local_path}"
     
-    {content: return_content ? content : nil, source: "network"}
+    return_content ? content : nil
   end
 
   # Load the file from the given `images.neopets.com` URI, but don't return its
@@ -73,9 +78,10 @@ module NeopetsMediaArchive
     # requests in parallel!
     Sync do
       response = INTERNET.get(uri)
-      if response.status != 200
-        raise ResponseNotOK.new(response.status),
-          "expected status 200 but got #{response.status} (#{uri})"
+      if response.status == 404
+        raise NotFound, "origin server returned 404: #{uri}"
+      elsif response.status != 200
+        raise "expected status 200 but got #{response.status} (#{uri})"
       end
       response.body.read
     end
@@ -100,13 +106,7 @@ module NeopetsMediaArchive
     ROOT_PATH + path_within_archive(uri)
   end
 
-  class ResponseNotOK < StandardError
-    attr_reader :status
-    def initialize(status)
-      super
-      @status = status
-    end
-  end
+  class NotFound < StandardError; end
 
   private
 

db/migrate/20240225231346_add_manifest_loaded_at_and_manifest_status_code_to_swf_assets.rb

@@ -1,6 +0,0 @@
-class AddManifestLoadedAtAndManifestStatusCodeToSwfAssets < ActiveRecord::Migration[7.1]
-  def change
-    add_column :swf_assets, :manifest_loaded_at, :datetime
-    add_column :swf_assets, :manifest_status_code, :integer
-  end
-end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2024_02_25_231346) do
+ActiveRecord::Schema[7.1].define(version: 2024_02_21_005949) do
   create_table "alt_styles", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t|
     t.integer "species_id", null: false
     t.integer "color_id", null: false
@@ -245,8 +245,6 @@ ActiveRecord::Schema[7.1].define(version: 2024_02_25_231346) do
     t.timestamp "manifest_cached_at"
     t.string "known_glitches", limit: 128, default: ""
     t.string "manifest_url"
-    t.datetime "manifest_loaded_at"
-    t.integer "manifest_status_code"
     t.index ["body_id"], name: "swf_assets_body_id_and_object_id"
     t.index ["type", "remote_id"], name: "swf_assets_type_and_id"
     t.index ["zone_id"], name: "idx_swf_assets_zone_id"