(PUP-10942) Cache environments during list

When using versioned code deploys, `list` and `get` could return different
modulepaths for the same environment between the time new code was deployed and
when the environment was expired. Now the "list" method returns the cached
environment (if there is one) or the newly created environment object. In the
latter case, we cache the environment, so it can be cleared if/when clear or
clear_all are called for that environment.

Author: joshcooper

lib/puppet/environments.rb

@@ -353,7 +353,19 @@ def initialize(loader)
 
     # @!macro loader_list
     def list
-      @loader.list
+      # Evict all that have expired, in the same way as `get`
+      clear_all_expired
+
+      @loader.list.map do |env|
+        name = env.name
+        old_entry = @cache[name]
+        if old_entry
+          old_entry.value
+        else
+          add_entry(name, entry(env))
+          env
+        end
+      end
     end
 
     # @!macro loader_search_paths

spec/unit/environments_spec.rb

@@ -557,6 +557,31 @@
       end
     end
 
+    it "returns the same cached environment object for list and get methods" do
+      cached_loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader|
+        env = loader.list.find { |e| e.name == :an_environment }
+
+        expect(env).to equal(loader.get(:an_environment)) # same object
+      end
+    end
+
+    it "returns the same cached environment object for multiple list calls" do
+      cached_loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader|
+        expect(loader.list.first).to equal(loader.list.first) # same object
+      end
+    end
+
+    it "expires environments and returns a new environment object with the same value" do
+      Puppet[:environment_timeout] = "0"
+
+      cached_loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader|
+        a = loader.list.first
+        b = loader.list.first
+        expect(a).to eq(b)        # same value
+        expect(a).to_not equal(b) # not same object
+      end
+    end
+
     it "has search_paths" do
       cached_loader_from(:filesystem => [directory_tree], :directory => directory_tree.children.first) do |loader|
         expect(loader.search_paths).to eq(["file://#{directory_tree.children.first}"])
@@ -719,6 +744,16 @@
         expect(service.created_envs).to eq([:an_environment, :an_environment])
         expect(service.evicted_envs).to eq([:an_environment])
       end
+
+      it "evicts expired environments when listing" do
+        expect(service).to receive(:expired?).with(:an_environment).and_return(true)
+
+        with_environment_loaded(service) do |cached|
+          cached.list
+        end
+
+        expect(service.evicted_envs).to eq([:an_environment])
+      end
     end
 
     context '#clear' do