cache writes should be skipped based on certain response codes and headers

[ColinDKelley]

Currently the cacheable? logic is based entirely on what is knowable from the request side.

But the cache should not be written to in some cases based on what comes back in the response. For example, here is the logic used by rack-cache:

    # Determine if the response is worth caching under any circumstance. Responses
    # marked "private" with an explicit Cache-Control directive are considered
    # uncacheable
    #
    # Responses with neither a freshness lifetime (Expires, max-age) nor cache
    # validator (Last-Modified, ETag) are considered uncacheable.
    def cacheable?
      return false unless CACHEABLE_RESPONSE_CODES.include?(status)
      return false if cache_control.no_store? || cache_control.private?
      validateable? || fresh?
    end

where

    CACHEABLE_RESPONSE_CODES = [
      200, # OK
      203, # Non-Authoritative Information
      300, # Multiple Choices
      301, # Moved Permanently
      302, # Found
      404, # Not Found
      410  # Gone
    ].to_set

no_store? and private? are cache-control: no-store or private respectively, and

    def validateable?
      headers.key?('Last-Modified') || headers.key?('ETag')
    end

    def fresh?
      ttl && ttl > 0
    end

    def ttl
      max_age - age if max_age
    end

    def max_age
      cache_control.reverse_max_age ||
        cache_control.shared_max_age ||
          cache_control.max_age ||
           (expires && (expires - date))
    end

    def expires
      headers['Expires'] && Time.httpdate(headers['Expires'])
    rescue ArgumentError
      nil
    end

    def date
      if date = headers['Date']
        Time.httpdate(date)
      else
        headers['Date'] = now.httpdate unless headers.frozen?
        now
      end
    rescue ArgumentError
      headers['Date'] = now.httpdate unless headers.frozen?
      now
    end

    def initialize(status, headers, body)
      @status = status.to_i
      @headers = Rack::Utils::HeaderHash.new(headers)
      @body = body
      @now = Time.now
      @headers['Date'] ||= @now.httpdate
    end

Also, when fixing this, it would helpful to have debug logging that indicates why the response wasn't cached.

(Aside: the mutation in the rack-cache code above is gross, and it looks like a race condition bug that it's looking at now more than once...)

[ioquatix]

This is now released.