The current CacheControl
class has (in my opinion) a few drawbacks:
1) it is mutable - there's been times this has caused me (and others, I would assume) headaches when stored in a static final
field for re-usability
2) there's no way to remove a directive from an already-created CacheControl
Comment From: bclozel
I think I would rather try to make CacheControl immutable than introducing a builder for this. Additionally this builder is not guiding developers to typical setups and might also allow invalid combinations.
Would you like to give the immutability update a try? At first glance this should not require API changes.
Comment From: kashike
@bclozel I've pushed an initial set of changes that make CacheControl
immutable - is this along the lines of what you were looking for? Is there any sort of validation you want to introduce here (ex: disallow public & private) to disallow invalid combinations?
Comment From: snicoll
This looks good @kashike, thanks.
Comment From: snicoll
@kashike sorry but looking at how it's used downstream we've decided to revert this. It really comes as a surprise that the instance can no longer be reused as it was before. I don't know if having an immutable version of CacheControl
is worth the change but if you wan to purse this, please open a new issue so that we can discuss if there's another way.
Comment From: kashike
Hey @snicoll! I am still interested in this, but before I make another attempt it would be useful to see how CacheControl
is being used downstream - are you able to share what kind of issues were encountered?
Comment From: snicoll
See above, https://github.com/spring-projects/spring-boot/commit/7343254090dddec210bb84cd08ce1db5dedc0782 in particular. Then we had more issues testing things as well. In short, we can't really change the existing class to return a new instance if the caller has just been calling those methods and expect the instance they had created using one of the static methods to be updated.
With all we know we may not have crafted the class the way we did but given its name, I am not sure there's much we can do about it, i.e. something that wouldn't be worse in terms of readability or consistency.