From 9c3a1808b357c2b513aca1b1f451f646fa7de1bd Mon Sep 17 00:00:00 2001 From: Jean Luis Urena <15808412+jlurena@users.noreply.github.com> Date: Fri, 19 Jul 2024 08:30:28 -0400 Subject: [PATCH] [ISSUE-68] Improved Specs and Added Coverage Report (#70) * Fix rails 4.2 failing specs * [ISSUE-66] Improved specs Added coverage report * [ISSUE-68] - Add linter, remove support ruby < 3.0 and rails < 6.1 * Apply Standard Ruby autofixes * [ISSUE-68] Fix github ci for cross-repos * Fix spec * Add/fix another set of spec and logic. Update standard.yml * Update Github CI to run tests on matrix of ruby/rails * Update readme * Added more specs and better logic for cache_prefix_key * Final mixed misc changes * One more update to README * Update .ruby to only read content * Update readme * Remove memoization of cache_key_prefix * Final misc touches * [ISSUE-68] - Removed concurrency option It does more bad than good in real world scenarios. Performance degrades. --------- Co-authored-by: standard-ruby-action[bot] --- .github/workflows/ruby.yml | 87 +- .gitignore | 2 + .standard.yml | 3 + Dockerfile.test | 74 ++ Gemfile | 22 +- README.md | 167 ++-- Rakefile | 5 +- cached_resource.gemspec | 30 +- lib/cached_resource.rb | 20 +- lib/cached_resource/cached_resource.rb | 14 +- lib/cached_resource/caching.rb | 72 +- lib/cached_resource/configuration.rb | 33 +- lib/cached_resource/version.rb | 2 +- scripts/docker/run_tests.sh | 61 ++ spec/cached_resource/cached_resource_spec.rb | 68 +- spec/cached_resource/caching_spec.rb | 833 ++++++------------- spec/cached_resource/configuration_spec.rb | 241 ++---- spec/spec_helper.rb | 25 +- spec/support/matchers.rb | 34 + 19 files changed, 772 insertions(+), 1021 deletions(-) create mode 100644 .standard.yml create mode 100644 Dockerfile.test create mode 100755 scripts/docker/run_tests.sh create mode 100644 spec/support/matchers.rb diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index bc101b9..e285292 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -5,95 +5,28 @@ # This workflow will download a prebuilt Ruby version, install dependencies and run tests with Rake # For more information see: https://github.com/marketplace/actions/setup-ruby-jruby-and-truffleruby -name: Test - -on: [push] - +name: Code Guard +on: + pull_request: + workflow_dispatch: +concurrency: + group: ${{ github.ref }}-${{ github.workflow }} + cancel-in-progress: true permissions: contents: read - jobs: test: - runs-on: ubuntu-20.04 - strategy: matrix: - ruby-version: ['2.3', '2.4', '2.5', '2.6', '2.7', '3.0', '3.1', '3.2', '3.3'] - rails-version: ['4.2', '5.0', '5.1', '6.0', '6.1', '7.0', '7.1'] - exclude: - # activesupport (~> 6.0.0) was resolved to 6.0.6.1, which depends on ruby (>= 2.5.0) - # activesupport (~> 6.1.0) was resolved to 6.1.7.2, which depends on ruby (>= 2.5.0) - - ruby-version: '2.3' - rails-version: '6.0' - - ruby-version: '2.3' - rails-version: '6.1' - - ruby-version: '2.4' - rails-version: '6.0' - - ruby-version: '2.4' - rails-version: '6.1' - # activesupport (~> 7.0.0) was resolved to 7.0.4.2, which depends on Ruby (>= 2.7.0) - - ruby-version: '2.3' - rails-version: '7.0' - - ruby-version: '2.4' - rails-version: '7.0' - - ruby-version: '2.5' - rails-version: '7.0' - - ruby-version: '2.6' - rails-version: '7.0' - # incompatbility with BigDecimal.new - - ruby-version: '2.7' - rails-version: '4.2' - - ruby-version: '3.0' - rails-version: '4.2' - - ruby-version: '3.1' - rails-version: '4.2' - - ruby-version: '3.2' - rails-version: '4.2' - # ArgumentError: expected attributes to be able to convert to Hash, got "#" - # probably keyword argument delegation different in Ruby 3 - # https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/ - - ruby-version: '3.0' - rails-version: '5.0' - - ruby-version: '3.0' - rails-version: '5.1' - - ruby-version: '3.1' - rails-version: '5.0' - - ruby-version: '3.1' - rails-version: '5.1' - - ruby-version: '3.2' - rails-version: '5.0' - - ruby-version: '3.2' - rails-version: '5.1' - # rails (~> 7.1.0) was resolved to 7.1.3.2, which depends on Ruby (>= 2.7.0) - - ruby-version: '2.3' - rails-version: '7.1' - - ruby-version: '2.4' - rails-version: '7.1' - - ruby-version: '2.5' - rails-version: '7.1' - - ruby-version: '2.6' - rails-version: '7.1' - # Because rails >= 4.0.0.beta1, < 5.0.5.rc1 depends on bundler >= 1.3.0, < 2.0 - # and the current Bundler version (2.5.9) does not satisfy bundler >= 1.3.0, < 2.0, - # rails >= 4.0.0.beta1, < 5.0.5.rc1 cannot be used. - # So, because Gemfile depends on rails ~> 4.2.0, - # version solving has failed. - - ruby-version: '3.3' - rails-version: '4.2' - + ruby-version: ['3.0', '3.1', '3.2', '3.3'] + rails-version: ['6.1', '7.0', '7.1'] steps: - uses: actions/checkout@v4 - - name: Set up Ruby ${{ matrix.ruby-version }} - # To automatically get bug fixes and new Ruby versions for ruby/setup-ruby, - # change this to (see https://github.com/ruby/setup-ruby#versioning): - # uses: ruby/setup-ruby@v1 + - name: Set up Ruby uses: ruby/setup-ruby@v1 with: ruby-version: ${{ matrix.ruby-version }} - - name: Install bundler 1.x.x - if: matrix.rails-version == '4.2' - run: gem uninstall -aIx bundler && gem install bundler -v 1.17.3 - name: Install dependencies run: bundle install env: diff --git a/.gitignore b/.gitignore index 83f5ebf..36a67c0 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,5 @@ pkg/* *DS_Store .ruby-version .tool-versions +coverage +.standard_todo.yml diff --git a/.standard.yml b/.standard.yml new file mode 100644 index 0000000..8e65e46 --- /dev/null +++ b/.standard.yml @@ -0,0 +1,3 @@ +fix: false +parallel: true +ruby_version: 3.0 \ No newline at end of file diff --git a/Dockerfile.test b/Dockerfile.test new file mode 100644 index 0000000..98f1c0c --- /dev/null +++ b/Dockerfile.test @@ -0,0 +1,74 @@ +FROM bash AS base + +ENV LANG C.UTF-8 +ENV LC_ALL C.UTF-8 +ENV ASDF_DIR=/root/.asdf + +# Install dependencies +RUN apk update && \ + apk add --no-cache \ + build-base \ + git \ + libffi-dev \ + openssl-dev \ + perl \ + readline-dev \ + tzdata \ + yaml-dev \ + zlib-dev + +RUN git clone https://github.com/asdf-vm/asdf.git /root/.asdf --branch v0.14.0 && \ + . "$ASDF_DIR/asdf.sh" && \ + asdf plugin add ruby + +## For parallelism, build in stages +# Ruby 3.0.7 +FROM base AS ruby-3.0.7 +RUN . "$ASDF_DIR/asdf.sh" && \ + asdf install ruby 3.0.7 && \ + asdf global ruby 3.0.7 && \ + gem install bundler + +# Ruby 3.1.4 +FROM base AS ruby-3.1.4 +RUN . "$ASDF_DIR/asdf.sh" && \ + asdf install ruby 3.1.4 && \ + asdf global ruby 3.1.4 && \ + gem install bundler + +# Ruby 3.2.2 +FROM base AS ruby-3.2.2 +RUN . "$ASDF_DIR/asdf.sh" && \ + asdf install ruby 3.2.2 && \ + asdf global ruby 3.2.2 && \ + gem install bundler + +# Ruby 3.3.4 +FROM base AS ruby-3.3.4 +RUN . "$ASDF_DIR/asdf.sh" && \ + asdf install ruby 3.3.4 && \ + asdf global ruby 3.3.4 && \ + gem install bundler + +# Final Image with Application Code +FROM base AS final + +# Copy and merge installed ASDF directory from ruby versions +COPY --from=ruby-3.0.7 /root/.asdf /tmp/.asdf-3.0.7 +COPY --from=ruby-3.1.4 /root/.asdf /tmp/.asdf-3.1.4 +COPY --from=ruby-3.2.2 /root/.asdf /tmp/.asdf-3.2.2 +COPY --from=ruby-3.3.4 /root/.asdf /tmp/.asdf-3.3.4 +RUN cp -r /tmp/.asdf-3.0.7/* /root/.asdf/ && \ + cp -r /tmp/.asdf-3.1.4/* /root/.asdf/ && \ + cp -r /tmp/.asdf-3.2.2/* /root/.asdf/ && \ + cp -r /tmp/.asdf-3.3.4/* /root/.asdf/ && \ + rm -rf /tmp/.asdf* + +WORKDIR /app + +COPY gemfiles gemfiles +COPY lib lib +COPY spec spec +COPY cached_resource.gemspec Gemfile Rakefile scripts/docker/run_tests.sh .standard.yml . + +CMD ["./run_tests.sh"] diff --git a/Gemfile b/Gemfile index 41ee41e..6cec6e3 100644 --- a/Gemfile +++ b/Gemfile @@ -5,27 +5,27 @@ gemspec def eval_gemfile(path) gemfile_local = File.expand_path(path, __FILE__) if File.readable?(gemfile_local) - puts "Loading #{gemfile_local}..." if ENV['DEBUG'] + puts "Loading #{gemfile_local}..." if ENV["DEBUG"] instance_eval(File.read(gemfile_local)) end end -puts "\e[93mUsing TEST_RAILS_VERSION #{ENV['TEST_RAILS_VERSION']}\e[0m" if ENV['DEBUG'] -case ENV['TEST_RAILS_VERSION'] +puts "\e[93mUsing TEST_RAILS_VERSION #{ENV["TEST_RAILS_VERSION"]}\e[0m" if ENV["DEBUG"] +case ENV["TEST_RAILS_VERSION"] when "4.2" - eval_gemfile('../gemfiles/4.2.gemfile') + eval_gemfile("../gemfiles/4.2.gemfile") when "5.0" - eval_gemfile('../gemfiles/5.0.gemfile') + eval_gemfile("../gemfiles/5.0.gemfile") when "5.1" - eval_gemfile('../gemfiles/5.1.gemfile') + eval_gemfile("../gemfiles/5.1.gemfile") when "6.0" - eval_gemfile('../gemfiles/6.0.gemfile') + eval_gemfile("../gemfiles/6.0.gemfile") when "6.1" - eval_gemfile('../gemfiles/6.1.gemfile') + eval_gemfile("../gemfiles/6.1.gemfile") when "7.0" - eval_gemfile('../gemfiles/7.0.gemfile') + eval_gemfile("../gemfiles/7.0.gemfile") when "7.1" - eval_gemfile('../gemfiles/7.1.gemfile') + eval_gemfile("../gemfiles/7.1.gemfile") else - puts "\e[93mNo TEST_RAILS_VERSION present, letting dependency manager decide what's best.\e[0m" if ENV['DEBUG'] + puts "\e[93mNo TEST_RAILS_VERSION present, letting dependency manager decide what's best.\e[0m" if ENV["DEBUG"] end diff --git a/README.md b/README.md index 5f0f59e..f991bf6 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # CachedResource ![Tests](https://github.com/mhgbrown/cached_resource/actions/workflows/ruby.yml/badge.svg) -CachedResource is a Ruby gem whose goal is to increase the performance of interacting with web services via ActiveResource by caching responses based on request parameters. It can help reduce the lag created by making repeated requests across a network. +CachedResource is a Ruby gem designed to enhance the performance of web service interactions through ActiveResource by caching responses based on request parameters. By reducing the need for repeated network requests, it minimizes latency and optimizes the efficiency of your application's data retrieval processes. ## Installation @@ -12,25 +12,11 @@ gem install cached_resource CachedResource is designed to be framework agnostic, but will hook into Rails for caching and logging if available. CachedResource supports the following ActiveSupport/Rails (right) and Ruby (down) version combinations: -| | 🛤️ 4.2 | 🛤️ 5.0 | 🛤️ 5.1 | 🛤️ 6.0 | 🛤️ 6.1 | 🛤️ 7.0 | 🛤️ 7.1 | -|-------|-----|-----|-----|-----|-----|-----|-----| -| 💎 2.3 | ✅ | ✅ | ✅ | | | | | -| 💎 2.4 | ✅ | ✅ | ✅ | | | | | -| 💎 2.5 | ✅ | ✅ | ✅ | ✅ | ✅ | | | -| 💎 2.6 | ✅ | ✅ | ✅ | ✅ | ✅ | | -| 💎 2.7 | | ✅ | ✅ | ✅ | ✅ | ✅ |✅ | -| 💎 3.0 | | | | ✅ | ✅ | ✅ | ✅ | -| 💎 3.1 | | | | ✅ | ✅ | ✅ | ✅ | -| 💎 3.2 | | | | ✅ | ✅ | ✅ | ✅ | -| 💎 3.3 | | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | - -## Limitations - -The following are limitations for ActiveResource/Rails versions - -| ActiveSupport Version | Limitation | -|---------------------- | ----------------------------------------------------------------------- | -| 🛤️ 4.X | You cannot chain calls. Ie `Thing.where(fn: 'foo').where(ln: 'bar')`.
However, you can still access `original_params` and the `resource_class` and replicate it with
`call1 = Thing.where(fn: 'foo')`
`call1.resource_class.where(call1.original_params.merge(ln: 'bar'))` | +| | 🛤️ 6.1 | 🛤️ 7.0 | 🛤️ 7.1 | +|----------|:------:|:------:|:------:| +| 💎 3.0 | ✅ | ✅ | ✅ | +| 💎 3.1 | ✅ | ✅ | ✅ | +| 💎 3.2 | ✅ | ✅ | ✅ | ## Configuration @@ -38,7 +24,7 @@ The following are limitations for ActiveResource/Rails versions ```ruby class ActiveResource::Base - cached_resource + cached_resource(options) end ``` @@ -46,81 +32,45 @@ Or set up CachedResource for a single class: ```ruby class MyActiveResource < ActiveResource::Base - cached_resource + cached_resource(options) end ``` ### Options -CachedResource accepts the following options: - -* `:enabled` Default: `true` -* `:cache_collections` Set to false to always remake a request for collections. Default: `true` -* `:cache` The cache store that CacheResource should use. Default: The `Rails.cache` if available, or an `ActiveSupport::Cache::MemoryStore` -* `:collection_arguments` The arguments that identify the principal collection request. Default: `[:all]` -* `:collection_synchronize` Use collections to generate cache entries for individuals. Update the existing cached principal collection when retrieving subsets of the principal collection or individuals. Default: `false` -* `:concurrent_write` Set to true to make concurrent writes to cache after successful API response. Default: `false` - * Requires the [concurrent-ruby](https://rubygems.org/gems/concurrent-ruby) gem -* `:logger` The logger to which CachedResource messages should be written. Default: The `Rails.logger` if available, or an `ActiveSupport::Logger` -* `:race_condition_ttl` The race condition ttl, to prevent [dog pile effect](https://en.wikipedia.org/wiki/Cache_stampede) or [cache stampede](https://en.wikipedia.org/wiki/Cache_stampede). Default: 86400 -* `:ttl_randomization_scale` A Range from which a random value will be selected to scale the ttl. Default: `1..2` -* `:ttl_randomization` Enable ttl randomization. Default: `false` -* `:ttl` The time in seconds until the cache should expire. Default: `604800` - -You can set them like this: - +CachedResource accepts the following options as a hash: + +| Option | Description | Default | +|----------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------| +| `:enabled` | Enables or disables caching. | `true` | +| `:cache_collections` | Set to false to always remake a request for collections. | `true` | +| `:cache` | The cache store that CacheResource should use. | The `Rails.cache` if available, or an `ActiveSupport::Cache::MemoryStore` | +| `:cache_key_prefix` | A prefix to be added to the cache keys. | `nil` | +| `:collection_arguments` | The arguments that identify the principal collection request. | `[:all]` | +| `:collection_synchronize` | Use collections to generate cache entries for individuals. Update the existing cached principal collection when retrieving subsets of the principal collection or individuals. | `false` | +| `:logger` | The logger to which CachedResource messages should be written. | The `Rails.logger` if available, or an `ActiveSupport::Logger` | +| `:race_condition_ttl` | The race condition ttl, to prevent [dog pile effect](https://en.wikipedia.org/wiki/Cache_stampede) or [cache stampede](https://en.wikipedia.org/wiki/Cache_stampede). | `86400` | +| `:ttl_randomization_scale` | A Range from which a random value will be selected to scale the ttl. | `1..2` | +| `:ttl_randomization` | Enable ttl randomization. | `false` | +| `:ttl` | The time in seconds until the cache should expire. | `604800` | + +For example: ```ruby cached_resource :cache => MyCacheStore.new, :ttl => 60, :collection_synchronize => true, :logger => MyLogger.new ``` -You can also change them on the fly. - -Turn CachedResource off. This will cause all responses to be retrieved normally (i.e. via the network). All responses will still be cached. - -```ruby - MyActiveResource.cached_resource.off! -``` - -Turn CachedResource on. -```ruby -MyActiveResource.cached_resource.on! -``` - -Set the cache expiry time to 60 seconds. - -```ruby -MyActiveResource.cached_resource.ttl = 60 -``` - -Enable cache expiry time randomization, allowing it to fall randomly between 60 and 120 seconds. - -```ruby -MyActiveResource.cached_resource.ttl_randomization = true -``` - -Change the cache expiry time randomization scale so that the cache expiry time falls randomly between 30 and 180 seconds. +#### You can also change them dynamically. Simply: ```ruby -MyActiveResource.cached_resource.ttl_randomization_scale = 0.5..3 + MyActiveResource.cached_resource.option = option_value ``` -Enable collection synchronization. This will cause a call to `MyActiveResource.all` to also create cache entries for each of its members. So, for example, a later call to `MyActiveResource.find(1)` will be read from the cache instead of requested from the remote service. -```ruby -MyActiveResource.cached_resource.collection_synchronize = true -``` -Change the arguments that identify the principal collection request. If for some reason you are concerned with a collection that is retrieved at a "non-standard" URL, you may specify the Ruby arguments that produce that URL. When `collection_synchronize` is `true`, the collection returned from a request that matches these arguments will be cached and later updated when one of its members or a subset is retrieved. +Additionally a couple of helper methods are available: ```ruby -MyActiveResource.cached_resource.collection_arguments = [:all, :params => {:name => "Bob"}] -``` -Set a different logger. - -```ruby -MyActiveResource.cached_resource.logger = MyLogger.new -``` -Set a different cache store. - -```ruby -MyActiveResource.cached_resource.cache = MyCacheStore.new + # Turn caching off + MyActiveResource.cached_resource.off! + # Turn caching on + MyActiveResource.cached_resource.on! ``` ### Caveats @@ -134,7 +84,7 @@ end ```ruby class MyActiveResource < ActiveResource::Base - self.cached_resource = CachedResource::Configuration.new(:collection_synchronize => true) + self.cached_resource = CachedResource::Configuration.new(:collection_synchronize: true) end ``` ## Usage @@ -161,45 +111,36 @@ Since resources are cached with an argument based key, you may pass in extra dat MyActiveResource.find(:one, from: "/admin/shop.json", uid: "unique value") ``` -## Testing +## Contribution +### Testing +#### Locally +You can set `TEST_RAILS_VERSION` to a value of the supported Rails versions in the [Compatability Matrix](#compatibility), or bundler will automatically select a version. -To test the Ruby + Rails combination configured by default: - -```bash -$ rake -``` - -or to test all supported environments...you have to do a little more work... - -Switch your Ruby version to the desired version. This project's maintainer uses `asdf`, so switching to Ruby 3 looks like this: - -```bash -$ asdf local ruby 3.0.5 +Examples: +```sh +TEST_RAILS_VERSION=7.1 bundle exec rspec # runs test suite + linter +bundle exec rspec # Runs test suite ``` -If you have a `Gemfile.lock`, delete it: +#### With Docker +```sh +docker build -t cached_resource_test -f Dockerfile.test . +docker run --rm -v ${PWD}/coverage:/app/coverage cached_resource_test -```bash -$ rm Gemfile.lock +# Coverage report can be found in coverage/index.html +# RSpec reports can be found in coverage/spec_results/${ruby-version}-${rails-version}.index.html +# Linter reports can be found in coverage/linter-results.index.html ``` -Then reinstall your dependencies: +### Code Coverage +Coverage can be found found within `coverage/index.html` after every test run. -```bash -$ bundle install -``` - -and finally, run the tests: +### Linter +We use unconfigurable linting rules from [Standard](https://github.com/standardrb/standard) -```bash -$ rake -``` +To lint run: `bundle exec rake standard` +To automatically apply linter fixes: `bundle exec rake standard:fix` -If you want to test with a specific Rails version, start over and install dependencies with `TEST_RAILS_VERSION` set: - -```bash -$ TEST_RAILS_VERSION=6.1 bundle install -``` ## Credit/Inspiration * quamen and [this gist](http://gist.github.com/947734) diff --git a/Rakefile b/Rakefile index 8851c33..93aa5a5 100644 --- a/Rakefile +++ b/Rakefile @@ -1,7 +1,8 @@ require "bundler/gem_tasks" -require 'rspec/core/rake_task' +require "rspec/core/rake_task" +require "standard/rake" desc "Run all examples" RSpec::Core::RakeTask.new(:spec) -task :default => :spec +task default: [:spec, :standard] diff --git a/cached_resource.gemspec b/cached_resource.gemspec index 3db4d19..b921563 100644 --- a/cached_resource.gemspec +++ b/cached_resource.gemspec @@ -1,29 +1,31 @@ -# -*- encoding: utf-8 -*- $:.push File.expand_path("../lib", __FILE__) require "cached_resource/version" Gem::Specification.new do |s| - s.name = "cached_resource" - s.version = CachedResource::VERSION - s.authors = "Morgan Brown" - s.email = "cached_resource@email.mhgbrown.is" - s.homepage = "https://github.com/mhgbrown/cached_resource" - s.summary = %q{Caching for ActiveResource} - s.description = %q{Enables request-based caching for ActiveResource} - s.licenses = ['MIT'] + s.name = "cached_resource" + s.version = CachedResource::VERSION + s.authors = "Morgan Brown" + s.email = "cached_resource@email.mhgbrown.is" + s.homepage = "https://github.com/mhgbrown/cached_resource" + s.summary = "Caching for ActiveResource" + s.description = "Enables request-based caching for ActiveResource" + s.licenses = ["MIT"] - s.files = `git ls-files`.split("\n") - s.test_files = `git ls-files -- {test,spec,features}/*`.split("\n") - s.executables = `git ls-files -- bin/*`.split("\n").map{ |f| File.basename(f) } + s.files = Dir.glob("lib/**/*") + Dir.glob("bin/**/*") + s.executables = Dir.glob("bin/*").map { |f| File.basename(f) } s.require_paths = ["lib"] - s.required_ruby_version = '>= 1.9.0' + s.required_ruby_version = ">= 3.0" s.add_runtime_dependency "activeresource", ">= 4.0" s.add_runtime_dependency "activesupport", ">= 4.0" s.add_runtime_dependency "nilio", ">= 1.0" + s.add_development_dependency "concurrent-ruby" + s.add_development_dependency "pry-byebug" s.add_development_dependency "rake" s.add_development_dependency "rspec" - s.add_development_dependency "concurrent-ruby", '~> 1.2', '>= 1.2.3' + s.add_development_dependency "simplecov", "~> 0.22.0" + s.add_development_dependency "standard", "~> 1.39", ">= 1.39.1" + s.add_development_dependency "timecop", "~> 0.9.10" end diff --git a/lib/cached_resource.rb b/lib/cached_resource.rb index d1f5db6..b3567a2 100644 --- a/lib/cached_resource.rb +++ b/lib/cached_resource.rb @@ -1,14 +1,14 @@ -require 'ostruct' +require "ostruct" -require 'nilio' -require 'active_support/cache' -require 'active_support/concern' -require 'active_support/logger' -require 'cached_resource/cached_resource' -require 'cached_resource/configuration' -require 'cached_resource/caching' -require 'cached_resource/version' -require 'active_resource' +require "nilio" +require "active_support/cache" +require "active_support/concern" +require "active_support/logger" +require "cached_resource/cached_resource" +require "cached_resource/configuration" +require "cached_resource/caching" +require "cached_resource/version" +require "active_resource" module CachedResource # nada diff --git a/lib/cached_resource/cached_resource.rb b/lib/cached_resource/cached_resource.rb index 0ff1750..3e89b1a 100644 --- a/lib/cached_resource/cached_resource.rb +++ b/lib/cached_resource/cached_resource.rb @@ -10,7 +10,7 @@ class << self attr_accessor :cached_resource # Initialize cached resource or retrieve the current cached resource configuration. - def cached_resource(options={}) + def cached_resource(options = {}) defined?(@cached_resource) && @cached_resource || setup_cached_resource!(options) end @@ -18,16 +18,6 @@ def cached_resource(options={}) # and establishing the necessary methods. def setup_cached_resource!(options) @cached_resource = CachedResource::Configuration.new(options) - if @cached_resource.concurrent_write - begin - send :require, 'concurrent/promise' - rescue LoadError - @cached_resource.logger.error( - "`concurrent_write` option is enabled, but `concurrent-ruby` is not an installed dependency" - ) - raise - end - end send :include, CachedResource::Caching @cached_resource end @@ -40,7 +30,7 @@ module ClassMethods # that wants an independent configuration will need to execute: # self.cached_resource = CachedResource::Configuration.new(options={}) def inherited(child) - child.cached_resource = self.cached_resource if defined?(@cached_resource) + child.cached_resource = cached_resource if defined?(@cached_resource) super end end diff --git a/lib/cached_resource/caching.rb b/lib/cached_resource/caching.rb index 803b534..0878c12 100644 --- a/lib/cached_resource/caching.rb +++ b/lib/cached_resource/caching.rb @@ -25,8 +25,10 @@ def find_with_cache(*arguments) end # Clear the cache. - def clear_cache(options=nil) + def clear_cache(options = nil) cache_clear(options) + + true end private @@ -73,7 +75,9 @@ def update_collection_cache(updates, *arguments) collection = cache_read(cache_key(cached_resource.collection_arguments)) if collection && !updates.empty? - index = collection.inject({}) { |hash, object| hash[object.send(primary_key)] = object; hash } + index = collection.each_with_object({}) { |object, hash| + hash[object.send(primary_key)] = object + } updates.each { |object| index[object.send(primary_key)] = object } cache_write(cache_key(cached_resource.collection_arguments), index.values, *arguments) end @@ -94,13 +98,12 @@ def is_collection?(*arguments) # Determine if the given arguments represent # any collection of objects def is_any_collection?(*arguments) - cached_resource.collection_arguments.all?{ |arg| arguments.include?(arg) } || arguments.include?(:all) + cached_resource.collection_arguments.all? { |arg| arguments.include?(arg) } || arguments.include?(:all) end # Read a entry from the cache for the given key. def cache_read(key) object = cached_resource.cache.read(key).try do |json_cache| - json = ActiveSupport::JSON.decode(json_cache) unless json.nil? @@ -110,7 +113,7 @@ def cache_read(key) next restored unless respond_to?(:collection_parser) collection_parser.new(restored).tap do |parser| parser.resource_class = self - parser.original_params = json['original_params'].deep_symbolize_keys + parser.original_params = json["original_params"].deep_symbolize_keys end else full_dup(cache) @@ -123,27 +126,17 @@ def cache_read(key) # Write an entry to the cache for the given key and value. def cache_write(key, object, *arguments) - if cached_resource.concurrent_write - Concurrent::Promise.execute { _cache_write(key, object, *arguments) } && true - else - _cache_write(key, object, *arguments) - end - end - - def _cache_write(key, object, *arguments) options = arguments[1] || {} params = options[:params] prefix_options, query_options = split_options(params) - result = cached_resource.cache.write(key, object_to_json(object, prefix_options, query_options), :race_condition_ttl => cached_resource.race_condition_ttl, :expires_in => cached_resource.generate_ttl) + result = cached_resource.cache.write(key, object_to_json(object, prefix_options, query_options), race_condition_ttl: cached_resource.race_condition_ttl, expires_in: cached_resource.generate_ttl) result && cached_resource.logger.info("#{CachedResource::Configuration::LOGGER_PREFIX} WRITE #{key}") result end - # Clear the cache. - def cache_clear(options=nil) - # Memcache doesn't support delete_matched, which can also be computationally expensive - if cached_resource.cache.class.to_s == 'ActiveSupport::Cache::MemCacheStore' || options.try(:fetch,:all) + def cache_clear(options = nil) + if !cached_resource.cache.respond_to?(:delete_matched) || options.try(:fetch, :all) cached_resource.cache.clear.tap do |result| cached_resource.logger.info("#{CachedResource::Configuration::LOGGER_PREFIX} CLEAR ALL") end @@ -155,8 +148,8 @@ def cache_clear(options=nil) end def cache_key_delete_pattern - case cached_resource.cache.class.to_s - when "ActiveSupport::Cache::MemoryStore", "ActiveSupport::Cache::FileStore" + case cached_resource.cache + when ActiveSupport::Cache::MemoryStore, ActiveSupport::Cache::FileStore /^#{name_key}\// else "#{name_key}/*" @@ -165,13 +158,22 @@ def cache_key_delete_pattern # Generate the request cache key. def cache_key(*arguments) - "#{name_key}/#{arguments.join('/')}".downcase.delete(' ') + "#{name_key}/#{arguments.join("/")}".downcase.delete(" ") end def name_key - @name_key ||= begin - prefix = cached_resource.cache_key_prefix.nil? ? "" : "#{cached_resource.cache_key_prefix}/" - prefix + name.parameterize.gsub("-", "/") + cache_key_prefix + name.parameterize.tr("-", "/") + end + + def cache_key_prefix + prefix = cached_resource.cache_key_prefix + return "" if prefix.nil? + + if prefix.respond_to?(:call) + result = prefix.call + result.nil? ? "" : "#{result}/" + else + "#{prefix}/" end end @@ -184,16 +186,16 @@ def full_dup(record) end def json_to_object(json) - resource = json['resource'] + resource = json["resource"] if resource.is_a? Array resource.map do |attrs| - self.new(attrs["object"], attrs["persistence"]).tap do |resource| - resource.prefix_options = json['prefix_options'] + new(attrs["object"], attrs["persistence"]).tap do |resource| + resource.prefix_options = json["prefix_options"] end end else - self.new(resource["object"], resource["persistence"]).tap do |resource| - resource.prefix_options = json['prefix_options'] + new(resource["object"], resource["persistence"]).tap do |resource| + resource.prefix_options = json["prefix_options"] end end end @@ -201,16 +203,14 @@ def json_to_object(json) def object_to_json(object, prefix_options, query_options) if object.is_a? Enumerable { - :resource => object.map { |o| { :object => o, :persistence => o.persisted? } }, - :prefix_options => prefix_options, - :original_params => query_options + resource: object.map { |o| {object: o, persistence: o.persisted?} }, + prefix_options: prefix_options, + original_params: query_options }.to_json - elsif object.nil? - nil.to_json else { - :resource => { :object => object, :persistence => object.persisted? }, - :prefix_options => prefix_options + resource: {object: object, persistence: object.persisted?}, + prefix_options: prefix_options }.to_json end end diff --git a/lib/cached_resource/configuration.rb b/lib/cached_resource/configuration.rb index 2c8655a..ae381da 100644 --- a/lib/cached_resource/configuration.rb +++ b/lib/cached_resource/configuration.rb @@ -2,7 +2,6 @@ module CachedResource # The Configuration class manages class specific options # for cached resource. class Configuration < OpenStruct - # default or fallback cache without rails CACHE = ActiveSupport::Cache::MemoryStore.new @@ -24,21 +23,19 @@ class Configuration < OpenStruct # :cache, default: Rails.cache or ActiveSupport::Cache::MemoryStore.new, # :logger, default: Rails.logger or ActiveSupport::Logger.new(NilIO.new), # :cache_collections, default: true - # :concurrent_write, default: false - def initialize(options={}) + def initialize(options = {}) super({ - :cache => defined?(Rails.cache) && Rails.cache || CACHE, - :cache_collections => true, - :cache_key_prefix => nil, - :collection_arguments => [:all], - :collection_synchronize => false, - :concurrent_write => false, - :enabled => true, - :logger => defined?(Rails.logger) && Rails.logger || LOGGER, - :race_condition_ttl => 86400, - :ttl => 604800, - :ttl_randomization => false, - :ttl_randomization_scale => 1..2 + cache: defined?(Rails.cache) && Rails.cache || CACHE, + cache_collections: true, + cache_key_prefix: nil, + collection_arguments: [:all], + collection_synchronize: false, + enabled: true, + logger: defined?(Rails.logger) && Rails.logger || LOGGER, + race_condition_ttl: 86400, + ttl: 604800, + ttl_randomization: false, + ttl_randomization_scale: 1..2 }.merge(options)) end @@ -50,6 +47,8 @@ def generate_ttl end # Enables caching. + # NOTE: Disabled coverage because it is not being picked up by simplecov + # :nocov: def on! self.enabled = true end @@ -58,6 +57,7 @@ def on! def off! self.enabled = false end + # :nocov: private @@ -69,10 +69,9 @@ def randomized_ttl # Choose a random value from within the given range, optionally # seeded by seed. - def sample_range(range, seed=nil) + def sample_range(range, seed = nil) srand seed if seed rand * (range.end - range.begin) + range.begin end - end end diff --git a/lib/cached_resource/version.rb b/lib/cached_resource/version.rb index 3325b48..c4d22b3 100644 --- a/lib/cached_resource/version.rb +++ b/lib/cached_resource/version.rb @@ -1,3 +1,3 @@ module CachedResource - VERSION = "8.0.0" + VERSION = "9.0.0" end diff --git a/scripts/docker/run_tests.sh b/scripts/docker/run_tests.sh new file mode 100755 index 0000000..913a78c --- /dev/null +++ b/scripts/docker/run_tests.sh @@ -0,0 +1,61 @@ +#!/bin/sh + +set -e + +export BUNDLE_SILENCE_ROOT_WARNING=1 +export BUNDLE_IGNORE_MESSAGES=1 + +. "$ASDF_DIR/asdf.sh" +SPEC_RESULTS="coverage/spec_results" +mkdir -p "$SPEC_RESULTS" + +# Matrix of Ruby and Rails versions +RUBY_VERSIONS=$(asdf list ruby) +RAILS_VERSIONS="6.1 7.0 7.1" + +# Maximum number of concurrent processes +MAX_PARALLEL=3 +CURRENT_PARALLEL=0 + +run_rspec() { + local ruby_version="$1" + local rails_version="$2" + + echo "********* Testing with Ruby $ruby_version and Rails $rails_version *********" + + # Install gems + TEST_RAILS_VERSION="$rails_version" bundle install --quiet --no-cache + + # Run tests + TEST_RAILS_VERSION="$rails_version" bundle exec rspec \ + --format failures \ + --format html --out "$SPEC_RESULTS/$ruby_version-$rails_version.index.html" +} + +# Iterate over Ruby versions +for RUBY_VERSION in $RUBY_VERSIONS; do + asdf reshim ruby $RUBY_VERSION + asdf shell ruby $RUBY_VERSION + + # Iterate over Rails versions + for RAILS_VERSION in $RAILS_VERSIONS; do + # Parallelize with arbitrary limit to prevent crashing + while [ $CURRENT_PARALLEL -ge $MAX_PARALLEL ]; do + sleep 1 + CURRENT_PARALLEL=$(jobs | wc -l) + done + + (run_rspec "$RUBY_VERSION" "$RAILS_VERSION") & CURRENT_PARALLEL=$((CURRENT_PARALLEL + 1)) + done + + wait + + CURRENT_PARALLEL=0 +done + +echo "********* Running Linter *********" +bundle exec standardrb \ + --format html --out "coverage/linter-results.index.html" \ + --format offenses + +echo "********* DONE *********" diff --git a/spec/cached_resource/cached_resource_spec.rb b/spec/cached_resource/cached_resource_spec.rb index 970a777..5dec298 100644 --- a/spec/cached_resource/cached_resource_spec.rb +++ b/spec/cached_resource/cached_resource_spec.rb @@ -1,29 +1,67 @@ -require 'spec_helper' +require "spec_helper" -RSpec.describe CachedResource do - before do - class BaseThing < ActiveResource::Base +RSpec.describe CachedResource::Model do + let(:logger) { double(:Logger, error: nil) } + let(:dummy_class) do + Class.new(ActiveResource::Base) do + include CachedResource::Model end + end + + describe ".cached_resource" do + context "when cached resource is not set up" do + it "initializes and returns a new cached resource configuration" do + options = {logger: logger} + allow(CachedResource::Configuration).to receive(:new).with(options).and_call_original + + cached_resource = dummy_class.cached_resource(options) - class FirstChildThing < BaseThing - self.site = 'http://api.first-child-thing.com' - cached_resource + expect(cached_resource).to be_a(CachedResource::Configuration) + expect(dummy_class.instance_variable_get(:@cached_resource)).to eq(cached_resource) + expect(CachedResource::Configuration).to have_received(:new).with(options) + end end - class SecondChildThing < BaseThing - self.site = 'http://api.second-child-thing.com' + context "when cached resource is already set up" do + it "returns the existing cached resource configuration" do + existing_config = CachedResource::Configuration.new + dummy_class.instance_variable_set(:@cached_resource, existing_config) + + cached_resource = dummy_class.cached_resource + + expect(cached_resource).to eq(existing_config) + end end end - after do - [:BaseThing, :FirstChildThing, :SecondChildThing].each do |klass| - Object.send(:remove_const, klass) + describe ".setup_cached_resource!" do + it "creates a new cached resource configuration" do + options = {logger: logger} + allow(CachedResource::Configuration).to receive(:new).with(options).and_call_original + + dummy_class.setup_cached_resource!(options) + + expect(dummy_class.instance_variable_get(:@cached_resource)).to be_a(CachedResource::Configuration) + expect(CachedResource::Configuration).to have_received(:new).with(options) end end - describe '.inherited' do - it 'should include descendants when calling .descendants' do - BaseThing.descendants.sort_by { |klass| klass.name }.should == [FirstChildThing, SecondChildThing] + describe ".inherited" do + let(:child_class) { Class.new(dummy_class) } + + context "when cached resource is defined in superclass" do + it "copies the cached resource configuration to the subclass" do + existing_config = CachedResource::Configuration.new + dummy_class.instance_variable_set(:@cached_resource, existing_config) + + expect(child_class.cached_resource).to eq(existing_config) + end + end + + context "when cached resource is not defined in superclass" do + it "does not set cached resource configuration in the subclass" do + expect(child_class.instance_variable_defined?(:@cached_resource)).to be_falsey + end end end end diff --git a/spec/cached_resource/caching_spec.rb b/spec/cached_resource/caching_spec.rb index efc927e..c254c9f 100644 --- a/spec/cached_resource/caching_spec.rb +++ b/spec/cached_resource/caching_spec.rb @@ -1,664 +1,385 @@ -require 'spec_helper' +require "spec_helper" -describe CachedResource do +CACHE = ActiveSupport::Cache::MemoryStore.new - def read_from_cache(key) - Thing.send(:cache_read, key) - end +class Thing < ActiveResource::Base + self.site = "http://api.thing.com" + cached_resource +end - before(:each) do - class Thing < ActiveResource::Base - self.site = "http://api.thing.com" - cached_resource - end +class NotTheThing < ActiveResource::Base + self.site = "http://api.notthething.com" + cached_resource +end - class NotTheThing < ActiveResource::Base - self.site = "http://api.notthething.com" - cached_resource - end +def read_from_cache(key, model = Thing) + model.send(:cache_read, key) +end - @thing = {:thing => {:id => 1, :name => "Ada"}} - @thing_collection = [{:id => 1, :name => "Ada"}, {:id => 2, :name => "Ada", :major => 'CS'}] - @thing_collection2 = [{:id => 2, :name => "Ada", :major => 'CS'}] - @other_thing = {:thing => {:id => 1, :name => "Ari"}} - @thing2 = {:thing => {:id => 2, :name => "Joe"}} - @other_thing2 = {:thing => {:id => 2, :name => "Jeb"}} - @thing3 = {:thing => {:id => 3, :name => "Stu"}} - @string_thing = {:thing => {:id => "fded", :name => "Lev"}} - @other_string_thing = {:thing => {:id => "fded", :name => "Lon"}} - @date_thing = {:thing => {:id => 4, :created_at => DateTime.new(2020)}} - @thing_json = @thing.to_json - @other_thing_json = @other_thing.to_json - @string_thing_json = @string_thing.to_json - @other_string_thing_json = @other_string_thing.to_json - @date_thing_json = @date_thing.to_json - @nil_thing = nil.to_json - @empty_array_thing = [].to_json - @not_the_thing = {:not_the_thing => {:id => 1, :name => "Not"}} - @not_the_thing_json = @not_the_thing.to_json +describe CachedResource::Caching do + let(:thing) { {thing: {id: 1, name: "Ada"}} } + let(:thing_collection) { [{id: 1, name: "Ada"}, {id: 2, name: "Ada", major: "CS"}] } + let(:thing_collection2) { [{id: 2, name: "Ada", major: "CS"}] } + let(:other_thing) { {thing: {id: 1, name: "Ari"}} } + let(:thing2) { {thing: {id: 2, name: "Joe"}} } + let(:other_thing2) { {thing: {id: 2, name: "Jeb"}} } + let(:thing3) { {thing: {id: 3, name: "Stu"}} } + let(:string_thing) { {thing: {id: "fded", name: "Lev"}} } + let(:other_string_thing) { {thing: {id: "fded", name: "Lon"}} } + let(:date_thing) { {thing: {id: 4, created_at: DateTime.new(2020)}} } + let(:nil_thing) { nil.to_json } + let(:empty_array_thing) { [].to_json } + let(:not_the_thing) { {not_the_thing: {id: 1, name: "Not"}} } + let(:not_the_thing_collection) { [{not_the_thing: {id: 1, name: "Not"}}] } + + let(:thing_cached_resource) do + double(:thing_cached_resource, + cache_collections: true, + cache_key_prefix: nil, + cache: CACHE, + collection_arguments: [:all], + collection_synchronize: false, + enabled: true, + generate_ttl: 604800, + logger: double(:thing_logger, info: nil, error: nil), + race_condition_ttl: 86400, + ttl_randomization_scale: 1..2, + ttl_randomization: false, + ttl: 604800) end - after(:each) do - Thing.cached_resource.cache.clear - Object.send(:remove_const, :Thing) - NotTheThing.cached_resource.cache.clear - Object.send(:remove_const, :NotTheThing) + let(:not_the_thing_cached_resource) do + double(:not_the_thing_cached_resource, + cache_collections: true, + cache_key_prefix: nil, + cache: CACHE, + collection_arguments: [:all], + collection_synchronize: false, + enabled: true, + generate_ttl: 604800, + logger: double(:not_the_thing_logger, info: nil, error: nil), + race_condition_ttl: 86400, + ttl_randomization_scale: 1..2, + ttl_randomization: false, + ttl: 604800) end - describe "when enabled" do - before(:each) do - # it's on by default, but lets call the method - # to make sure it works - Thing.cached_resource.cache.clear - Thing.cached_resource.on! - NotTheThing.cached_resource.cache.clear - NotTheThing.cached_resource.on! - - ActiveResource::HttpMock.reset! - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/1.json", {}, @thing_json - mock.get "/things/1.json?foo=bar", {}, @thing_json - mock.get "/things/fded.json", {}, @string_thing_json - mock.get "/things.json?name=42", {}, @nil_thing, 404 - mock.get "/things.json?name=43", {}, @empty_array_thing - mock.get "/things/4.json", {}, @date_thing_json - mock.get "/not_the_things/1.json", {}, @not_the_thing_json - end + before do + CACHE.clear + allow(Thing).to receive(:cached_resource).and_return(thing_cached_resource) + allow(NotTheThing).to receive(:cached_resource).and_return(not_the_thing_cached_resource) + ActiveResource::HttpMock.reset! + ActiveResource::HttpMock.respond_to do |mock| + mock.get "/things/1.json", {}, thing.to_json + mock.get "/things/1.json?foo=bar", {}, thing.to_json + mock.get "/things/fded.json", {}, string_thing.to_json + mock.get "/things.json?name=42", {}, nil_thing, 404 + mock.get "/things.json?name=43", {}, empty_array_thing + mock.get "/things/4.json", {}, date_thing.to_json + mock.get "/not_the_things/1.json", {}, not_the_thing.to_json + mock.get "/things.json", {}, thing_collection.to_json + mock.get "/not_the_things.json", {}, not_the_thing_collection.to_json end + end - shared_examples "caching" do - it "should cache a response" do - result = Thing.find(1) - read_from_cache("thing/1").should == result - end - - it "shouldn't cache nil response" do - Thing.find(:all, :params => { :name => '42' }) - read_from_cache("thing/all/name/42").should == nil - end - - it "shouldn't cache blank response" do - Thing.find(:all, :params => { :name => '43' }) - read_from_cache("thing/all/name/43").should == nil - end + context "when caching is enabled" do + before do + allow(thing_cached_resource).to receive(:enabled).and_return(true) + allow(not_the_thing_cached_resource).to receive(:enabled).and_return(true) end - include_examples "caching" - - # NOTE: Possibly flaky, but best easy/cheap way to test concurrency. - context 'when concurrency is turned on' do - before do - # it's on by default, but lets call the method - # to make sure it works - Thing.cached_resource.cache.clear - Thing.cached_resource.on! - NotTheThing.cached_resource.cache.clear - NotTheThing.cached_resource.on! - - ActiveResource::HttpMock.reset! - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/5.json", {}, {:thing => {:id => 1, :name => ("x" * 1_000_000) }}.to_json - end + context "Caching single resource" do + it "caches a response" do + result = Thing.find(1) + Thing.find(1) + expect(read_from_cache("thing/1")).to eq(result) + expect(ActiveResource::HttpMock.requests.length).to eq(1) end - after do - Thing.cached_resource.concurrent_write = false - end - - it "should cache a response asynchronusly when on" do - Thing.cached_resource.concurrent_write = true - result = Thing.find(5) - read_from_cache("thing/5").should == nil - loops = 0 - begin - Timeout.timeout(5) do - loop do - loops += 1 - if read_from_cache("thing/5") == result - break - else - sleep 0.5 - end - end - end - rescue Timeout::Error - RSpec::Expectations.fail_with("Concurrency failed: Cache was not populated within the expected time") - end - - loops.should > 0 - read_from_cache("thing/5").should == result + it "caches without whitespace in keys" do + result = Thing.find(1, from: "path", params: {foo: "bar"}) + expect(read_from_cache('thing/1/{:from=>"path",:params=>{:foo=>"bar"}}')).to eq(result) end - it "will cache a response synchronously when off" do - Thing.cached_resource.concurrent_write = false - result = Thing.find(5) - read_from_cache("thing/5").should == result + it "empties the cache when clear_cache is called" do + Thing.find(1) + expect { Thing.clear_cache }.to change { read_from_cache("thing/1") }.from(kind_of(Thing)).to(nil) end - end - context "When there is a cache prefix" do - before do - Thing.cached_resource.cache_key_prefix = "prefix123" + it "does not empty cache of other ActiveResource objects" do + Thing.find(1) + NotTheThing.find(1) + expect { Thing.clear_cache }.to change { read_from_cache("thing/1") }.from(kind_of(Thing)).to(nil).and( + not_change { read_from_cache("notthething/1", NotTheThing) } + ) end - after do - Thing.cached_resource.cache_key_prefix = nil + it "empties all shared cache when clear_cache is called on Thing with :all option set" do + Thing.find(1) + NotTheThing.find(1) + expect { Thing.clear_cache(all: true) }.to change { read_from_cache("thing/1") }.to(nil).and( + change { read_from_cache("notthething/1", NotTheThing) }.to(nil) + ) end - it "caches with the cache_key_prefix" do - result = Thing.find(1) - read_from_cache("prefix123/thing/1").should == result + it "caches a response with the same persistence" do + result1 = Thing.find(1) + expect(result1.persisted?).to be true + result2 = Thing.find(1) + expect(result2.persisted?).to eq(result1.persisted?) end - end - - it "should cache a response for a string primary key" do - result = Thing.find("fded") - read_from_cache("thing/fded").should == result - end - - it "should cache without whitespace in keys" do - result = Thing.find(1, :from => 'path', :params => { :foo => 'bar' }) - read_from_cache('thing/1/{:from=>"path",:params=>{:foo=>"bar"}}').should == result - end - - it "should empty the cache when clear_cache is called" do - result = Thing.find(1) - Thing.clear_cache - read_from_cache("thing/1").should == nil - end - - it "should not empty the cache of NotTheThing when clear_cache is called on the Thing" do - result1 = Thing.find(1) - result2 = NotTheThing.find(1) - Thing.clear_cache - NotTheThing.send(:cache_read, 'notthething/1').should == result2 - end - it "should empty all the cache when clear_cache is called on the Thing with :all option set" do - result1 = Thing.find(1) - result2 = NotTheThing.find(1) - Thing.clear_cache(all: true) - NotTheThing.send(:cache_read, 'notthething/1').should == nil - end - - it "should cache a response with the same persistence" do - result1 = Thing.find(1) - result1.persisted?.should be true - result2 = Thing.find(1) - result1.persisted?.should == result2.persisted? - end - - it "should read a response when the request is made again" do - # make a request - Thing.find(1) - # make the same request - Thing.find(1) - # only one request should have happened - ActiveResource::HttpMock.requests.length.should == 1 - end - - it "should read a response when the request is made again for a string primary key" do - # make a request - Thing.find("fded") - # make the same request - Thing.find("fded") - # only one request should have happened - ActiveResource::HttpMock.requests.length.should == 1 - end - - it "should remake a request when reloaded" do - # make a request - Thing.find(1) - # make the same request, but reload it - Thing.find(1, :reload => true) - # we should get two requests - ActiveResource::HttpMock.requests.length.should == 2 - end - - it "should remake a request when reloaded for a string primary key" do - # make a request - Thing.find("fded") - # make the same request, but reload it - Thing.find("fded", :reload => true) - # we should get two requests - ActiveResource::HttpMock.requests.length.should == 2 - end - - it "should rewrite the cache when the request is reloaded" do - # make a request - Thing.find(1) - # get the cached result of the request - old_result = read_from_cache("thing/1") - - # change the response - ActiveResource::HttpMock.reset! - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/1.json", {}, @other_thing_json + it "remakes a request when reloaded" do + Thing.find(1) + expect { Thing.find(1, reload: true) }.to change(ActiveResource::HttpMock.requests, :length).from(1).to(2) end - Thing.find(1, :reload => true) - new_result = read_from_cache("thing/1") - # since active resources are equal if and only if they - # are the same object or an instance of the same class, - # not new?, and have the same id. - new_result.name.should_not == old_result.name - end - - it "should remake the request when the ttl expires" do - # set cache time to live to 1 second - Thing.cached_resource.ttl = 1 - # make a request - Thing.find(1) - # wait for the cache to expire - sleep(1.5) - # make the same request - Thing.find(1) - ActiveResource::HttpMock.requests.length.should == 2 - end - - it "should not return a frozen object on first request" do - result1 = Thing.find(1) - result1.should_not be_frozen - end - - it "should not return frozen object on a subsequent request" do - result1 = Thing.find(1) - result2 = Thing.find(1) - result2.should_not be_frozen - end + it "rewrites the cache when the request is reloaded" do + Thing.find(1) + old_result = read_from_cache("thing/1").dup - it "should not freeze first requested object on a subsequent request" do - result1 = Thing.find(1) - result2 = Thing.find(1) - result1.should_not be_frozen - end + ActiveResource::HttpMock.respond_to do |mock| + mock.get "/things/1.json", {}, other_thing.to_json + end - describe "when ActiveSupport.parse_json_times is enabled" do - before(:all) do - Time.zone = 'UTC' - ActiveSupport.parse_json_times = true + expect { Thing.find(1, reload: true) }.to change { read_from_cache("thing/1").name }.from(old_result.name) end - it "should convert date times to objects when reading from cache" do - Thing.find(4) - - read_from_cache("thing/4").created_at.should == @date_thing[:thing][:created_at] + it "does not return a frozen object on first request, or subsequent" do + result1 = Thing.find(1) + expect(result1).not_to be_frozen + expect(Thing.find(1)).not_to be_frozen + expect(result1).not_to be_frozen end end - shared_examples "collection_return_type" do - if ActiveResource::VERSION::MAJOR >= 4 - it "should return an ActiveResource::Collection" do - cached = read_from_cache("thing/all") - cached.should be_instance_of(ActiveResource::Collection) - end - - it "should return a chainable instance of the collection_parser" do - Thing.cached_resource.cache.clear - class CustomCollection < ActiveResource::Collection; end - Thing.collection_parser = CustomCollection - - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things.json?name=ada", {}, @thing_collection.to_json - mock.get "/things.json?major=CS&name=ada", {}, @thing_collection2.to_json - end - - non_cached = Thing.where(name: 'ada') - non_cached.original_params.should == { :name => 'ada' } - non_cached.map(&:id).should == @thing_collection.map { |h| h[:id]} - - cached = read_from_cache('thing/all/{:params=>{:name=>"ada"}}') - cached.should be_instance_of(CustomCollection) - cached.original_params.should == { :name => 'ada' } - cached.resource_class.should == Thing - cached.map(&:id).should == @thing_collection.map { |h| h[:id]} - - if ActiveResource::VERSION::MAJOR < 5 - non_cached = cached.resource_class.where(cached.original_params.merge(major: 'CS')) - else - non_cached = cached.where(major: 'CS') - end - - non_cached.original_params.should == { :name => 'ada', :major => 'CS' } - non_cached.resource_class.should == Thing - non_cached.map(&:id).should == @thing_collection2.map { |h| h[:id]} - cached = read_from_cache('thing/all/{:params=>{:name=>"ada",:major=>"cs"}}') - cached.original_params.should == { :name => 'ada', :major => 'CS' } - cached.resource_class.should == Thing - cached.map(&:id).should == @thing_collection2.map { |h| h[:id]} - end - else - it "should return an Array" do - cached = read_from_cache("thing/all") - cached.should be_instance_of(Array) - end + context "Caching a collection" do + it "does not cache an empty array response" do + Thing.find(:all, params: {name: "43"}) + expect(read_from_cache("thing/all/name/43")).to be_nil end - end - shared_examples "collection_freezing" do - it "should not return a frozen collection on first request" do - Thing.cached_resource.cache.clear - collection1 = Thing.all - collection1.should_not be_frozen + it "does not cache a nil response" do + Thing.find(:all, params: {name: "42"}) + expect(read_from_cache("thing/all/name/42")).to be_nil end - it "should not return a frozen collection on a subsequent request" do - Thing.cached_resource.cache.clear - collection1 = Thing.all - collection2 = Thing.all - collection2.should_not be_frozen + it "empties the cache when clear_cache is called" do + Thing.all + expect { Thing.clear_cache }.to change { read_from_cache("thing/all") }.from(kind_of(Enumerable)).to(nil) end - it "should not freeze first requested collection on a subsequent request" do - Thing.cached_resource.cache.clear - result1 = Thing.all - result2 = Thing.all - result1.should_not be_frozen + it "does not empty cache of other ActiveResource objects" do + Thing.all + NotTheThing.all + expect { Thing.clear_cache }.to change { read_from_cache("thing/all") }.from(kind_of(Enumerable)).to(nil).and( + not_change { read_from_cache("notthething/all", NotTheThing) } + ) end - it "should not return frozen members on first request" do - Thing.cached_resource.cache.clear - collection1 = Thing.all - collection1.first.should_not be_frozen + it "remakes a request when reloaded" do + Thing.all + expect { Thing.all(reload: true) }.to change(ActiveResource::HttpMock.requests, :length).from(1).to(2) end + end - it "should not return frozen members on a subsequent request" do - Thing.cached_resource.cache.clear - collection1 = Thing.all - collection2 = Thing.all - collection2.first.should_not be_frozen + context "Caching collection is turned off" do + before do + allow(thing_cached_resource).to receive(:cache_collections).and_return(false) end - it "should not freeze members on a subsequent request" do - Thing.cached_resource.cache.clear - collection1 = Thing.all - member1 = Thing.find(1) - collection1.first.should_not be_frozen + it "always remakes a request" do + Thing.all + expect { Thing.all }.to change(ActiveResource::HttpMock.requests, :length).from(1).to(2) end - end + context "custom collection arguments" do + before do + allow(thing_cached_resource).to receive(:collection_arguments).and_return([:all, params: {name: 42}]) + end - shared_examples "collection_cache_clearing" do - it "should empty the cache when clear_cache is called" do - Thing.clear_cache - read_from_cache("thing/all").should == nil - read_from_cache("thing/1").should == nil + it "checks for custom collection arguments" do + Thing.all + expect { Thing.find(:all, params: {name: 42}) }.to change(ActiveResource::HttpMock.requests, :length).from(1).to(2) + end end - end - describe "when collection synchronize is enabled" do - before(:each) do - Thing.cached_resource.cache.clear - Thing.cached_resource.collection_synchronize = true + context "TTL" do + let(:now) { Time.new(1999, 12, 31, 12, 0, 0) } + let(:travel_seconds) { 1000 } - ActiveResource::HttpMock.reset! - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/1.json", {}, @thing_json - mock.get "/things/fded.json", {}, @string_thing_json - mock.get "/things.json", {}, [@thing[:thing], @string_thing[:thing]].to_json(:root => :thing) - end - - # make a request for all things - Thing.all + before do + Timecop.freeze(now) + allow(thing_cached_resource).to receive(:generate_ttl).and_return(travel_seconds) end - include_examples "collection_return_type" - - include_examples "collection_freezing" - - it "should write cache entries for its members" do - result = Thing.find(1) - string_result = Thing.find("fded") - # only the all request should have been made - ActiveResource::HttpMock.requests.length.should == 1 - # the result should be cached with the appropriate key - read_from_cache("thing/1").should == result - read_from_cache("thing/fded").should == string_result + after do + Timecop.return end - include_examples "collection_cache_clearing" - - it "should rewrite cache entries for its members when reloaded" do - # get the soon to be stale result so that we have a cache entry - old_result = Thing.find(1) - old_string_result = Thing.find("fded") - # change the server - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/1.json", {}, @other_thing_json - mock.get "/things/fded.json", {}, @other_string_thing_json - mock.get "/things.json", {}, [@other_thing[:thing], @other_string_thing[:thing]].to_json(:root => :thing) - end - # reload the collection - Thing.all(:reload => true) - # get the updated result, read from the cache - result = Thing.find(1) - read_from_cache("thing/all")[0].should == result - read_from_cache("thing/all")[0].name.should == result.name - string_result = Thing.find("fded") - read_from_cache("thing/all")[1].should == string_result - read_from_cache("thing/all")[1].name.should == string_result.name + it "remakes the request when the ttl expires" do + expect { Thing.find(1) }.to change { ActiveResource::HttpMock.requests.length }.from(0).to(1) + Timecop.travel(now + travel_seconds) + expect { Thing.find(1) }.to change { ActiveResource::HttpMock.requests.length }.from(1).to(2) end + end - it "should update the collection when an individual request is reloaded" do - # change the server - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/1.json", {}, @other_thing_json - mock.get "/things/fded.json", {}, @other_string_thing_json - mock.get "/things.json", {}, [@other_thing[:thing], @other_string_thing[:thing]].to_json(:root => :thing) + context "when cache prefix is set" do + context "cache_key_prefix is a string" do + before { allow(thing_cached_resource).to receive(:cache_key_prefix).and_return("prefix123") } + it "caches with the cache_key_prefix" do + result = Thing.find(1) + expect(read_from_cache("prefix123/thing/1")).to eq(result) end - - # reload the individual - result = Thing.find(1, :reload => true) - read_from_cache("thing/all")[0].should == result - read_from_cache("thing/all")[0].name.should == result.name - string_result = Thing.find("fded", :reload => true) - read_from_cache("thing/all")[1].should == string_result - read_from_cache("thing/all")[1].name.should == string_result.name end - it "should update both the collection and the member cache entries when a subset of the collection is retrieved" do - # create cache entries for 1 and all - old_individual = Thing.find(1) - old_collection = Thing.all - - # change the server - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things.json?name=Ari", {}, [@other_thing[:thing]].to_json(:root => :thing) - mock.get "/things.json?name=Lon", {}, [@other_string_thing[:thing]].to_json(:root => :thing) + context "cache_key_prefix is a callable" do + before do + allow(thing_cached_resource).to receive(:cache_key_prefix).and_return(proc { "prefix123" }) end - # make a request for a subset of the "mother" collection - result = Thing.find(:all, :params => {:name => "Ari"}) - # the collection should be updated to reflect the server change - read_from_cache("thing/all")[0].should == result[0] - read_from_cache("thing/all")[0].name.should == result[0].name - # the individual should be updated to reflect the server change - read_from_cache("thing/1").should == result[0] - read_from_cache("thing/1").name.should == result[0].name - - # make a request for a subset of the "mother" collection - result = Thing.find(:all, :params => {:name => "Lon"}) - # the collection should be updated to reflect the server change - read_from_cache("thing/all")[1].should == result[0] - read_from_cache("thing/all")[1].name.should == result[0].name - # the individual should be updated to reflect the server change - read_from_cache("thing/fded").should == result[0] - read_from_cache("thing/fded").name.should == result[0].name - end - - it "should maintain the order of the collection when updating it" do - # change the server to return a longer collection - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things.json", {}, [@thing[:thing], @thing3[:thing], @thing2[:thing], @string_thing[:thing]].to_json(:root => :thing) + it "caches with the cache_key_prefix" do + result = Thing.find(1) + expect(read_from_cache("prefix123/thing/1")).to eq(result) + allow(thing_cached_resource).to receive(:cache_key_prefix).and_return(proc { "prefix456" }) + result = Thing.find(1, reload: true) + expect(read_from_cache("prefix456/thing/1")).to eq(result) end + end + end - # create cache entry for the collection (we reload because in before block we make an all request) - old_collection = Thing.all(:reload => true) - - # change the server's response for the thing with id 2 - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/2.json", {}, @other_thing2.to_json(:root => :thing) - mock.get "/things/fded.json", {}, @other_string_thing.to_json(:root => :thing) - end + context "when ActiveSupport.parse_json_times is enabled" do + before(:all) do + Time.zone = "UTC" + ActiveSupport.parse_json_times = true + end - # get thing 2, thereby updating the collection - result = Thing.find(2, :reload => true) - # get the updated collection from the cache - updated_collection = Thing.all - # name should have changed to "Jeb" - updated_collection[2].name.should == result.name - # the updated collection should have the elements in the same order - old_collection.each_with_index do |thing, i| - updated_collection[i].id.should == thing.id - end + after(:all) do + ActiveSupport.parse_json_times = false + Time.zone = nil + end - # get string thing, thereby updating the collection - string_result = Thing.find("fded", :reload => true) - # get the updated collection from the cache - updated_collection = Thing.all - # name should have changed to "Lon" - updated_collection[3].name.should == string_result.name - # the updated collection should have the elements in the same order - old_collection.each_with_index do |thing, i| - updated_collection[i].id.should == thing.id - end + it "returns a time object when a time attribute is present in the response" do + result = Thing.find(4) + expect(result.created_at).to be_a(Time) end end - describe "when collection synchronize is disabled" do - before(:each) do - Thing.cached_resource.cache.clear - Thing.cached_resource.collection_synchronize = false - - ActiveResource::HttpMock.reset! + context "cache_collection_synchronize" do + before do + allow(thing_cached_resource).to receive(:collection_synchronize).and_return(true) ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/1.json", {}, @thing_json - mock.get "/things/fded.json", {}, @string_thing_json - mock.get "/things.json", {}, [@thing[:thing], @string_thing[:thing]].to_json(:root => :thing) + mock.get "/things/1.json", {}, thing.to_json + mock.get "/things.json", {}, [thing2[:thing], other_string_thing[:thing]].to_json end - - # make a request for all things - Thing.all - end - - include_examples "collection_return_type" - - include_examples "collection_freezing" - - it "should not write cache entries for its members" do - result = Thing.find(1) - result = Thing.find("fded") - # both the all in the before each and this request should have been made - ActiveResource::HttpMock.requests.length.should == 3 end - include_examples "collection_cache_clearing" - - it "should not update the collection when an individual request is reloaded" do - # change the server + it "should rewrite cache entries for its members when reloaded" do + old_results = Thing.all(reload: true) + # Update response ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/1.json", {}, @other_thing_json - mock.get "/things/fded.json", {}, @other_string_thing_json - mock.get "/things.json", {}, [@other_thing[:thing], @other_string_thing[:thing]].to_json(:root => :thing) + mock.get "/things/1.json", {}, other_thing.to_json end - # reload the individual - result = Thing.find(1, :reload => true) - # the ids are the same, but the names should be different - read_from_cache("thing/all")[0].name.should_not == result.name + new_result = Thing.find(1, reload: true) - # reload the individual - string_result = Thing.find("fded", :reload => true) - # the ids are the same, but the names should be different - read_from_cache("thing/all")[1].name.should_not == string_result.name + expect(old_results).not_to include(new_result) + expect(read_from_cache("thing/all")).to include(new_result) end end - describe "when ttl randomization is enabled" do - before(:each) do - @ttl = 1 - Thing.cached_resource.ttl = @ttl - Thing.cached_resource.ttl_randomization = true - Thing.cached_resource.send(:sample_range, 1..2, @ttl) - # next ttl 1.72032449344216 + context "Cache clearing" do + it "clears the cache" do + Thing.find(1) + expect { Thing.clear_cache }.to change { read_from_cache("thing/1") }.to(nil) end + end + end - it "should generate a random ttl" do - Thing.cached_resource.cache.should_receive(:write) - Thing.cached_resource.cache.stub(:write) do |key, value, options| - # we know the ttl should not be the same as the set ttl - options[:expires_in].should_not == @ttl - end + context "when caching is disabled" do + before do + allow(thing_cached_resource).to receive(:enabled).and_return(false) + allow(not_the_thing_cached_resource).to receive(:enabled).and_return(false) + end - Thing.find(1) - end + it "does not cache a response" do + Thing.find(1) + expect(read_from_cache("thing/1")).to be_nil + end + + it "does not cache a nil response" do + Thing.find(:all, params: {name: "42"}) + expect(read_from_cache("thing/all/name/42")).to be_nil + end + it "does not cache an empty array response" do + Thing.find(:all, params: {name: "43"}) + expect(read_from_cache("thing/all/name/43")).to be_nil end end - describe "when disabled" do - before(:each) do - Thing.cached_resource.cache.clear - Thing.cached_resource.off! + describe ".cache_key_delete_pattern" do + let(:cache_class) { "Redis" } - ActiveResource::HttpMock.reset! - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things/1.json", {}, @thing_json - mock.get "/things/fded.json", {}, @string_thing_json - end + before do + allow(Thing.cached_resource).to receive(:cache).and_return(cache_class) end - it "should not cache a response" do - result = Thing.find(1) - read_from_cache("thing/1").should be_nil + context "with cache ActiveSupport::Cache::MemoryStore" do + let(:cache_class) { ActiveSupport::Cache::MemoryStore.new } + it do + expect(Thing.send(:cache_key_delete_pattern)).to eq(/^thing\//) + end end - it "should always remake the request" do - Thing.find(1) - ActiveResource::HttpMock.requests.length.should == 1 - Thing.find(1) - ActiveResource::HttpMock.requests.length.should == 2 + context "with cache ActiveSupport::Cache::FileStore" do + let(:cache_class) { ActiveSupport::Cache::FileStore.new("tmp/") } + it do + expect(Thing.send(:cache_key_delete_pattern)).to eq(/^thing\//) + end end - it "should always remake the request for a string primary key" do - Thing.find("fded") - ActiveResource::HttpMock.requests.length.should == 1 - Thing.find("fded") - ActiveResource::HttpMock.requests.length.should == 2 + context "default" do + it do + expect(Thing.send(:cache_key_delete_pattern)).to eq("thing/*") + end end end - describe "when cache_collections is disabled" do - before(:each) do - Thing.cached_resource.cache.clear - Thing.cached_resource.cache_collections = false + describe ".clear_cache" do + it "clears the cache immediately" do + expect(thing_cached_resource.cache).to receive(:delete_matched) + expect(Thing.clear_cache).to be true + expect(thing_cached_resource.logger).to have_received(:info).with(/CLEAR/) + end - ActiveResource::HttpMock.reset! - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/things.json", {}, [@thing[:thing],@string_thing[:thing]].to_json(:root => :thing) - mock.get "/things/1.json", {}, @thing_json - mock.get "/things/fded.json", {}, @string_thing_json + context "when options are provided" do + it "clears all cache if options[:all] is true" do + options = {all: true} + expect(thing_cached_resource.cache).to receive(:clear).and_call_original + Thing.clear_cache(options) end - end - it "should cache a response" do - result = Thing.find(1) - read_from_cache("thing/1").should == result + it "deletes matched cache keys if options[:all] is not provided" do + options = {all: false} + allow(thing_cached_resource.cache).to receive(:delete_matched) + Thing.clear_cache(options) + expect(thing_cached_resource.cache).to have_received(:delete_matched) + end end - it "should not remake a single request" do - result = Thing.find(1) - ActiveResource::HttpMock.requests.length.should == 1 - result = Thing.find(1) - ActiveResource::HttpMock.requests.length.should == 1 - end + context 'when using a cache store that does not support "delete_matched"' do + before do + mem_cache_store = double(:cache_store_without_delete_matched, clear: nil) + allow(thing_cached_resource).to receive(:cache).and_return(mem_cache_store) + end - it "should always remake the request for collections" do - Thing.all - ActiveResource::HttpMock.requests.length.should == 1 - Thing.all - ActiveResource::HttpMock.requests.length.should == 2 + it "clears the cache if Dalli is defined" do + expect(thing_cached_resource.cache).to receive(:clear) + Thing.clear_cache + expect(thing_cached_resource.logger).to have_received(:info).with(/CLEAR ALL/) + end end end end diff --git a/spec/cached_resource/configuration_spec.rb b/spec/cached_resource/configuration_spec.rb index 445364b..a0826be 100644 --- a/spec/cached_resource/configuration_spec.rb +++ b/spec/cached_resource/configuration_spec.rb @@ -1,25 +1,67 @@ -require 'spec_helper' +require "spec_helper" + +class Foo < ActiveResource::Base + cached_resource +end + +class Bar < ActiveResource::Base + cached_resource ttl: 1, + race_condition_ttl: 5, + cache: "cache", + logger: "logger", + enabled: false, + collection_synchronize: true, + collection_arguments: [:every], + custom: "irrelevant", + cache_collections: true +end + +class Bar2 < Bar; end + +class Bar3 < Bar + # override the superclasses configuration + self.cached_resource = CachedResource::Configuration.new(ttl: 60) +end + +describe CachedResource::Configuration do + let(:configuration) { described_class.new } + let(:default_logger) { defined?(ActiveSupport::Logger) ? ActiveSupport::Logger : ActiveSupport::BufferedLogger } -describe "CachedResource::Configuration" do + describe "#off!" do + subject { configuration } + before { subject.on! } + after { subject.off! } - let(:configuration) { CachedResource::Configuration.new } - let(:default_logger) { defined?(ActiveSupport::Logger) ? ActiveSupport::Logger : ActiveSupport::BufferedLogger } + it "sets cache off" do + expect { subject.off! }.to change { subject.enabled }.from(true).to(false) + end + end + + describe "#on!" do + subject { configuration } + before { subject.off! } + after { subject.off! } + + it "sets cache off" do + expect { subject.on! }.to change { subject.enabled }.from(false).to(true) + end + end - describe "by default" do + context "defaults" do it "should be enabled" do - configuration.enabled.should == true + expect(configuration.enabled).to eq(true) end it "should have a cache expiry of 1 week" do - configuration.ttl.should == 604800 + expect(configuration.ttl).to eq(604800) end it "should disable collection synchronization" do - configuration.collection_synchronize.should == false + expect(configuration.collection_synchronize).to eq(false) end it "should default to :all for collection arguments" do - configuration.collection_arguments.should == [:all] + expect(configuration.collection_arguments).to eq([:all]) end it "should cache collections" do @@ -28,181 +70,82 @@ describe "outside a Rails environment" do it "should be logging to a buffered logger attached to a NilIO" do - configuration.logger.class.should == default_logger + expect(configuration.logger.class).to eq(default_logger) # ActiveSupport switched around the log destination variables # Check if either are what we expect to be compatible - old_as = configuration.logger.instance_variable_get(:@log).class == NilIO - new_as = configuration.logger.instance_variable_get(:@log_dest).class == NilIO - newer_as = configuration.logger.instance_variable_get(:@logdev).instance_variable_get(:@dev).class == NilIO - (old_as || new_as || newer_as).should == true + old_as = configuration.logger.instance_variable_get(:@log).instance_of?(NilIO) + new_as = configuration.logger.instance_variable_get(:@log_dest).instance_of?(NilIO) + newer_as = configuration.logger.instance_variable_get(:@logdev).instance_variable_get(:@dev).instance_of?(NilIO) + expect(old_as || new_as || newer_as).to eq(true) end it "should cache responses in a memory store" do - configuration.cache.class.should == ActiveSupport::Cache::MemoryStore + expect(configuration.cache.class).to eq(ActiveSupport::Cache::MemoryStore) end end describe "inside a Rails environment" do before(:each) do - Rails = OpenStruct.new(:logger => "logger", :cache => "cache") - load "cached_resource/configuration.rb" - end - - after(:each) do - # remove the rails constant and unbind the - # cache and logger from the configuration - # defaults - Object.send(:remove_const, :Rails) - load "cached_resource/configuration.rb" + stub_const("Rails", double(:Rails, logger: "logger", cache: "cache")) end it "should be logging to the rails logger" do - configuration.logger.should == "logger" + expect(configuration.logger).to eq("logger") end it "should cache responses in a memory store" do - configuration.cache.should == "cache" + expect(configuration.cache).to eq("cache") end end end - describe "when initialized through cached resource" do - before(:each) do - class Foo < ActiveResource::Base - cached_resource :ttl => 1, - :race_condition_ttl => 5, - :cache => "cache", - :logger => "logger", - :enabled => false, - :collection_synchronize => true, - :collection_arguments => [:every], - :custom => "irrelevant", - :cache_collections => true - end - end - - after(:each) do - Object.send(:remove_const, :Foo) - end - + context "when initialized through cached resource" do it "should relfect the specified options" do - cr = Foo.cached_resource - cr.ttl.should == 1 + cr = Bar.cached_resource + expect(cr.ttl).to eq(1) expect(cr.race_condition_ttl).to eq(5) - cr.cache.should == "cache" - cr.logger.should == "logger" - cr.enabled.should == false - cr.collection_synchronize.should == true - cr.collection_arguments.should == [:every] - cr.custom.should == "irrelevant" - cr.cache_collections.should == true + expect(cr.cache).to eq("cache") + expect(cr.logger).to eq("logger") + expect(cr.enabled).to eq(false) + expect(cr.collection_synchronize).to eq(true) + expect(cr.collection_arguments).to eq([:every]) + expect(cr.custom).to eq("irrelevant") + expect(cr.cache_collections).to eq(true) end end - # re-evaluate - describe "when multiple are initialized through cached resource" do - before(:each) do - class Foo < ActiveResource::Base - cached_resource - end - - class Bar < ActiveResource::Base - cached_resource - end - end - - after(:each) do - Object.send(:remove_const, :Foo) - Object.send(:remove_const, :Bar) - end - + context "when multiple are initialized through cached resource" do it "they should have different configuration objects" do - Foo.cached_resource.object_id.should_not == Bar.cached_resource.object_id + expect(Foo.cached_resource.object_id).not_to eq(Bar.cached_resource.object_id) end - - it "they should have the same attributes" do - Foo.cached_resource.instance_variable_get(:@table).should == Bar.cached_resource.instance_variable_get(:@table) - end - end - describe "when cached resource is inherited" do - before(:each) do - class Bar < ActiveResource::Base - cached_resource :ttl => 1, - :race_condition_ttl => 5, - :cache => "cache", - :logger => "logger", - :enabled => false, - :collection_synchronize => true, - :collection_arguments => [:every], - :custom => "irrelevant", - :cache_collections => true - end - - class Foo < Bar - end - end - - after(:each) do - Object.send(:remove_const, :Foo) - Object.send(:remove_const, :Bar) - end - + context "when cached resource is inherited" do it "it should make sure each subclass has the same configuration" do - Bar.cached_resource.object_id.should == Foo.cached_resource.object_id + expect(Bar.cached_resource.object_id).to eq(Bar2.cached_resource.object_id) end - end - describe "when cached resource is inherited and then overriden" do - before(:each) do - class Bar < ActiveResource::Base - cached_resource :ttl => 1, - :race_condition_ttl => 5, - :cache => "cache", - :logger => "logger", - :enabled => false, - :collection_synchronize => true, - :collection_arguments => [:every], - :custom => "irrelevant", - :cache_collections => true - end - - class Foo < Bar - # override the superclasses configuration - self.cached_resource = CachedResource::Configuration.new(:ttl => 60) - end - end - - after(:each) do - Object.send(:remove_const, :Foo) - Object.send(:remove_const, :Bar) - end - - it "should have the specified options" do - Foo.cached_resource.ttl.should == 60 - end - - it "should have the default options for anything unspecified" do - cr = Foo.cached_resource - cr.cache.class.should == ActiveSupport::Cache::MemoryStore - cr.logger.class.should == default_logger - cr.enabled.should == true - cr.collection_synchronize.should == false - cr.collection_arguments.should == [:all] - cr.custom.should == nil - cr.ttl_randomization.should == false - cr.ttl_randomization_scale.should == (1..2) - cr.cache_collections.should == true + context "when cached resource is inherited and then overriden" do + it "only overwrites explicitly set options" do + cr = Bar3.cached_resource + expect(Bar3.cached_resource.ttl).to eq(60) + expect(cr.cache.class).to eq(ActiveSupport::Cache::MemoryStore) + expect(cr.logger.class).to eq(default_logger) + expect(cr.enabled).to eq(true) + expect(cr.collection_synchronize).to eq(false) + expect(cr.collection_arguments).to eq([:all]) + expect(cr.custom).to eq(nil) + expect(cr.ttl_randomization).to eq(false) + expect(cr.ttl_randomization_scale).to eq(1..2) + expect(cr.cache_collections).to eq(true) expect(cr.race_condition_ttl).to eq(86400) end - end # At the moment, not too keen on implementing some fancy # randomness validator. - describe "when ttl randomization is enabled" do + context "when ttl randomization is enabled" do before(:each) do @ttl = 1 configuration.ttl = @ttl @@ -213,8 +156,8 @@ class Foo < Bar it "it should produce a random ttl between ttl and ttl * 2" do generated_ttl = configuration.generate_ttl - generated_ttl.should_not == 10 - (@ttl..(2 * @ttl)).should include(generated_ttl) + expect(generated_ttl).not_to eq(10) + expect(@ttl..(2 * @ttl)).to include(generated_ttl) end describe "when a ttl randomization scale is set" do @@ -228,8 +171,8 @@ class Foo < Bar it "should produce a random ttl between ttl * lower bound and ttl * upper bound" do lower = @ttl * @lower upper = @ttl * @upper - (lower..upper).should include(configuration.generate_ttl) + expect(lower..upper).to include(configuration.generate_ttl) end end end -end \ No newline at end of file +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index cde1699..86a757b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,13 +1,22 @@ -require 'rubygems' -require 'bundler/setup' -require 'active_resource' -require 'active_support' -require 'active_support/time' +require "simplecov" +SimpleCov.start do + add_filter "/spec/" + add_group "Lib", "lib/" + minimum_coverage 100 + refuse_coverage_drop +end -$:.unshift(File.dirname(__FILE__) + '/../lib') -require 'cached_resource' +require "rubygems" +require "bundler/setup" +require "active_resource" +require "active_support" +require "active_support/time" +require "cached_resource" +require "support/matchers" +require "pry-byebug" +require "timecop" +RSpec::Matchers.define_negated_matcher :not_change, :change RSpec.configure do |config| # nada end - diff --git a/spec/support/matchers.rb b/spec/support/matchers.rb new file mode 100644 index 0000000..950129e --- /dev/null +++ b/spec/support/matchers.rb @@ -0,0 +1,34 @@ +require "timeout" + +RSpec::Matchers.define :eventually do |expected_matcher| + supports_block_expectations + + chain :within do |timeout_duration| + @timeout_duration = timeout_duration + end + + match do |actual| + Timeout.timeout(@timeout_duration || 5) do + until expected_matcher.matches?(actual.call) + sleep 0.5 + end + true + end + rescue Timeout::Error + false + end + + description do + description_text = "eventually #{expected_matcher.description}" + description_text += " within #{@timeout_duration} seconds" if @timeout_duration + description_text + end + + failure_message do |actual| + "expected #{actual.call} to #{description}" + end + + failure_message_when_negated do |actual| + "expected #{actual.call} not to #{description}" + end +end