[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: |
|
||||||||
| 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
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 CT file |
| Comments |
| Comment by Robert Šiška [ 31/Aug/20 ] |
|
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! |
| Comment by Canh Nguyen [ 01/Sep/20 ] |
|
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" |
| 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 ] |
|
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 |