From 7b983fecb85aaec6403bad588b6c5e2bfb980264 Mon Sep 17 00:00:00 2001 From: Cameron Roudebush Date: Fri, 16 May 2025 15:32:54 -0400 Subject: [PATCH] feat(simplefin): Add rate limit - You can now specify the rate limit so Maybe can't over do it with the backend calls - Fix various file placements from the updates - Adding an actual configuration class for SimpleFIN --- .env.example | 1 + app/controllers/simple_fin_controller.rb | 6 +- app/models/provider/registry.rb | 3 +- app/models/provider/simple_fin.rb | 42 +++++- app/models/simple_fin_account.rb | 9 +- app/models/simple_fin_item.rb | 140 ++---------------- app/models/simple_fin_rate_limit.rb | 4 + .../_simple_fin_item.html.erb} | 0 config/initializers/simple_fin.rb | 23 ++- config/locales/simple_fin/en.yml | 2 + .../20250509134646_simple_fin_integration.rb | 11 +- db/schema.rb | 10 +- sample.simple.fin.json | 3 +- 13 files changed, 102 insertions(+), 152 deletions(-) create mode 100644 app/models/simple_fin_rate_limit.rb rename app/views/{simple_fin_connections/_simple_fin_connection.html.erb => simple_fin_items/_simple_fin_item.html.erb} (100%) diff --git a/.env.example b/.env.example index 409e23de..3db72473 100644 --- a/.env.example +++ b/.env.example @@ -52,6 +52,7 @@ APP_DOMAIN= # Allows configuration of SimpleFIN for account linking: https://www.simplefin.org/ # You'll want to follow the steps here for getting an AccessURL https://beta-bridge.simplefin.org/info/developers SIMPLE_FIN_ACCESS_URL= +SIMPLE_FIN_RATE_LIMIT=24 # How many queries we may perform to SimpleFIN a day # Disable enforcing SSL connections # DISABLE_SSL=true diff --git a/app/controllers/simple_fin_controller.rb b/app/controllers/simple_fin_controller.rb index b09ea810..9f6225ed 100644 --- a/app/controllers/simple_fin_controller.rb +++ b/app/controllers/simple_fin_controller.rb @@ -12,7 +12,7 @@ class SimpleFinController < ApplicationController @simple_fin_accounts = @simple_fin_accounts.filter { |acc| !account_exists(acc) } rescue StandardError => e Rails.logger.error "SimpleFIN: Failed to fetch accounts - #{e.message}" - redirect_to new_account_path, alert: t(".fetch_failed") + redirect_to root_path, alert: t(".fetch_failed") end ## @@ -26,7 +26,7 @@ class SimpleFinController < ApplicationController def sync @simple_fin_item = Current.family.simple_fin_items.find(params[:id]) unless @simple_fin_item.syncing? - @simple_fin_item.sync + @simple_fin_item.sync_later end respond_to do |format| @@ -58,8 +58,6 @@ class SimpleFinController < ApplicationController sfc.institution_name = org_details["name"] sfc.institution_url = org_details["url"] sfc.institution_domain = org_details["domain"] - # TODO: Fix - sfc.last_sync_count_reset_at = Time.current # Mark as synced upon creation end sf_accounts_for_institution.each do |acc_detail| diff --git a/app/models/provider/registry.rb b/app/models/provider/registry.rb index 1237e193..408f6d87 100644 --- a/app/models/provider/registry.rb +++ b/app/models/provider/registry.rb @@ -13,6 +13,7 @@ class Provider::Registry end def get_provider(name) + puts "NAME: #{name}" send(name) rescue NoMethodError raise Error.new("Provider '#{name}' not found in registry") @@ -65,7 +66,7 @@ class Provider::Registry end def simple_fin - config = Rails.application.config.simple_fin + config = Provider::SimpleFin.provider_config return nil unless config.present? diff --git a/app/models/provider/simple_fin.rb b/app/models/provider/simple_fin.rb index 5a43e82b..2c6070af 100644 --- a/app/models/provider/simple_fin.rb +++ b/app/models/provider/simple_fin.rb @@ -1,10 +1,19 @@ class Provider::SimpleFin attr_reader :client, :region + # Error subclasses + Error = Class.new(Provider::Error) + RateLimitExceededError = Class.new(Error) + # SimpleFIN only supports these account types MAYBE_SUPPORTED_SIMPLE_FIN_PRODUCTS = %w[Depository Investment Loan CreditCard].freeze - # TODO - MAX_HISTORY_DAYS = 1 + + class << self + # Helper class method to access the SimpleFin specific configuration + def provider_config + Rails.application.config.simple_fin + end + end def initialize(config, region: :us) @region = region @@ -22,7 +31,7 @@ class Provider::SimpleFin return false unless is_supported_account_type # Verify it is configured - config = Rails.application.config.simple_fin + config = Provider::SimpleFin.provider_config return false unless config.present? # Make sure this API version is supported @@ -37,8 +46,8 @@ class Provider::SimpleFin # @param [Boolean] include_creds Controls if credentials should be included or if this request should be anonymous. Default true. def send_request_to_sf(path, include_creds = true) # Grab access URL from the env - config = Rails.application.config.simple_fin - access_url = config["ACCESS_URL"] + config = Provider::SimpleFin.provider_config + access_url = config.access_url # Add the access URL to the path uri = URI.parse(access_url + path) # Setup the request @@ -77,6 +86,7 @@ class Provider::SimpleFin # @param [int?] trans_end_date A linux epoch of the end date to get transactions between. # @param [Boolean] trans_pending If we should include pending transactions. Default is true. def get_available_accounts(accountable_type, trans_start_date = nil, trans_end_date = nil, trans_pending = true) + check_rate_limit endpoint = "/accounts?pending=#{trans_pending}" # Add any parameters we care about @@ -133,10 +143,30 @@ class Provider::SimpleFin end end + ## + # Increments the call count for tracking rate limiting of SimpleFIN. + # + # @raises [RateLimitExceededError] if the daily API call limit has been reached. + def check_rate_limit + today = Date.current + # Find or initialize the rate limit record for the family for today + rate_limit_record = SimpleFinRateLimit.find_or_initialize_by(date: today) + + # Determine the actual limit: from config + limit = Provider::SimpleFin.provider_config.rate_limit + + if rate_limit_record.call_count >= limit + raise RateLimitExceededError, "SimpleFIN API daily rate limit exceeded. Limit: #{limit} calls." + end + + # Increment the call count for today. This also saves the record if new or updates if existing. + rate_limit_record.update!(call_count: rate_limit_record.call_count + 1) + end + # Returns if this is a supported API of SimpleFIN by the access url in the config. def is_supported_api # Make sure the config is loaded since this is called early - config = Rails.application.config.simple_fin + config = Provider::SimpleFin.provider_config return false unless config.present? get_api_versions().include?("1.0") diff --git a/app/models/simple_fin_account.rb b/app/models/simple_fin_account.rb index 43f59fc9..5cf6da21 100644 --- a/app/models/simple_fin_account.rb +++ b/app/models/simple_fin_account.rb @@ -21,6 +21,10 @@ class SimpleFinAccount < ApplicationRecord class << self def find_or_create_from_simple_fin_data!(sf_account_data, sfc) sfc.simple_fin_accounts.find_or_create_by!(external_id: sf_account_data["id"]) do |sfa| + sfa.current_balance = sf_account_data["balance"].to_d + sfa.available_balance = sf_account_data["balance"].to_d + sfa.currency = sf_account_data["currency"] + new_account = sfc.family.accounts.new( name: sf_account_data["name"], balance: sf_account_data["balance"].to_d, @@ -64,16 +68,15 @@ class SimpleFinAccount < ApplicationRecord end end + ## + # Syncs all account data for the given sf_account_data parameter # sf_account_data is a hash from Provider::SimpleFin#get_available_accounts def sync_account_data!(sf_account_data) - # Ensure accountable_attributes has the ID for updates - # 'account' here refers to self.account (the associated Account instance) accountable_attributes = { id: self.account.accountable_id } self.update!( current_balance: sf_account_data["balance"].to_d, available_balance: sf_account_data["available-balance"]&.to_d, currency: sf_account_data["currency"], - # simple_fin_errors: sf_account_data["errors"] || [], account_attributes: { id: self.account.id, balance: sf_account_data["balance"].to_d, diff --git a/app/models/simple_fin_item.rb b/app/models/simple_fin_item.rb index a351fa52..985f98b0 100644 --- a/app/models/simple_fin_item.rb +++ b/app/models/simple_fin_item.rb @@ -16,86 +16,16 @@ class SimpleFinItem < ApplicationRecord scope :ordered, -> { order(created_at: :desc) } scope :needs_update, -> { where(status: :requires_update) } - class << self - # # `provided_access_url` is the full URL from SimpleFIN (https://user:pass@beta-bridge.simplefin.org/simplefin) - # # `connection_name` can be user-provided or derived. - # def create_and_sync_from_access_url(provided_access_url, connection_name, family_obj) - # # Basic validation of the URL format - # uri = URI.parse(provided_access_url) - # raise ArgumentError, "Invalid SimpleFIN Access URL: Missing credentials" unless uri.user && uri.password - # raise ArgumentError, "Invalid SimpleFIN Access URL: Must be HTTPS" unless uri.scheme == "https" - - # # Create the connection object first - # connection = family_obj.simple_fin_connections.create!( - # name: connection_name, - # access_url: provided_access_url, - # status: :good # Assume good initially - # ) - - # # Perform an initial sync to populate institution details and accounts - # connection.sync_later - # connection - # end - end - ## # Syncs the simple_fin_item given and all other simple_fin accounts available (reduces calls to the API) def sync_data(sync, start_date: nil) - # TODO: Rate limit - # now = Time.current - # # Rate Limiting Check - # # We use a transaction here to ensure that checking the count, resetting it if needed, - # # and incrementing it are atomic. - # ActiveRecord::Base.transaction do - # # Reload self to ensure we have the latest values from the DB, - # # especially if this method could be called concurrently for the same item. - # self.reload - - # if self.last_sync_count_reset_at.nil? || self.last_sync_count_reset_at.to_date < now.to_date - # # If it's a new day (or first sync ever for rate limiting), reset the count and the reset timestamp. - # self.update_columns(syncs_today_count: 0, last_sync_count_reset_at: now) - # self.reload # Reload again to get the just-updated values for the check below. - # end - - # if self.syncs_today_count >= 24 - # msg = "SimpleFinItem ID #{self.id}: Sync limit of 24 per day reached. Count: #{self.syncs_today_count}." - # Rails.logger.warn(msg) - # sync.fail!(StandardError.new(msg)) # Record failure in the Sync object - # raise StandardError, msg # Raise to stop execution and ensure SyncJob handles it as a failure - # end - - # # If not rate-limited, increment the count for this sync attempt. - # self.increment!(:syncs_today_count) - # end - - - # unless access_url.present? - # # This is a configuration error for the connection itself. - # msg = "SimpleFinConnection: Sync cannot proceed for connection ID #{id}: Missing access_url." - # Rails.logger.error(msg) - # update!(status: :requires_update) # Mark connection as needing attention - # # Raise an error to ensure the SyncJob records this failure. - # # Sync#perform will catch this and call sync.fail! - # raise StandardError, msg - # end - - # TODO: Populate this - # update!(last_synced_at: Time.current, status: :requires_update) - - Rails.logger.info("SimpleFinConnection: Starting sync for connection ID #{id}") + Rails.logger.info("SimpleFINItem: Starting sync for all SimpleFIN accounts") begin # Fetch all accounts for this specific connection from SimpleFIN. sf_accounts_data = provider.get_available_accounts(nil) - - - # Keep track of external IDs reported by the provider in this sync. - # This can be used later to identify accounts that might have been removed on the SimpleFIN side. - current_provider_external_ids = [] - + # Iterate over every account and attempt to apply transactions where possible sf_accounts_data.each do |sf_account_data| - current_provider_external_ids << sf_account_data["id"] - begin # Find or create the SimpleFinAccount record. sfa = SimpleFinAccount.find_by(external_id: sf_account_data["id"]) @@ -104,69 +34,27 @@ class SimpleFinItem < ApplicationRecord end if sfa != nil - # Sync the detailed data for this account (e.g., balance, and potentially transactions). - # This method is expected to be on the SimpleFinAccount model. - sfa.sync_account_data!(sf_account_data) + begin + # Sync the detailed data for this account + sfa.sync_account_data!(sf_account_data) + rescue StandardError => e + Rails.logger.error("SimpleFINItem: Sync failed for account #{sf_account_data["id"]}: #{e.message}") + sfa.simple_fin_item.update(id: sf_account_data["id"], status: :requires_update) # We had problems so make sure this account knows + end end end - # Optional: You could add logic here to handle accounts that exist in your DB for this - # SimpleFinConnection but were NOT reported by the provider in `sf_accounts_data`. - # These could be marked as closed, archived, etc. For example: - # simple_fin_accounts.where.not(external_id: current_provider_external_ids).find_each(&:archive!) + Rails.logger.info("SimpleFINItem: Sync completed for all accounts") - # update!(status: :good) # Mark connection as successfully synced. - Rails.logger.info("SimpleFinConnection: Sync completed for connection ID #{id}") - - # rescue Provider::SimpleFin::AuthenticationError => e # Catch specific auth errors if your provider defines them. - # Rails.logger.error("SimpleFinConnection: Authentication failed for connection ID #{id}: #{e.message}") - # update!(status: :requires_update) # Mark the connection so the user knows to update credentials. - # raise e # Re-raise so Sync#perform can record the failure. + rescue Provider::SimpleFin::RateLimitExceededError =>e + Rails.logger.error("SimpleFINItem: Sync failed: #{e.message}") + raise StandardError, "SimpleFIN Rate Limit: #{e.message}" # Re-raise as a generic StandardError rescue StandardError => e - Rails.logger.error("SimpleFinConnection: Sync failed for connection ID #{id}: #{e.message}") - update!(status: :requires_update) + Rails.logger.error("SimpleFINItem: Sync failed: #{e.message}") raise e # Re-raise so Sync#perform can record the failure. end end - # def sync_data(sync, start_date: nil) - # update!(last_synced_at: Time.current) - # Rails.logger.info("SimpleFinConnection: Starting sync for connection ID #{id}") - - # # begin - # # # Fetch initial info if not present (like institution details) - # # if institution_id.blank? || api_versions_supported.blank? - # # info_data = provider.get_api_versions_and_org_details_from_accounts - # # update!( - # # institution_id: info_data[:org_id], - # # institution_name: info_data[:org_name], - # # institution_url: info_data[:org_url], - # # institution_domain: info_data[:org_domain], - # # api_versions_supported: info_data[:versions] - # # ) - # # end - - # # sf_accounts_data = provider.get_available_accounts(nil) # Pass nil to get all types - - # # sf_accounts_data.each do |sf_account_data| - # # accountable_klass_name = Provider::SimpleFin::ACCOUNTABLE_TYPE_MAPPING.find { |key, _val| sf_account_data["type"]&.downcase == key.downcase }&.last - # # accountable_klass_name ||= (sf_account_data["balance"].to_d >= 0 ? Depository : CreditCard) # Basic fallback - # # accountable_klass = accountable_klass_name - - # # sfa = simple_fin_accounts.find_or_create_from_simple_fin_data!(sf_account_data, self, accountable_klass) - # # sfa.sync_account_data!(sf_account_data) - # # end - - # update!(status: :good) if requires_update? - # Rails.logger.info("SimpleFinConnection: Sync completed for connection ID #{id}") - - # # rescue StandardError => e - # # Rails.logger.error("SimpleFinConnection: Sync failed for connection ID #{id}: #{e.message}") - # # update!(status: :requires_update) - # # raise e - # # end - # end - def provider @provider ||= Provider::Registry.get_provider(:simple_fin) end diff --git a/app/models/simple_fin_rate_limit.rb b/app/models/simple_fin_rate_limit.rb new file mode 100644 index 00000000..007ae310 --- /dev/null +++ b/app/models/simple_fin_rate_limit.rb @@ -0,0 +1,4 @@ +class SimpleFinRateLimit < ApplicationRecord + validates :date, presence: true + validates :call_count, numericality: { greater_than_or_equal_to: 0 } +end diff --git a/app/views/simple_fin_connections/_simple_fin_connection.html.erb b/app/views/simple_fin_items/_simple_fin_item.html.erb similarity index 100% rename from app/views/simple_fin_connections/_simple_fin_connection.html.erb rename to app/views/simple_fin_items/_simple_fin_item.html.erb diff --git a/config/initializers/simple_fin.rb b/config/initializers/simple_fin.rb index 64746feb..6d34aa98 100644 --- a/config/initializers/simple_fin.rb +++ b/config/initializers/simple_fin.rb @@ -1,13 +1,24 @@ -require "ostruct" + +class SimpleFinConfig + attr_accessor :access_url, :rate_limit, :max_history_days + + def initialize + @rate_limit = 24 + # TODO + @max_history_days = 1 # Was a constant, now part of config + end +end Rails.application.configure do config.simple_fin = nil + # We use the access URL as the base of if we want to run SimpleFIN or not if ENV["SIMPLE_FIN_ACCESS_URL"].present? - config.simple_fin = OpenStruct.new() - config.simple_fin["ACCESS_URL"] = ENV["SIMPLE_FIN_ACCESS_URL"] - config.simple_fin["UPDATE_CRON"] = ENV["SIMPLE_FIN_UPDATE_CRON"] - # Fallback - config.simple_fin["UPDATE_CRON"] = "0 6 * * *" if config.simple_fin["UPDATE_CRON"].nil? + + sf_config = SimpleFinConfig.new + sf_config.access_url = ENV["SIMPLE_FIN_ACCESS_URL"] + sf_config.rate_limit = ENV["SIMPLE_FIN_RATE_LIMIT"].to_i if ENV["SIMPLE_FIN_RATE_LIMIT"].present? + + config.simple_fin = sf_config end end diff --git a/config/locales/simple_fin/en.yml b/config/locales/simple_fin/en.yml index 3d2f37b4..e4a2a76b 100644 --- a/config/locales/simple_fin/en.yml +++ b/config/locales/simple_fin/en.yml @@ -3,6 +3,8 @@ en: simple_fin: form: add_accounts: "Add Selected Accounts" + new: + fetch_failed: "Failed to fetch accounts" create: accounts_created_success: "Accounts Successfully Created" fetch_failed: "Failed to fetch accounts" diff --git a/db/migrate/20250509134646_simple_fin_integration.rb b/db/migrate/20250509134646_simple_fin_integration.rb index d6545b03..2b77338c 100644 --- a/db/migrate/20250509134646_simple_fin_integration.rb +++ b/db/migrate/20250509134646_simple_fin_integration.rb @@ -9,9 +9,6 @@ class SimpleFinIntegration < ActiveRecord::Migration[7.2] t.string :status, default: "good" # e.g., good, requires_update t.string :institution_errors, array: true, default: [] t.boolean :scheduled_for_deletion, default: false - # Columns for rate limiting - t.integer :syncs_today_count, default: 0, null: false - t.datetime :last_sync_count_reset_at t.timestamps end @@ -37,5 +34,13 @@ class SimpleFinIntegration < ActiveRecord::Migration[7.2] add_column :holdings, :simple_fin_holding_id, :string add_index :holdings, :simple_fin_holding_id, unique: true, where: "simple_fin_holding_id IS NOT NULL" add_column :holdings, :source, :string + + create_table :simple_fin_rate_limits, id: :uuid do |t| + t.date :date, null: false + t.integer :call_count, null: false, default: 0 + + t.timestamps + end + add_index :simple_fin_rate_limits, [ :date ], unique: true, name: 'index_sfrl_on_date' end end diff --git a/db/schema.rb b/db/schema.rb index 15e40eb7..2914efb7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -580,13 +580,19 @@ ActiveRecord::Schema[7.2].define(version: 2025_05_09_134646) do t.string "status", default: "good" t.string "institution_errors", default: [], array: true t.boolean "scheduled_for_deletion", default: false - t.integer "syncs_today_count", default: 0, null: false - t.datetime "last_sync_count_reset_at" t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["family_id"], name: "index_simple_fin_items_on_family_id" end + create_table "simple_fin_rate_limits", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.date "date", null: false + t.integer "call_count", default: 0, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["date"], name: "index_sfrl_on_date", unique: true + end + create_table "stock_exchanges", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.string "name", null: false t.string "acronym" diff --git a/sample.simple.fin.json b/sample.simple.fin.json index 9162fdb7..95e4d56d 100644 --- a/sample.simple.fin.json +++ b/sample.simple.fin.json @@ -1,6 +1,7 @@ { "errors": [ - "Connection to Fidelity Investments may need attention" + "Connection to Fidelity Investments may need attention", + "Connection to Wright-Patt Credit Union may need attention" ], "accounts": [ {