Strict Types, BigDecimal everywhere (#7)

.github/workflows/ci.yml

@@ -3,6 +3,7 @@ on: [push]
 jobs:
   test:
     strategy:
+      fail-fast: false
       matrix:
         ruby:
           - 3.0
@@ -42,6 +43,7 @@ jobs:
           bundle install -j $(getconf _NPROCESSORS_ONLN) --retry 3
 
       - name: Run RSspec
+        continue-on-error: true
         run: bundle exec rspec --format documentation
 
       - name: Run Sorbet Typechecks

lib/money/distributed/fetcher/base.rb

@@ -1,36 +1,53 @@
-# typed: true
+# typed: strict
 # frozen_string_literal: true
 
 class Money
   module Distributed
     module Fetcher
       # Base class for rates fetchers
       module Base
+        extend T::Sig
+        extend T::Generic
+
+        abstract!
+
+        sig { params(bank: T.nilable(Money::Bank::VariableExchange)).void }
         def initialize(bank = nil)
-          @bank = bank || Money.default_bank
+          @bank = T.let(bank || Money.default_bank, Money::Bank::VariableExchange)
         end
 
+        sig { void }
         def fetch
           rates = exchange_rates
           currencies = rates.keys
 
-          currencies.each { add_rate(_1, _1, 1) }
+          # rate from currency to itself is always 1
+          currencies.each { add_rate(_1, _1, BigDecimal('1')) }
 
           currencies.combination(2).each do |curr_1, curr_2|
-            rate = rates[curr_2] / rates[curr_1]
+            curr_1 = T.cast(curr_1, String)
+            curr_2 = T.cast(curr_2, String)
+            rate = rates.fetch(curr_2) / rates.fetch(curr_1)
             add_rate(curr_1, curr_2, rate)
           end
         end
 
+        sig do
+          params(
+            from_iso: String,
+            to_iso: String,
+            rate: BigDecimal,
+          ).void
+        end
         private def add_rate(from_iso, to_iso, rate)
-          @bank.add_rate(from_iso, to_iso, rate.round(4))
+          @bank.add_rate(from_iso, to_iso, rate)
           return if from_iso == to_iso
 
-          @bank.add_rate(to_iso, from_iso, (1 / rate).round(4))
+          @bank.add_rate(to_iso, from_iso, 1 / rate)
         end
 
+        sig { abstract.returns(T::Hash[String, BigDecimal]) }
         private def exchange_rates
-          raise NotImplementedError
         end
       end
     end

lib/money/distributed/fetcher/file.rb

@@ -24,7 +24,7 @@ def initialize(file_path, bank = nil)
           @file_path = file_path
         end
 
-        sig { returns(T::Hash[String, BigDecimal]) }
+        sig { override.returns(T::Hash[String, BigDecimal]) }
         private def exchange_rates
           ::File.read(@file_path).split("\n").each_with_object({}) do |line, h|
             code_rate = line.split

lib/money/distributed/redis.rb

@@ -1,4 +1,4 @@
-# typed: true
+# typed: strict
 # frozen_string_literal: true
 
 class Money
@@ -9,16 +9,16 @@ class Redis
 
       sig do
         params(
-          redis: T.any(::Redis, ConnectionPool, Hash, Proc),
+          redis: T.any(::Redis, ConnectionPool, T::Hash[T.untyped, T.untyped], Proc),
         ).void
       end
       def initialize(redis)
-        @redis_proc = build_redis_proc(redis)
+        @redis_proc = T.let(build_redis_proc(redis), Proc)
       end
 
       sig do
         params(
-          block: T.proc.returns(T.untyped),
+          block: T.proc.params(redis_or_similar: T.untyped).returns(T.untyped),
         ).returns(T.untyped)
       end
       def exec(&block)
@@ -27,7 +27,7 @@ def exec(&block)
 
       sig do
         params(
-          redis: T.any(::Redis, ConnectionPool, Hash, Proc),
+          redis: T.any(::Redis, ConnectionPool, T::Hash[T.untyped, T.untyped], Proc),
         ).returns(Proc)
       end
       private def build_redis_proc(redis)

lib/money/distributed/storage.rb

@@ -5,26 +5,47 @@ class Money
   module Distributed
     # Storage for `Money::Bank::VariableExchange` that stores rates in Redis
     class Storage
+      extend T::Sig
+
       INDEX_KEY_SEPARATOR = '_TO_'
       REDIS_KEY = 'money_rates'
 
+      sig do
+        params(
+          redis: T.any(::Redis, ConnectionPool, T::Hash[T.untyped, T.untyped], Proc),
+          cache_ttl: T.nilable(Integer),
+        ).void
+      end
       def initialize(redis, cache_ttl = nil)
-        @redis = Money::Distributed::Redis.new(redis)
+        @redis = T.let(Money::Distributed::Redis.new(redis), Money::Distributed::Redis)
 
-        @cache = {}
+        @cache = T.let({}, T::Hash[String, BigDecimal])
         @cache_ttl = cache_ttl
-        @cache_updated_at = nil
+        @cache_updated_at = T.let(nil, T.nilable(Time))
 
         @lock = Concurrent::ReentrantReadWriteLock.new
       end
 
+      sig do
+        params(
+          iso_from: String,
+          iso_to: String,
+          rate: BigDecimal,
+        ).void
+      end
       def add_rate(iso_from, iso_to, rate)
         @redis.exec do |r|
-          r.hset(REDIS_KEY, key_for(iso_from, iso_to), rate)
+          r.hset(REDIS_KEY, key_for(iso_from, iso_to), rate.to_s)
         end
         clear_cache
       end
 
+      sig do
+        params(
+          iso_from: String,
+          iso_to: String,
+        ).returns(T.nilable(BigDecimal))
+      end
       def get_rate(iso_from, iso_to)
         cached_rates[key_for(iso_from, iso_to)]
       end
@@ -45,10 +66,17 @@ def transaction
         yield
       end
 
+      sig { returns(T::Array[T.untyped]) }
       def marshal_dump
         [self.class, @cache_ttl]
       end
 
+      sig do
+        params(
+          iso_from: String,
+          iso_to: String,
+        ).returns(String)
+      end
       private def key_for(iso_from, iso_to)
         [iso_from, iso_to].join(INDEX_KEY_SEPARATOR).upcase
       end
@@ -60,19 +88,22 @@ def cached_rates
         end
       end
 
+      sig { returns(T::Boolean) }
       private def cache_outdated?
         return false unless @cache_ttl
 
         @cache_updated_at.nil? ||
         @cache_updated_at < Time.now - @cache_ttl
       end
 
+      sig { void }
       def clear_cache
         Money::Distributed::ReadWriteLock.write(@lock) do
           @cache.clear
         end
       end
 
+      sig { void }
       private def retrieve_rates
         updated_cache = {}
 

spec/money/distributed/fetcher/file_spec.rb

@@ -7,19 +7,28 @@
   subject { described_class.new(file_path, bank) }
 
   let(:file_path) { File.expand_path('../../../fixtures/rates.txt', __dir__) }
-  let(:bank) { double(add_rate: true) }
+  let(:bank) { Money::Bank::VariableExchange.new }
+
+  before do
+    allow(bank).to receive(:add_rate).and_call_original
+  end
 
   it 'fetches rates from the file' do
     subject.fetch
 
-    expect(bank).to have_received(:add_rate).with('USD', 'USD', 1.0)
-    expect(bank).to have_received(:add_rate).with('AUD', 'AUD', 1.0)
-    expect(bank).to have_received(:add_rate).with('EUR', 'EUR', 1.0)
-    expect(bank).to have_received(:add_rate).with('USD', 'AUD', 1.3209)
-    expect(bank).to have_received(:add_rate).with('AUD', 'USD', 0.7571)
-    expect(bank).to have_received(:add_rate).with('USD', 'EUR', 0.9076)
-    expect(bank).to have_received(:add_rate).with('EUR', 'USD', 1.1018)
-    expect(bank).to have_received(:add_rate).with('AUD', 'EUR', 0.6871)
-    expect(bank).to have_received(:add_rate).with('EUR', 'AUD', 1.4554)
+    # trivial
+    expect(bank).to have_received(:add_rate).with('USD', 'USD', BigDecimal('1.0'))
+    expect(bank).to have_received(:add_rate).with('AUD', 'AUD', BigDecimal('1.0'))
+    expect(bank).to have_received(:add_rate).with('EUR', 'EUR', BigDecimal('1.0'))
+
+    # non-trivial combinations
+    expect(bank).to have_received(:add_rate).with('USD', 'AUD', BigDecimal('1.320898'))
+    expect(bank).to have_received(:add_rate).with('AUD', 'USD', 1 / BigDecimal('1.320898'))
+    expect(bank).to have_received(:add_rate).with('USD', 'EUR', BigDecimal('0.907601'))
+    expect(bank).to have_received(:add_rate).with('EUR', 'USD', 1 / BigDecimal('0.907601'))
+    expect(bank).to have_received(:add_rate).with('AUD', 'EUR', BigDecimal('0.907601') / BigDecimal('1.320898'))
+
+    # @NOTE: due to precision, 1/(a/b) != b/a
+    expect(bank).to have_received(:add_rate).with('EUR', 'AUD', 1 / (BigDecimal('0.907601') / BigDecimal('1.320898')))
   end
 end

spec/money/distributed/storage_spec.rb

@@ -10,8 +10,8 @@
   let(:ttl) { 3600 }
 
   it 'stores rates in redis' do
-    subject.add_rate 'USD', 'RUB', 60.123
-    expect(redis.hget(described_class::REDIS_KEY, 'USD_TO_RUB')).to eq '60.123'
+    subject.add_rate 'USD', 'RUB', BigDecimal('60.123')
+    expect(redis.hget(described_class::REDIS_KEY, 'USD_TO_RUB')).to eq('0.60123e2')
   end
 
   it 'gets rates from redis' do
@@ -40,14 +40,14 @@
     let(:storage) { subject }
     let(:rates) do
       {
-        'USD_TO_EUR' => '0.85',
-        'EUR_TO_USD' => '1.18',
+        'USD_TO_EUR' => BigDecimal('0.85'),
+        'EUR_TO_USD' => BigDecimal('1.18'),
       }
     end
 
     before do
       rates.each do |key, rate|
-        redis.hset(described_class::REDIS_KEY, key, rate)
+        redis.hset(described_class::REDIS_KEY, key, rate.to_s)
       end
     end
 
@@ -58,7 +58,7 @@
       threads = []
       10.times do
         threads << Thread.new do
-          1000.times do
+          100.times do
             key, rate = rates.to_a.sample
             iso_from, iso_to = key.split(described_class::INDEX_KEY_SEPARATOR)
             storage.add_rate(iso_from, iso_to, rate)

spec/spec_helper.rb

@@ -1,4 +1,4 @@
-# typed: false
+# typed: true
 # frozen_string_literal: true
 
 require 'rspec'