[richfaces-svn-commits] JBoss Rich Faces SVN: r13267 - trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager.

Author: nbelaevski Date: 2009-03-27 11:51:22 -0400 (Fri, 27 Mar 2009) New Revision: 13267 Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/AlbumManager.java trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/Authenticator.java trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/Controller.java trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/DnDManager.java trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/ImageManager.java Log: Code review results committed Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/AlbumManager.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/AlbumManager.java 2009-03-27 15:40:29 UTC (rev 13266) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/AlbumManager.java 2009-03-27 15:51:22 UTC (rev 13267) @@ -57,10 +57,13 @@ public void addAlbum(Album album){ if(album.getShelf() == null){ + //TODO nick - remove null argument + //TODO nick - externalize message facesMessages.add("Shelf must be not-null", null); return; } albumAction.addAlbum(album); + //TODO nick - addDirectory() uses this.album, do we need: this.album = album? addDirectory(); model.resetModel(model.getMainArea(), model.getSelectedUser(), model.getSelectedShelf(), album, null); Events.instance().raiseEvent("albumAdded"); @@ -83,7 +86,9 @@ //Update domain model String string = album.getId().toString(); albumAction.deleteAlbum(album); + //TODO nick - should be deleteDirectory(album). And call file manager directly deleteDirectory(string); + //TODO nick - why not call just model.setAlbum(null). More: is deleted album the ones currently selected in model? model.resetModel(NavigationEnum.ALL_ALBUMS, model.getSelectedUser(), model.getSelectedShelf(), null, null); Events.instance().raiseEvent("albumDeleted"); Events.instance().raiseEvent("clearTree"); Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/Authenticator.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/Authenticator.java 2009-03-27 15:40:29 UTC (rev 13266) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/Authenticator.java 2009-03-27 15:51:22 UTC (rev 13267) @@ -31,7 +31,6 @@ import org.jboss.seam.annotations.Out; import org.jboss.seam.annotations.Scope; import org.jboss.seam.annotations.web.RequestParameter; -import org.jboss.seam.contexts.Contexts; import org.jboss.seam.core.Conversation; import org.jboss.seam.faces.FacesMessages; import org.jboss.seam.log.Log; @@ -79,6 +78,7 @@ public String login(){ if(identity.hasRole("admin")){ + //TODO nick - return null return ""; } String password = credentials.getPassword(); @@ -88,9 +88,10 @@ credentials.setUsername(username); try { identity.authenticate(); + //TODO nick - maybe clearShelves() before trying to aunthenticate? shelfManager.clearShelfs(); } catch (LoginException e) { - // TODO Auto-generated catch block + // TODO nick - Auto-generated catch block e.printStackTrace(); } return "main"; @@ -99,11 +100,13 @@ public boolean authenticate() { if (wantLoginAnonymous()) { + //TODO nick - remove another roles? identity.addRole(Constants.GUEST_ROLE); model.setMainArea(NavigationEnum.ANONYM); return true; } try { + //TODO nick - move password holder into userAction user = userAction.login(credentials.getUsername(), passwordHolder.hash(credentials.getPassword())); if (user != null) { identity.addRole(Constants.ADMIN_ROLE); @@ -151,6 +154,7 @@ } public String startConversation(){ + //TODO nick - rename to destroyConversation()? identity.logout(); identity.unAuthenticate(); credentials.clear(); Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/Controller.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/Controller.java 2009-03-27 15:40:29 UTC (rev 13266) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/Controller.java 2009-03-27 15:51:22 UTC (rev 13267) @@ -53,6 +53,7 @@ private User user; public void selectShelfs(){ + //TODO nick - i think model should reset selected albums, etc. internally model.resetModel(NavigationEnum.ALL_SHELFS, user, null, null, null); } @@ -73,6 +74,7 @@ } public void cancelEditShelf(Shelf shelf){ + //TODO nick - unused "shelf" argument model.setMainArea(NavigationEnum.SHELF_PREVIEW); } @@ -90,6 +92,7 @@ return; } model.resetModel(NavigationEnum.ALBUM_IMAGE_PREVIEW, image.getAlbum().getOwner(), image.getAlbum().getShelf(), image.getAlbum(), image); + //TODO nick - model should already has enough information to set up image index model.setSelectedImageIndex(model.getSelectedAlbum().getIndex(model.getSelectedImage())+1); image.getAlbum().getShelf().visitImage(image); image.getAlbum().visitImage(image); @@ -101,6 +104,7 @@ Image image = model.getSelectedAlbum().getImages().get(index - 1); model.resetModel(NavigationEnum.ALBUM_IMAGE_PREVIEW, image.getAlbum().getOwner(), image.getAlbum().getShelf(), image.getAlbum(), image); image.getAlbum().visitImage(image); + //TODO nick - album can notify shelf image.getAlbum().getShelf().visitImage(image); } @@ -109,10 +113,12 @@ pushEvent(Constants.ADD_ERROR_EVENT, new Exception(HAVENT_ACCESS)); return; } + //TODO nick - resetModel? model.setMainArea(NavigationEnum.ALBUM_IMAGE_EDIT); } public void cancelEditImage(Image image){ + //TODO nick - unused image argument model.setMainArea(NavigationEnum.ALBUM_IMAGE_PREVIEW); } @@ -124,6 +130,7 @@ if(image == null || image.getAlbum() == null || image.getAlbum().getOwner() == null){ return false; } + //TODO nick - compare User objects? return image.getAlbum().getOwner().getLogin().equals(user.getLogin()); } @@ -132,11 +139,13 @@ pushEvent(Constants.ADD_ERROR_EVENT, new Exception(HAVENT_ACCESS)); return; } + //TODO nick - resetModel? model.setSelectedAlbum(album); model.setMainArea(NavigationEnum.ALBUM_EDIT); } public void cancelEditAlbum(Album album){ + //TODO nick - unused "album" argument model.setMainArea(NavigationEnum.ALBUM_PREVIEW); } @@ -158,6 +167,7 @@ } public boolean isUserShelf(Shelf shelf){ + //TODO nick - compare User objects? return shelf.getOwner()!=null && shelf.getOwner().getLogin().equals(user.getLogin()); } @@ -165,6 +175,7 @@ if(album == null || album.getOwner() == null){ return false; } + //TODO nick - compare User objects? return album.getOwner().getLogin().equals(user.getLogin()); } @@ -181,6 +192,7 @@ } public void showTag(MetaTag metatag){ + //TODO nick - model.setMainArea()? model.resetModel(NavigationEnum.TAGS, model.getSelectedUser(), model.getSelectedShelf(), model.getSelectedAlbum(), model.getSelectedImage()); model.setSelectedTag(metatag); } @@ -205,6 +217,7 @@ if(selectedUser == null){ return false; } + //TODO nick - equals()? if(selectedUser == user){ return true; } Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/DnDManager.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/DnDManager.java 2009-03-27 15:40:29 UTC (rev 13266) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/DnDManager.java 2009-03-27 15:51:22 UTC (rev 13267) @@ -52,11 +52,13 @@ } handleImage((Image)dragValue, (Album)dropValue); }else if(dragValue instanceof Album){ + //TODO nick - check user handleAlbum((Album)dragValue, (Shelf)dropValue); } } private void handleAlbum(Album dragValue, Shelf dropValue) { + //TODO nick - add check for the current shelf dropValue.addAlbum(dragValue); albumAction.editAlbum(dragValue); Events.instance().raiseEvent("clearTree"); @@ -74,8 +76,10 @@ private String getNewPathOfImage(Image dragValue, Album dropValue) { String fileNameOld = dragValue.getPath(); + //TODO nick - is "/" correct for windows? int lastIndexOf = dragValue.getPath().lastIndexOf("/"); String prevPathEnd = dragValue.getPath().substring(lastIndexOf); + //TODO nick - file manager should handle naming!!! String fileNameNew = user.getLogin()+"/"+dropValue.getId()+"/"+prevPathEnd; fileManager.renameImage(fileNameOld, fileNameNew); return fileNameNew; Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/ImageManager.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/ImageManager.java 2009-03-27 15:40:29 UTC (rev 13266) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/ImageManager.java 2009-03-27 15:51:22 UTC (rev 13267) @@ -58,8 +58,11 @@ private User user; public void deleteImage(Image image) { + //TODO nick - should we check image owner? String imagePath = image.getPath(); model.resetModel(NavigationEnum.ALBUM_PREVIEW, image.getAlbum().getOwner(), image.getAlbum().getShelf(), image.getAlbum(), null); + + //TODO nick - imageAction can call file manager itself imageAction.deleteImage(image); fileManager.deleteImage(imagePath); Events.instance().raiseEvent("imageDeleted"); @@ -71,6 +74,7 @@ Events.instance().raiseEvent("imageEdited"); } + //TODO nick - Constants.ADD_IMAGE_EVENT is not used anywhere else @Observer(Constants.ADD_IMAGE_EVENT) public void addImage(Image image) { imageAction.addImage(image);