From 303f3178c9237116e6d869c1c476250887f2f205 Mon Sep 17 00:00:00 2001
From: Jeremy Woertink <jeremywoertink@gmail.com>
Date: Sun, 14 Nov 2021 15:14:41 -0800
Subject: [PATCH 1/5] Adds in the ability to store query results in cache.
 Fixes #63

---
 shard.yml              |  3 +++
 spec/queryable_spec.cr | 31 ++++++++++++++++++++++++++++++-
 spec/spec_helper.cr    |  6 ++++++
 src/avram.cr           |  1 +
 src/avram/model.cr     |  1 +
 src/avram/queryable.cr | 22 ++++++++++++++++++++--
 6 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/shard.yml b/shard.yml
index 647b060a9..5d6ea5cd4 100644
--- a/shard.yml
+++ b/shard.yml
@@ -34,6 +34,9 @@ dependencies:
   pulsar:
     github: luckyframework/pulsar
     version: ~> 0.2.2
+  lucky_cache:
+    github: luckyframework/lucky_cache
+    branch: master
 
 development_dependencies:
   ameba:
diff --git a/spec/queryable_spec.cr b/spec/queryable_spec.cr
index 85bc8700d..41848bdf3 100644
--- a/spec/queryable_spec.cr
+++ b/spec/queryable_spec.cr
@@ -32,7 +32,20 @@ end
 class PostQuery < Post::BaseQuery
 end
 
+class UserQuery
+  class_property query_counter : Int32 = 0
+
+  private def exec_query
+    @@query_counter += 1
+    super
+  end
+end
+
 describe Avram::Queryable do
+  Spec.before_each do
+    UserQuery.query_counter = 0
+  end
+
   it "can chain scope methods" do
     ChainedQuery.new.young.named("Paul")
   end
@@ -1358,7 +1371,7 @@ describe Avram::Queryable do
 
       users = UserQuery.new.group(&.age).group(&.id)
       users.query.statement.should eq "SELECT #{User::COLUMN_SQL} FROM users GROUP BY users.age, users.id"
-      users.map(&.name).should eq ["Dwight", "Michael", "Jim"]
+      users.map(&.name).sort!.should eq ["Dwight", "Jim", "Michael"]
     end
 
     it "raises an error when grouped incorrectly" do
@@ -1471,4 +1484,20 @@ describe Avram::Queryable do
       query.to_sql.should eq original_query_sql
     end
   end
+
+  describe "with query cache" do
+    it "only runs the query once" do
+      LuckyCache.temp_config(storage: LuckyCache::MemoryStore.new) do
+        UserFactory.create &.name("Amy")
+        UserQuery.query_counter.should eq(0)
+
+        UserQuery.new.name("Amy").first
+        UserQuery.new.name("Amy").first
+        user = UserQuery.new.name("Amy").first
+
+        UserQuery.query_counter.should eq(1)
+        user.name.should eq("Amy")
+      end
+    end
+  end
 end
diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr
index 8aaba2726..ac2b98812 100644
--- a/spec/spec_helper.cr
+++ b/spec/spec_helper.cr
@@ -19,8 +19,14 @@ Db::Create.new(quiet: true).run_task
 Db::Migrate.new(quiet: true).run_task
 Db::VerifyConnection.new(quiet: true).run_task
 
+LuckyCache.configure do |settings|
+  settings.storage = LuckyCache::NullStore.new
+  settings.default_duration = 1.millisecond
+end
+
 Spec.before_each do
   TestDatabase.truncate
+  LuckyCache.settings.storage.flush
 end
 
 class SampleBackupDatabase < Avram::Database
diff --git a/src/avram.cr b/src/avram.cr
index 8f153a1bd..166e4bf03 100644
--- a/src/avram.cr
+++ b/src/avram.cr
@@ -2,6 +2,7 @@ require "dexter"
 require "wordsmith"
 require "habitat"
 require "pulsar"
+require "lucky_cache"
 require "db"
 require "pg"
 require "uuid"
diff --git a/src/avram/model.cr b/src/avram/model.cr
index b297e1cc7..7feb2abaf 100644
--- a/src/avram/model.cr
+++ b/src/avram/model.cr
@@ -18,6 +18,7 @@ abstract class Avram::Model
   macro inherited
     COLUMNS = [] of Nil # types are not checked in macros
     ASSOCIATIONS = [] of Nil # types are not checked in macros
+    include LuckyCache::Cachable
   end
 
   def self.primary_key_name : Symbol?
diff --git a/src/avram/queryable.cr b/src/avram/queryable.cr
index 46dfbedb6..41a723961 100644
--- a/src/avram/queryable.cr
+++ b/src/avram/queryable.cr
@@ -10,6 +10,8 @@ module Avram::Queryable(T)
   delegate :database, :table_name, :primary_key_name, to: T
 
   macro included
