From be654bbabad34c508d0500b60b415709ef78651e Mon Sep 17 00:00:00 2001 From: Michael Anstis Date: Wed, 21 Dec 2016 15:16:42 +0000 Subject: [PATCH] GUVNOR-2807: File Explorer in Admin view freezes when a project is extended. (#397) (cherry picked from commit f600932648223403e7313efcd83891d60c38df2c) --- .../fileexplorer/FileExplorerItem.java | 6 +- .../fileexplorer/FileExplorerViewImpl.java | 20 +-- .../FileExplorerViewImplTest.java | 153 ++++++++++++++++++ 3 files changed, 167 insertions(+), 12 deletions(-) create mode 100644 guvnor-structure/guvnor-structure-client/src/test/java/org/guvnor/structure/client/editors/fileexplorer/FileExplorerViewImplTest.java diff --git a/guvnor-structure/guvnor-structure-client/src/main/java/org/guvnor/structure/client/editors/fileexplorer/FileExplorerItem.java b/guvnor-structure/guvnor-structure-client/src/main/java/org/guvnor/structure/client/editors/fileexplorer/FileExplorerItem.java index 1cc2cfe24d..c408bf6996 100644 --- a/guvnor-structure/guvnor-structure-client/src/main/java/org/guvnor/structure/client/editors/fileexplorer/FileExplorerItem.java +++ b/guvnor-structure/guvnor-structure-client/src/main/java/org/guvnor/structure/client/editors/fileexplorer/FileExplorerItem.java @@ -16,7 +16,7 @@ package org.guvnor.structure.client.editors.fileexplorer; - +import org.guvnor.structure.client.resources.i18n.CommonConstants; import org.uberfire.backend.vfs.Path; import org.uberfire.ext.widgets.core.client.tree.TreeItem; @@ -24,7 +24,7 @@ class FileExplorerItem { - private static final String LAZY_LOAD = "Loading..."; + private CommonConstants constants = CommonConstants.INSTANCE; private final TreeItem parent; @@ -37,7 +37,7 @@ public void addDirectory( final Path child ) { final TreeItem newDirectory = parent.addItem( TreeItem.Type.FOLDER, child.getFileName() ); newDirectory.addItem( TreeItem.Type.LOADING, - LAZY_LOAD ); + constants.Loading() ); newDirectory.setUserObject( child ); } diff --git a/guvnor-structure/guvnor-structure-client/src/main/java/org/guvnor/structure/client/editors/fileexplorer/FileExplorerViewImpl.java b/guvnor-structure/guvnor-structure-client/src/main/java/org/guvnor/structure/client/editors/fileexplorer/FileExplorerViewImpl.java index dc45b575a8..bff287f3ed 100644 --- a/guvnor-structure/guvnor-structure-client/src/main/java/org/guvnor/structure/client/editors/fileexplorer/FileExplorerViewImpl.java +++ b/guvnor-structure/guvnor-structure-client/src/main/java/org/guvnor/structure/client/editors/fileexplorer/FileExplorerViewImpl.java @@ -37,14 +37,13 @@ public class FileExplorerViewImpl extends Composite implements FileExplorerView { - private CommonConstants constants = CommonConstants.INSTANCE; TreeItem rootTreeItem = null; - private final Tree tree = GWT.create(Tree.class); + private final Tree tree = GWT.create( Tree.class ); - private final FlowPanel panel = GWT.create(FlowPanel.class); + private final FlowPanel panel = GWT.create( FlowPanel.class ); private final Map repositoryToTreeItemMap = new HashMap(); @@ -57,15 +56,15 @@ public void init( final FileExplorerPresenter presenter ) { rootTreeItem = tree.addItem( TreeItem.Type.FOLDER, constants.Repositories() ); rootTreeItem.setState( TreeItem.State.OPEN ); - panel.getElement().getStyle().setFloat(Style.Float.LEFT); - panel.getElement().getStyle().setWidth(100, Style.Unit.PCT); + panel.getElement().getStyle().setFloat( Style.Float.LEFT ); + panel.getElement().getStyle().setWidth( 100, Style.Unit.PCT ); panel.add( tree ); initWidget( panel ); tree.addOpenHandler( new OpenHandler() { @Override public void onOpen( final OpenEvent event ) { - if ( needsLoading( event.getTarget() ) && event.getTarget().getUserObject() instanceof Path ) { + if ( needsLoading( event.getTarget() ) ) { presenter.loadDirectoryContent( new FileExplorerItem( event.getTarget() ), (Path) event.getTarget().getUserObject() ); } } @@ -109,7 +108,7 @@ public void removeRepository( final Repository repo ) { @Override public void addNewRepository( final Repository repository, - final String branch) { + final String branch ) { final TreeItem repositoryRootItem = rootTreeItem.addItem( TreeItem.Type.FOLDER, repository.getAlias() ); repositoryRootItem.setUserObject( repository ); @@ -124,8 +123,11 @@ public void addNewRepository( final Repository repository, repository.getBranchRoot( branch ) ); } - private boolean needsLoading( final TreeItem item ) { - return item.getChildCount() == 1 && constants.Loading().equals( item.getChild( 0 ).getText() ); + boolean needsLoading( final TreeItem item ) { + return item.getUserObject() instanceof Path + && item.getType() == TreeItem.Type.FOLDER + && item.getChildCount() == 1 + && constants.Loading().equals( item.getChild( 0 ).getText() ); } } \ No newline at end of file diff --git a/guvnor-structure/guvnor-structure-client/src/test/java/org/guvnor/structure/client/editors/fileexplorer/FileExplorerViewImplTest.java b/guvnor-structure/guvnor-structure-client/src/test/java/org/guvnor/structure/client/editors/fileexplorer/FileExplorerViewImplTest.java new file mode 100644 index 0000000000..ecd9c23230 --- /dev/null +++ b/guvnor-structure/guvnor-structure-client/src/test/java/org/guvnor/structure/client/editors/fileexplorer/FileExplorerViewImplTest.java @@ -0,0 +1,153 @@ +/* + * Copyright 2016 Red Hat, Inc. and/or its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.guvnor.structure.client.editors.fileexplorer; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import com.google.gwtmockito.GwtMockitoTestRunner; +import org.guvnor.structure.client.resources.i18n.CommonConstants; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.uberfire.backend.vfs.Path; +import org.uberfire.ext.widgets.core.client.tree.TreeItem; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; + +@RunWith(GwtMockitoTestRunner.class) +public class FileExplorerViewImplTest { + + @Mock + private FileExplorerPresenter presenter; + + private TreeItem item; + + private FileExplorerViewImpl view; + + @Before + public void setUp() { + view = new FileExplorerViewImpl(); + view.init( presenter ); + } + + @Test + public void checkItemsAreNotLazyLoaded() { + item = newTreeItem( new TreeItemData( TreeItem.Type.ITEM, + "file", + mock( Path.class ) ) ); + assertFalse( view.needsLoading( item ) ); + } + + @Test + public void checkFoldersWithNoChildrenAreNotLazyLoaded() { + item = newTreeItem( new TreeItemData( TreeItem.Type.FOLDER, + "folder", + mock( Path.class ) ) ); + assertFalse( view.needsLoading( item ) ); + } + + @Test + public void checkFoldersWithExistingChildrenAreNotLazyLoaded() { + item = newTreeItem( new TreeItemData( TreeItem.Type.FOLDER, + "folder", + mock( Path.class ) ), + new TreeItemData( TreeItem.Type.ITEM, + "file1", + mock( Path.class ) ), + new TreeItemData( TreeItem.Type.ITEM, + "file2", + mock( Path.class ) ) ); + assertFalse( view.needsLoading( item ) ); + } + + @Test + public void checkFoldersWithLazyFlagAreLazyLoaded() { + item = newTreeItem( new TreeItemData( TreeItem.Type.FOLDER, + "folder", + mock( Path.class ) ), + new TreeItemData( TreeItem.Type.ITEM, + CommonConstants.INSTANCE.Loading(), + mock( Path.class ) ) ); + assertTrue( view.needsLoading( item ) ); + } + + private class TreeItemData { + + TreeItem.Type type; + String value; + Path path; + + TreeItemData( final TreeItem.Type type, + final String value, + final Path path ) { + this.type = type; + this.value = value; + this.path = path; + } + } + + private TreeItem newTreeItem( TreeItemData parent, + TreeItemData... children ) { + final List cti = new ArrayList<>(); + + final TreeItem item = new TreeItem( parent.type, + parent.value ) { + + @Override + public int getChildCount() { + return cti.size(); + } + + @Override + public TreeItem getChild( int i ) { + return cti.get( i ); + } + + @Override + public Iterable getChildren() { + return cti; + } + + @Override + protected TreeItem makeChild( final Type type, + final String value ) { + return new TreeItem( type, + value ) { + @Override + public String getText() { + return value; + } + }; + } + }; + item.setUserObject( parent.path ); + + Arrays.asList( children ).stream().forEach( ( c ) -> { + final TreeItem ti = item.addItem( c.type, + c.value ); + ti.setUserObject( c.path ); + cti.add( ti ); + } ); + + return item; + } + +}