[MGNLSITE-35] SiteProvider should fallback to default site, when outside WebContext Created: 28/Aug/15  Updated: 15/Apr/16  Resolved: 23/Oct/15

Status: Closed
Project: Magnolia Site Module
Component/s: None
Affects Version/s: None
Fix Version/s: 1.0.3

Type: Bug Priority: Critical
Reporter: Milan Divilek Assignee: Sang Ngo Huu
Resolution: Fixed Votes: 0
Labels: quickwin, support
Remaining Estimate: 0d
Time Spent: 2.5d
Original Estimate: 2d

Issue Links:
Relates
supersession
supersedes MGNLEESOLR-78 IllegalStateException is thrown when ... Closed
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)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:
Sprint: Saigon 15, Basel 16
Story Points: 5

 Description   

SiteI18nContentSupport works only in WebContext. This is because info.magnolia.module.site.provider.SiteProvider is bound to info.magnolia.module.site.ExtendedAggregationState and aggregation state can be set only in WebContext.

This is problem for example with Observation or Scheduler which are working out of WebContext.

Use case:
In event listener (Observation) I get asset by using AssetProvider. JcrAssetProvider wraps asset node into I18nNodeWrapper. So then for example info.magnolia.dam.api.Asset#getTitle or info.magnolia.dam.api.Asset#getContentStream methods will fail with java.lang.IllegalStateException.

Exception in thread "Thread-234" java.lang.IllegalStateException: Expected an instance of [info.magnolia.module.site.ExtendedAggregationState] for the [info.magnolia.cms.core.AggregationState]
	at info.magnolia.module.site.provider.SiteProvider.get(SiteProvider.java:67)
	at info.magnolia.module.site.i18n.SiteI18nContentSupport.getI18nContentSupport(SiteI18nContentSupport.java:69)
	at info.magnolia.module.site.i18n.SiteI18nContentSupport.getProperty(SiteI18nContentSupport.java:124)
	at info.magnolia.module.site.i18n.SiteI18nContentSupport$$EnhancerByCGLIB$$317c544e.getProperty(<generated>)
	at info.magnolia.jcr.wrapper.I18nNodeWrapper.getProperty(I18nNodeWrapper.java:65)
	at info.magnolia.dam.jcr.JcrAsset.getContentStream(JcrAsset.java:146)
	at info.magnolia.search.solrsearchprovider.logic.indexer.SolrIndexerImpl.pull(SolrIndexerImpl.java:104)
	at info.magnolia.search.solrsearchprovider.logic.indexer.SolrIndexerImpl.addUpdate(SolrIndexerImpl.java:65)
	at info.magnolia.search.solrsearchprovider.logic.indexer.BasicSolrIndexService.index(BasicSolrIndexService.java:73)
	at info.magnolia.module.indexer.DataIndexer.init(DataIndexer.java:90)
	at info.magnolia.module.indexer.DataIndexerFactory.init(DataIndexerFactory.java:59)
	at info.magnolia.module.indexer.ContentIndexerModule.start(ContentIndexerModule.java:73)
	at info.magnolia.module.ModuleManagerImpl.startModule(ModuleManagerImpl.java:402)
	at info.magnolia.module.ModuleManagerImpl$2$1.doExec(ModuleManagerImpl.java:361)
	at info.magnolia.context.MgnlContext$VoidOp.exec(MgnlContext.java:421)
	at info.magnolia.context.MgnlContext$VoidOp.exec(MgnlContext.java:418)
	at info.magnolia.context.MgnlContext.doInSystemContext(MgnlContext.java:392)
	at info.magnolia.module.ModuleManagerImpl$2.onEvent(ModuleManagerImpl.java:356)
	at info.magnolia.cms.util.ObservationUtil$ObservationBasedDelayedExecutor$1.run(ObservationUtil.java:253)
	at info.magnolia.cms.util.DelayedExecutor$RunnableWrapper.run(DelayedExecutor.java:103)
	at EDU.oswego.cs.dl.util.concurrent.ClockDaemon$RunLoop.run(Unknown Source)
	at java.lang.Thread.run(Thread.java:745)


 Comments   
Comment by Sang Ngo Huu [ 21/Oct/15 ]

1. Investigate
I can reproduce the issue by:

  • Create CustomCommand and configure it which will get title from asset
  • Config a job in scheduler module to execute this command

2. Approach to fix
Get fix on info.magnolia.module.site.provider.SiteProvider of magnolia-site module:

  • Add new constructor, Inject Provider<SiteManager>
  • Make sure old constructor backward compatibility
  • Get default site from SiteManager if info.magnolia.cms.core.AggregationState is null

Hi ejervidalo

  • Please help me check if this is a potential approach for this case. I tried and it worked fine
  • I still don't have any clue to provide unit test for this issue. Do you have any ideas?
Comment by Espen Jervidalo [ 21/Oct/15 ]

Your solution looks correct.
If site is installed it is the primary source for i18nContentSupport. We have fallback mechanisms in place if i18n is not configured on the site, and if there is no site available through AS it should fallback to the default site. For the test-case see private message.

Update after discussions with pmundt:
First off, the fix is valid. We need to fallback to the default site.

But when dealing with Multisite, one thing we are not taking into account is the mapping from node to site. You might have configured a site definition for your asset-node with custom i18n settings and expect it to be wrapped accordingly. This will not work.
For this we would have to resolve the correct i18nContentSupport when wrapping the node.

info.magnolia.dam.jcr.JcrAssetProvider#createAsset

Asset createAsset(final Node assetNode) {
..
  i18n = siteManager.getAssignedSite(assetnode)
  return new JcrAsset(this, new I18nNodeWrapper(assetNode, i18n)); 
..

SiteManager is obviously not available in DAM.. which is also why moving site to 'core' have been discussed.

Comment by Sang Ngo Huu [ 23/Oct/15 ]

On multisite, it still works fine.

I created PR, please let me know if I'm wrong.
Thanks,

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