+    #private property cache_duration : Time::Span = LuckyCache.settings.default_duration
+
     def self.new_with_existing_query(query : Avram::QueryBuilder)
       new.tap do |queryable|
         queryable.query = query
@@ -245,9 +247,25 @@ module Avram::Queryable(T)
     @preloads << block
   end
 
+  # TODO: https://github.com/crystal-lang/crystal/issues/11453
+  # e.g. UserQuery.new.with_cache_length(1.hour)
+  # def with_cache_length(@cache_duration : Time::Span) : self
+  #   self
+  # end
+
+  def cache_store
+    LuckyCache.settings.storage
+  end
+
+  private def cache_key : String
+    [query.statement, query.args].join(':')
+  end
+
   def results : Array(T)
-    exec_query.tap do |records|
-      preloads.each(&.call(records))
+    cache_store.fetch(cache_key, as: Array(T)) do
+      exec_query.tap do |records|
+        preloads.each(&.call(records))
+      end
     end
   end
 

From 5f14930b633f53114f43757a229c8c68026b182a Mon Sep 17 00:00:00 2001
From: Jeremy Woertink <jeremywoertink@gmail.com>
Date: Sat, 20 Nov 2021 10:26:48 -0800
Subject: [PATCH 2/5] Move the query cache setup to Fiber so it's cleared per
 HTTP request. This also circumvents the singlton setup from Habitat that
 would cause your query cache to be stored in redis or something

---
 spec/queryable_spec.cr    |  4 +++-
 spec/spec_helper.cr       | 10 ++++------
 src/avram.cr              |  1 +
 src/avram/charms/fiber.cr | 12 ++++++++++++
 src/avram/queryable.cr    |  2 +-
 5 files changed, 21 insertions(+), 8 deletions(-)
 create mode 100644 src/avram/charms/fiber.cr

diff --git a/spec/queryable_spec.cr b/spec/queryable_spec.cr
index 41848bdf3..56e76b777 100644
--- a/spec/queryable_spec.cr
+++ b/spec/queryable_spec.cr
@@ -1487,7 +1487,9 @@ describe Avram::Queryable do
 
   describe "with query cache" do
     it "only runs the query once" do
-      LuckyCache.temp_config(storage: LuckyCache::MemoryStore.new) do
+      # We're testing the actual caching
+      Fiber.current.query_cache = LuckyCache::MemoryStore.new
+      Avram.temp_config(query_cache_enabled: true) do
         UserFactory.create &.name("Amy")
         UserQuery.query_counter.should eq(0)
 
diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr
index ac2b98812..2bd030eec 100644
--- a/spec/spec_helper.cr
+++ b/spec/spec_helper.cr
@@ -19,14 +19,12 @@ Db::Create.new(quiet: true).run_task
 Db::Migrate.new(quiet: true).run_task
 Db::VerifyConnection.new(quiet: true).run_task
 
-LuckyCache.configure do |settings|
-  settings.storage = LuckyCache::NullStore.new
-  settings.default_duration = 1.millisecond
-end
-
 Spec.before_each do
   TestDatabase.truncate
-  LuckyCache.settings.storage.flush
+  # All specs seem to run on the same Fiber,
+  # so we set back to NullStore before each spec
+  # to ensure queries aren't randomly cached
+  Fiber.current.query_cache = LuckyCache::NullStore.new
 end
 
 class SampleBackupDatabase < Avram::Database
diff --git a/src/avram.cr b/src/avram.cr
index 166e4bf03..098326197 100644
--- a/src/avram.cr
+++ b/src/avram.cr
@@ -23,6 +23,7 @@ module Avram
     setting database_to_migrate : Avram::Database.class, example: "AppDatabase"
     setting time_formats : Array(String) = [] of String
     setting i18n_backend : Avram::I18nBackend = Avram::I18n.new, example: "Avram::I18n.new"
+    setting query_cache_enabled : Bool = false
   end
 
   Log            = ::Log.for(Avram)
diff --git a/src/avram/charms/fiber.cr b/src/avram/charms/fiber.cr
new file mode 100644
index 000000000..1c8416f57
--- /dev/null
+++ b/src/avram/charms/fiber.cr
@@ -0,0 +1,12 @@
+# https://crystal-lang.org/api/latest/Fiber.html
+class Fiber
+  # This is stored on Fiber so it's released after each
+  # HTTP Request.
+  property query_cache : LuckyCache::BaseStore do
+    if Avram.settings.query_cache_enabled
+      LuckyCache::MemoryStore.new
+    else
+      LuckyCache::NullStore.new
+    end
+  end
+end
diff --git a/src/avram/queryable.cr b/src/avram/queryable.cr
index 41a723961..ac3613c60 100644
--- a/src/avram/queryable.cr
+++ b/src/avram/queryable.cr
@@ -254,7 +254,7 @@ module Avram::Queryable(T)
   # end
 
   def cache_store
