Refactor PageableResponse to avoid busting Ruby's constant cache (#2670)

gems/aws-sdk-core/CHANGELOG.md

@@ -1,6 +1,8 @@
 Unreleased Changes
 ------------------
 
+* Issue - Fixed `Aws::PageableResponse` invalidating Ruby's global constant cache.
+
 3.128.0 (2022-03-04)
 ------------------
 

gems/aws-sdk-core/lib/aws-sdk-core/pageable_response.rb

@@ -48,11 +48,11 @@ module Aws
   #
   module PageableResponse
 
-    def self.extended(base)
-      base.extend Enumerable
-      base.extend UnsafeEnumerableMethods
-      base.instance_variable_set("@last_page", nil)
-      base.instance_variable_set("@more_results", nil)
+    def self.apply(base)
+      base.extend Extension
+      base.instance_variable_set(:@last_page, nil)
+      base.instance_variable_set(:@more_results, nil)
+      base
     end
 
     # @return [Paging::Pager]
@@ -62,39 +62,26 @@ def self.extended(base)
     # when this method returns `false` will raise an error.
     # @return [Boolean]
     def last_page?
-      if @last_page.nil?
-        @last_page = !@pager.truncated?(self)
-      end
-      @last_page
+      # Actual implementation is in PageableResponse::Extension
     end
 
     # Returns `true` if there are more results.  Calling {#next_page} will
     # return the next response.
     # @return [Boolean]
     def next_page?
-      !last_page?
+      # Actual implementation is in PageableResponse::Extension
     end
 
     # @return [Seahorse::Client::Response]
     def next_page(params = {})
-      if last_page?
-        raise LastPageError.new(self)
-      else
-        next_response(params)
-      end
+      # Actual implementation is in PageableResponse::Extension
     end
 
     # Yields the current and each following response to the given block.
     # @yieldparam [Response] response
     # @return [Enumerable,nil] Returns a new Enumerable if no block is given.
     def each(&block)
-      return enum_for(:each_page) unless block_given?
-      response = self
-      yield(response)
-      until response.last_page?
-        response = response.next_page
-        yield(response)
-      end
+      # Actual implementation is in PageableResponse::Extension
     end
     alias each_page each
 
@@ -105,23 +92,15 @@ def each(&block)
     # @return [Seahorse::Client::Response] Returns the next page of
     #   results.
     def next_response(params)
-      params = next_page_params(params)
-      request = context.client.build_request(context.operation_name, params)
-      request.send_request
+      # Actual implementation is in PageableResponse::Extension
     end
 
     # @param [Hash] params A hash of additional request params to
     #   merge into the next page request.
     # @return [Hash] Returns the hash of request parameters for the
     #   next page, merging any given params.
     def next_page_params(params)
-      # Remove all previous tokens from original params
-      # Sometimes a token can be nil and merge would not include it.
-      tokens = @pager.tokens.values.map(&:to_sym)
-
-      params_without_tokens = context[:original_params].reject { |k, _v| tokens.include?(k) }
-      params_without_tokens.merge!(@pager.next_tokens(self).merge(params))
-      params_without_tokens
+      # Actual implementation is in PageableResponse::Extension
     end
 
     # Raised when calling {PageableResponse#next_page} on a pager that
@@ -168,5 +147,66 @@ def to_h
       end
 
     end
+
+    # The actual decorator module implementation. It is in a distinct module
+    # so that it can be used to extend objects without busting Ruby's constant cache.
+    # object.extend(mod) bust the constant cache only if `mod` contains constants of its own.
+    # @api private
+    module Extension
+
+      include Enumerable
+      include UnsafeEnumerableMethods
+
+      attr_accessor :pager
+
+      def last_page?
+        if @last_page.nil?
+          @last_page = !@pager.truncated?(self)
+        end
+        @last_page
+      end
+
+      def next_page?
+        !last_page?
+      end
+
+      def next_page(params = {})
+        if last_page?
+          raise LastPageError.new(self)
+        else
+          next_response(params)
+        end
+      end
+
+      def each(&block)
+        return enum_for(:each_page) unless block_given?
+        response = self
+        yield(response)
+        until response.last_page?
+          response = response.next_page
+          yield(response)
+        end
+      end
+      alias each_page each
+
+      private
+
+      def next_response(params)
+        params = next_page_params(params)
+        request = context.client.build_request(context.operation_name, params)
+        request.send_request
+      end
+
+      def next_page_params(params)
+        # Remove all previous tokens from original params
+        # Sometimes a token can be nil and merge would not include it.
+        tokens = @pager.tokens.values.map(&:to_sym)
+
+        params_without_tokens = context[:original_params].reject { |k, _v| tokens.include?(k) }
+        params_without_tokens.merge!(@pager.next_tokens(self).merge(params))
+        params_without_tokens
+      end
+
+    end
   end
 end

gems/aws-sdk-core/lib/aws-sdk-core/plugins/response_paging.rb

@@ -10,7 +10,7 @@ class Handler < Seahorse::Client::Handler
         def call(context)
           context[:original_params] = context.params
           resp = @handler.call(context)
-          resp.extend(PageableResponse)
+          PageableResponse.apply(resp)
           resp.pager = context.operation[:pager] || Aws::Pager::NullPager.new
           resp
         end

gems/aws-sdk-core/spec/aws/pageable_response_spec.rb

@@ -6,7 +6,7 @@ module Aws
   describe PageableResponse do
 
     def pageable(resp, pager)
-      resp.extend(PageableResponse)
+      PageableResponse.apply(resp)
       resp.pager = pager
       resp.context[:original_params] = resp.context.params.freeze
       resp
@@ -259,5 +259,17 @@ def pageable(resp, pager)
 
     end
 
+    describe '.apply' do
+
+      it 'does not bump RubyVM.stat(:global_constant_state)' do
+        skip "Only applies to MRI"  unless defined? RubyVM.stat
+
+        object = Object.new
+        expect {
+          PageableResponse.apply(object)
+        }.to_not change { RubyVM.stat(:global_constant_state) }
+      end
+    end
+
   end
 end

gems/aws-sdk-core/spec/aws/resources/collection_spec.rb

@@ -1,8 +1,5 @@
 # frozen_string_literal: true
 
-module Enumerable
-  class Enumerator; end
-end
 require_relative '../../spec_helper'
 
 module Aws