[MGNLRES-261] Resources app: AdminCentral UI interface crashes upon attempt to expand/collapse a deleted folder Created: 26/Feb/16  Updated: 29/Mar/22  Resolved: 09/Aug/16

Status: Closed
Project: Magnolia Resources Module
Component/s: None
Affects Version/s: 2.4.3
Fix Version/s: 2.4.7, 2.5

Type: Bug Priority: Neutral
Reporter: Aleksandr Pchelintcev Assignee: Sang Ngo Huu
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: 0d
Time Spent: 4d 7h
Original Estimate: 3d

Attachments: PNG File whoops.png    
Issue Links:
Relates
relates to MAGNOLIA-6223 Provide proper API for resource chang... 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: Resource reloading without restart
Sprint: Saigon 56
Story Points: 8
Team: Nucleus

 Description   

Underlying ResourceContainer does not subscribe to ResourceOrigin change monitoring and => is rather vulnerable in the cases when it tries to operate the deleted resources, e.g. expand/collapse a deleted folder:

  • Origin throws a resource not found exception, e.g.
    Caused by: info.magnolia.resourceloader.ResourceOrigin$ResourceNotFoundException: No resource found for path /test-module in origin layered
    	at info.magnolia.resourceloader.layered.LayeredResourceOrigin.getByPath(LayeredResourceOrigin.java:105)
    	at info.magnolia.resourceloader.layered.LayeredResourceOrigin.getByPath(LayeredResourceOrigin.java:74)
    	at info.magnolia.resources.app.workbench.ResourcesContainer.getParent(ResourcesContainer.java:297)
    	... 92 more
    
  • Container finds nothing better to do than to throw a RuntimeException.
  • Since it happens (may happen) during Vaadin rendering phase - it might cause serious consequences for whole Vaadin app (Internal Error, Vaadin Session borked etc)
    See

Ways out:

  • either make sure ResourcesContainer somehow synchronises w/ the state of ResourceOrigin
  • handle missing resources without runtime exceptions (replace with stubs or smth)


 Comments   
Comment by Sang Ngo Huu [ 27/Jul/16 ]

Hi apchelintcev,

I saw that info.magnolia.resources.app.workbench.ResourcesContainer#getParent throws throw new IllegalArgumentException("Attempted to resolve the parent resource item id of non-existing (possibly removed) resource", e);, It will crash Vaadin on getDepth() function. So, there is my solution:

  • Remove that line. I also add exception handler for function setCollapsed:
@Override
    public void setCollapsed(Object itemId, boolean collapsed) {
        try {
            if (!collapsed) {
                doExpand(itemId);
            } else {
                doCollapse(itemId);
            }
        } catch (Exception e) {
            log.warn("Item {} is removed or deleted!", e);
        }
    }
  • I added these code to constructor of ResoucesContainer to make sure that data is consistent:

        origin.registerResourceChangeHandler(new ResourceChangeHandler() {
            @Override
            public void onResourceChanged(ResourceOriginChange change) {
                refresh();
            }
        });

I'm still fixing Unitest, I don't know why JcrResourceOriginFactory and LayeredResourceOriginFactory is not generated, it caused missing class issue.

Please let me know your ideas if any.

Thanks,
Sang

Comment by Aleksandr Pchelintcev [ 27/Jul/16 ]

sang.ngo yes, clearly throwing such an exception was like calling for unnecessary trouble. I think your solution is a good way to go.

Could you pls create a PR, so it's easier to follow and judge about it?

Generated at Mon Feb 12 06:48:58 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.