-    LuckyCache.settings.storage
+    Fiber.current.query_cache
   end
 
   private def cache_key : String

From 0c3ee8a6f8bee4823be5c8f3c65b8651a793868e Mon Sep 17 00:00:00 2001
From: Jeremy Woertink <jeremywoertink@gmail.com>
Date: Sat, 20 Nov 2021 10:29:52 -0800
Subject: [PATCH 3/5] Caching queries for longer periods will be done at a
 lower level cache. Remove these

---
 src/avram/queryable.cr | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/src/avram/queryable.cr b/src/avram/queryable.cr
index ac3613c60..ae7cd9302 100644
--- a/src/avram/queryable.cr
+++ b/src/avram/queryable.cr
@@ -10,8 +10,6 @@ module Avram::Queryable(T)
   delegate :database, :table_name, :primary_key_name, to: T
 
   macro included
-    #private property cache_duration : Time::Span = LuckyCache.settings.default_duration
-
     def self.new_with_existing_query(query : Avram::QueryBuilder)
       new.tap do |queryable|
         queryable.query = query
@@ -247,12 +245,6 @@ module Avram::Queryable(T)
     @preloads << block
   end
 
-  # TODO: https://github.com/crystal-lang/crystal/issues/11453
-  # e.g. UserQuery.new.with_cache_length(1.hour)
-  # def with_cache_length(@cache_duration : Time::Span) : self
-  #   self
-  # end
-
   def cache_store
     Fiber.current.query_cache
   end

From 36e69ae4d14989685ce4fb766afbb70cccb8c90d Mon Sep 17 00:00:00 2001
From: Jeremy Woertink <jeremywoertink@gmail.com>
Date: Mon, 22 Nov 2021 14:33:49 -0800
Subject: [PATCH 4/5] cache any? and select_count queryable methods. Making
 cache_key public

---
 src/avram/queryable.cr | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/src/avram/queryable.cr b/src/avram/queryable.cr
index ae7cd9302..4acdc4b87 100644
--- a/src/avram/queryable.cr
+++ b/src/avram/queryable.cr
@@ -214,10 +214,12 @@ module Avram::Queryable(T)
   end
 
   def any? : Bool
-    queryable = clone
-    new_query = queryable.query.limit(1).select("1 AS one")
-    results = database.query_one?(new_query.statement, args: new_query.args, queryable: schema_class.name, as: Int32)
-    !results.nil?
+    cache_store.fetch(cache_key, as: Bool) do
+      queryable = clone
+      new_query = queryable.query.limit(1).select("1 AS one")
+      results = database.query_one?(new_query.statement, args: new_query.args, queryable: schema_class.name, as: Int32)
+      !results.nil?
+    end
   end
 
   def none? : Bool
@@ -225,12 +227,16 @@ module Avram::Queryable(T)
   end
 
   def select_count : Int64
-    table = "(#{query.statement}) AS temp"
-    new_query = Avram::QueryBuilder.new(table).select_count
-    result = database.scalar new_query.statement, args: query.args, queryable: schema_class.name
-    result.as(Int64)
-  rescue e : DB::NoResultsError
-    0_i64
+    cache_store.fetch(cache_key, as: Int64) do
+      begin
+        table = "(#{query.statement}) AS temp"
+        new_query = Avram::QueryBuilder.new(table).select_count
+        result = database.scalar new_query.statement, args: query.args, queryable: schema_class.name
+        result.as(Int64)
+      rescue e : DB::NoResultsError
+        0_i64
+      end
+    end
   end
 
   def each
@@ -249,7 +255,7 @@ module Avram::Queryable(T)
     Fiber.current.query_cache
   end
 
-  private def cache_key : String
+  def cache_key : String
     [query.statement, query.args].join(':')
   end
 

From f8d6f2af69f4db858462ca074f1fd0df0de1fb90 Mon Sep 17 00:00:00 2001
From: Jeremy Woertink <jeremywoertink@gmail.com>
Date: Mon, 29 Nov 2021 11:42:49 -0800
Subject: [PATCH 5/5] lock lucky_cache in to specific version

---
 shard.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/shard.yml b/shard.yml
index 5d6ea5cd4..c5be7a213 100644
--- a/shard.yml
+++ b/shard.yml
@@ -36,7 +36,7 @@ dependencies:
     version: ~> 0.2.2
   lucky_cache:
     github: luckyframework/lucky_cache
-    branch: master
+    version: ~> 0.1.0
 
 development_dependencies:
   ameba: