[MGNLDATA-100] Make dependency on the cache module optional Created: 03/Apr/10  Updated: 16/Dec/13  Resolved: 16/Dec/13

Status: Closed
Project: Magnolia Data Module (closed)
Component/s: None
Affects Version/s: 1.4.2
Fix Version/s: 1.5

Type: Improvement Priority: Major
Reporter: Fabrizio Giustina Assignee: Fabrizio Giustina
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: Text File MGNLDATA-100.patch    
Issue Links:
dependency
depends upon MAGNOLIA-3178 Several modules have a dependency fro... Closed
Template:
Acceptance criteria:
Empty
Date of First Response:

 Comments   
Comment by Fabrizio Giustina [ 03/Apr/10 ]

added <optional> to the module descriptor and fixed the version handler in order to silently ignore the failure when instantiating info.magnolia.module.cache.RegisterWorkspaceForCacheFlushingTask

Comment by Magnolia International [ 04/Apr/10 ]

There are IsModuleAvailable sort of tasks (can't recall the exact class name now); please revert this and use those instead.

Comment by Fabrizio Giustina [ 05/Apr/10 ]

reworked patch using IsModuleInstalledOrRegistered task

Comment by Fabrizio Giustina [ 05/Apr/10 ]

no way, we can't use the the IsModuleInstalledOrRegistered way: RegisterWorkspaceForCacheFlushingTask needs to be passed to the constructor so it will fail with a noclassdeffound exception if the media module is not available, far before the task can check if the module is registered (remember the RegisterWorkspaceForCacheFlushingTask is the cache module).

Catching the exception is IMHO the safer way at this moment, other solutions would be:

  • using a task which is not in the cache module itself (better, since this only adds a node in the config and there is no actual cache module dependency... but we will have to move the task to core or duplicate it everywhere)
  • adding a specific if block in the moduleVersionHandler, by triggering the RegisterWorkspaceForCacheFlushingTask instantiations only when the cache module is detected: pretty bad and unsafe (note that this will also mean that the module version handler will crash in case on leftover jcr configuration for a removed cache module)

so, rolling back to the catch() solution at the moment, it makes no sense that lots of modules depends on the cache module just because they need to use a task found in the cache module when the cache module is installed...

Comment by Magnolia International [ 06/Apr/10 ]

at the very least then this should catch the specific exception and not everything else ...

Comment by Philipp Bärfuss [ 08/Apr/10 ]

I propose something like:

IsModuleInstalledOrRegistered(.., new AbstractTask(){
    execute(ctx){
        new RegisterWorkspaceForCacheFlushingTask().execute(ctx)
    }
}

This works as the instantiation will only happen on execution which will only be called if the module is installed.

Comment by Fabrizio Giustina [ 08/Apr/10 ]

This works as the instantiation will only happen on execution which will only be called if the module is installed.

it's a good solution, the only concern is that the IsModuleInstalledOrRegistered executes if it just find the /modules/cache node on jcr... in case of the cache module not being installed but with a leftover configuration all the other moduleVersionHandlers will still die with a ClassNotFound exception.

Comment by Philipp Bärfuss [ 09/Apr/10 ]

True, but I think this is an acceptable exception as removing just the jar is never enough. This would for instance also left the cache filter behind which can not be instantiated either. So unless the node /modules/<names> is removed a module is considered installed.

Comment by Jan Haderka [ 30/Sep/10 ]

Previous changes to the code in single patch file. The changes have been rolled back due to issue not being closed at the time of release.

Comment by Jan Haderka [ 19/Nov/10 ]

Fabrizio, could you have a look at this issue again and re-apply the patch and close the issue if it is done?

Generated at Mon Feb 12 05:11:21 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.