[MGNLCT-154] Once contentType file has error, changes are no longer detected Created: 27/Jul/20  Updated: 03/Sep/20  Resolved: 03/Sep/20

Status: Closed
Project: Content Types
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major
Reporter: Christopher Zimmermann Assignee: Canh Nguyen
Resolution: Obsolete Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Cloners
is cloned by MGNLUI-6182 Once contentType file has error, chan... 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:
Epic Link: Content Types phase 2
Sprint: HL & LD 10
Story Points: 0

 Description   

Timebox for investigation: 3 SP

Once a contentType file has an error which causes the app definition to be broken with a severre error, then additional changes to the contentType file will not be detected, and therefore the app will remain broken even once the error in the contentType file is fixed. 

The ContentTypes system should be improved so that even when an app definition has errors, any changes to the referenced contenttype files will be registered and applied - such that the app will work again, as would be expected.

To reproduce

  1. Add an app and contenttype definition in a light module.
  2. Check in Admincentral that the app is working.
  3. Break the contenttype definition, for example by adding a letter 'x' at the top of the file.
  4. Check in Admincentral that the app is broken. Note problem in definitions app.
  5. Fix the contentType definition.
  6. Check in AC that the app is still broken. (Note, problem still in definitions app.)
  7. Touch the app file, for example add a space at the end.
  8. Check in AC that the app is working again.

 

Note, there must aldready be some code to apply detected changes to contentTypes files to the apps, as when one makes changes to the CT that are valid, then the app is correctly updated.

 

You could use these valid files:

App file

https://git.magnolia-cms.com/projects/DEMOS/repos/website-spa-demo/browse/light-modules/spa-website-lm/apps/tours.yaml

CT file 

https://git.magnolia-cms.com/projects/DEMOS/repos/website-spa-demo/browse/light-modules/spa-website-lm/contentTypes/event.yaml



 Comments   
Comment by Robert Šiška [ 31/Aug/20 ]

canh.nguyen

From my findings, it seems that constructing of AppWithContentType throws exception when you're referencing broken but existing content-type, so the app fails to register correctly.

 

Adding provider.isValid() check to getContentTypeDefinitionProvider seems to fix the issue.

Comment by Mikaël Geljić [ 31/Aug/20 ]

+1 rsiska;

For the record canh.nguyen, we do track definition dependencies, from generated app to the content-type definition. See the DefinitionDependency usage in AppWithContentType#construct. This triggers app descriptor re-resolution upon content-type changes (or at least it's supposed to do so! )
Handling the InvalidDefinitionException would seemingly maintain that definition dependency.

Comment by Canh Nguyen [ 01/Sep/20 ]

Thanks rsiska and mgeljic

I've added provider.isValid() to the filter in getContentTypeDefinitionProvider and it worked. We don't need to reload broken apps, when we refresh the page, it will load the app and get the content type. It seems getAllProviders() of contentTypeRegistry returns both working and broken content types, and provider.get() will throw an exception in case it's the broken one.

Comment by Christopher Zimmermann [ 01/Sep/20 ]

Regarding "we do track definition dependencies, from generated app to the content-type definition"
What if the CT file is in error right at the beginning? Would that interfere? Will the solution handle this case?

Comment by Canh Nguyen [ 01/Sep/20 ]

If the content type is in error, the content app cannot be registered, so the app does not exist. In this case, we need to fix the content type and make changes to content app file to register the app.

Comment by Christopher Zimmermann [ 01/Sep/20 ]

OK, that is also not ideal. Ideally the user would never need to "touch" the content app file to get the system to reload the app. But it sounds like fixing that as well would add a lot of complexity? So should I file another ticket for that case?

Comment by Robert Šiška [ 01/Sep/20 ]

I tried the exact same scenario in ticket description (adding x line at top of the file) and it worked without touching the app definition.

Comment by Canh Nguyen [ 02/Sep/20 ]

rsiska you could try this scenario: adding x line at top of the file before starting the server. When the server is up, then delete the x line.

There's no content app, that uses the content type, in the the app launcher. You can type the URL to open the app manually, but it's not working.

Comment by Robert Šiška [ 02/Sep/20 ]

canh.nguyen

Hm, maybe we're not on the same page with the solution, because even that worked for me. Start server with x line at top of CT def. → app is not in app launcher → fix content type → logout/login → app is visible & working.

Maybe we can have a call about it tomorrow?

Comment by Christopher Zimmermann [ 02/Sep/20 ]

"What if the CT file is in error right at the beginning? " - OK if it works that's great. It was just a corner case I was concerned about.

Comment by Canh Nguyen [ 03/Sep/20 ]

Thanks rsiska.

Logout then login it works.

Comment by Christopher Zimmermann [ 03/Sep/20 ]

canh.nguyen Do the reproduction steps still produce the error?

Comment by Canh Nguyen [ 03/Sep/20 ]

czimmermann I cloned this ticket to MGNLUI-6182, and closed this one as obsolete. Next time if the fix is on a project only, I'll move the ticket to the right project instead of cloning.

Generated at Mon Feb 12 00:37:40 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.