Fragment cache + array payload compounds non-cached keys

[apolzon]

If you define a serializer with a fragment cache, such as

class TestSerializer < ActiveModel::Serializer
  cache key: "test", except: [:field1]
  attributes :field1, :field2
end

and then render in your controller, an array:

render json: resources, each_serializer: TestSerializer

the fragment cache's call to attribute (line 52) will continually add field1, such that if resources has 20 objects, by the end, field1 will be called 20 times for the last invocation of the TestSerializer.

I believe ActiveModel::Serializer.attribute should only concat the key onto @_attributes if it is not already present.
i.e., line 45 (in master) should be:

      @_attributes.concat [key] unless @_attributes.include? key

Also, since the @_attributes variable is at the class level, subsequent requests compound the issue even further.

[groyoh]

Nice catch! It was indeed a bug which would also happen simply by calling multiple times .attributes or .attribute with the same attribute name. I don't know if this has any consequence on the behavior itself, but it might slow the overall system since we iterate over the @_attributes variable when calling #attributes.

I created a PR to fix this (#914).

[joaomdmoura]

Tks @apolzon and @groyoh !
Indeed it was a great catch, and an important issue.
I'm running some benchmark test yet to check if it made some impact 😄

Update:
It has some good impact into fragment cache, I was looking forward to have some improvement in its performance, thank you both! ❤️