From 419b14e58b0a52a652ff12aa80245c82b2818a18 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Fri, 14 Sep 2018 12:54:11 +1000
Subject: [PATCH] FIX: correctly keep stylesheet cache entries

The intent from day one was to keep MAX_TO_KEEP stylesheets per target
however the DELETE statement did not perform target filtering

This meant we often deleted the wrong stylesheets from the cache
---
 app/models/stylesheet_cache.rb       | 12 ++++++++----
 spec/models/stylesheet_cache_spec.rb | 12 ++++++++++++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/app/models/stylesheet_cache.rb b/app/models/stylesheet_cache.rb
index 52f6ef337e6..b25fc51fa26 100644
--- a/app/models/stylesheet_cache.rb
+++ b/app/models/stylesheet_cache.rb
@@ -3,7 +3,8 @@ class StylesheetCache < ActiveRecord::Base
 
   MAX_TO_KEEP = 50
 
-  def self.add(target, digest, content, source_map)
+  def self.add(target, digest, content, source_map, max_to_keep: nil)
+    max_to_keep ||= MAX_TO_KEEP
     old_logger = ActiveRecord::Base.logger
 
     return false if where(target: target, digest: digest).exists?
@@ -15,16 +16,19 @@ class StylesheetCache < ActiveRecord::Base
     success = create(target: target, digest: digest, content: content, source_map: source_map)
 
     count = StylesheetCache.count
-    if count > MAX_TO_KEEP
+    if count > max_to_keep
 
       remove_lower = StylesheetCache
         .where(target: target)
-        .limit(MAX_TO_KEEP)
+        .limit(max_to_keep)
         .order('id desc')
         .pluck(:id)
         .last
 
-      DB.exec("DELETE FROM stylesheet_cache where id < :id", id: remove_lower)
+      DB.exec(<<~SQL, id: remove_lower, target: target)
+        DELETE FROM stylesheet_cache
+        WHERE id < :id AND target = :target
+      SQL
     end
 
     success
diff --git a/spec/models/stylesheet_cache_spec.rb b/spec/models/stylesheet_cache_spec.rb
index bffa8564d84..f42bc379bab 100644
--- a/spec/models/stylesheet_cache_spec.rb
+++ b/spec/models/stylesheet_cache_spec.rb
@@ -24,5 +24,17 @@ describe StylesheetCache do
       expect(StylesheetCache.first.content).to eq "c"
     end
 
+    it "it retains stylesheets for competing targets" do
+      StylesheetCache.destroy_all
+
+      StylesheetCache.add("desktop", SecureRandom.hex, "body { }", "map", max_to_keep: 2)
+      StylesheetCache.add("desktop", SecureRandom.hex, "body { }", "map", max_to_keep: 2)
+      StylesheetCache.add("mobile", SecureRandom.hex, "body { }", "map", max_to_keep: 2)
+      StylesheetCache.add("mobile", SecureRandom.hex, "body { }", "map", max_to_keep: 2)
+      StylesheetCache.add("mobile", SecureRandom.hex, "body { }", "map", max_to_keep: 2)
+
+      expect(StylesheetCache.order(:id).pluck(:target)).to eq(["desktop", "desktop", "mobile", "mobile"])
+    end
+
   end
 end