[MGNLIMG-159] Let NullCachingStrategy not to store image variation right under root node Created: 18/May/15  Updated: 15/Sep/15  Resolved: 08/Sep/15

Status: Closed
Project: Imaging
Component/s: None
Affects Version/s: None
Fix Version/s: 3.1.5

Type: Improvement Priority: Neutral
Reporter: Zdenek Skodik Assignee: Trang Truong
Resolution: Fixed Votes: 0
Labels: re-evaluate, support
Remaining Estimate: 0d
Time Spent: 4d
Original Estimate: 1d

Issue Links:
Relates
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Date of First Response:
Sprint: Saigon 10
Story Points: 5

 Description   

This strategy is used by all UI image generators. It might bring negative performance implications at large environments due to node locking.

edit: this CacheStrategy isn't used anymore since resolution of MGNLIMG-158 and MGNLIMG-97



 Comments   
Comment by Magnolia International [ 29/Jun/15 ]

This gets "solved" by MGNLIMG-158/MGNLIMG-97 since nothing will use the NullCachingStrategy by default anymore.
That said, if we could write a test to demonstrate the problem, it is clearly located in info.magnolia.imaging.caching.CachingImageStreamer#generateAndStore. I'd be tempted to think the whole block below delegate.serveImage() should in fact simply move to the CachingStrategy implementations, but it might not be that trivial.

Comment by Trang Truong [ 08/Sep/15 ]

It's fixed on branch #MGNLIMG-159

Comment by Magnolia International [ 09/Sep/15 ]

There was a misunderstanding. trang.truong, I pushed a change to the test on the branch which (hopefully) shows what was intended: if NullCachingStrategy is used, generated images should not be stored at all.

The silly thing is, if we fix CachingImageStreamer#generateAndStore naively, we'll be buffering generated images in byte arrays for every call. IF caching needs to be disabled, one can simply configure the storeGeneratedImages property of the ImagingServlet to true; this will however be applied to all images. There is currently no way to configure this at generator level.

zdenekskodik, since we fixed MGNLIMG-158 and MGNLIMG-97, is this still really necessary ? If not, I'd favor pushing the test anyway, but @Ignore it. CachingImageStreamer could also benefit from a couple of comments that explain the above as well.

Comment by Magnolia International [ 11/Sep/15 ]

Further changed the test: we were testing the generateAndStore method, but really, we should have been using the serveImage method (which then uses the whole job scheduling and cache loading infrastructure). That would have made apparent that we can't actually return null in generateAndStore (as per com.google.common.cache.CacheLoader#load).

What that means is that there is no way to fix this without breaking API (except if we go the ugly route and delete the generated NodeData right away).

Generated at Mon Feb 12 02:12:52 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.