Author: nbelaevski Date: 2009-03-25 19:09:12 -0400 (Wed, 25 Mar 2009) New Revision: 13202 Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/FileManager.java trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/startup/CopyImageStuff.java trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/tags/RealworldCommandButtonRenderer.java trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/tags/RealworldImageScrollerRenderer.java trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/ui/FileItem.java trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/ui/FileUploadBean.java trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/ui/ImageSizeHelper.java trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/ui/UserPrefsHelper.java trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/DirectLinkHelper.java trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/ErrorHandlerBean.java trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/FileUtils.java trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/ImageLoader.java trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/test.java Log: Code review results committed Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/FileManager.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/FileManager.java 2009-03-25 20:07:42 UTC (rev 13201) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/manager/FileManager.java 2009-03-25 23:09:12 UTC (rev 13202) @@ -63,6 +63,7 @@ private File uploadRoot; private String uploadRootPath; + //TODO nick - i suggest to hide this from external services at all public File getUploadRoot() { return uploadRoot; } Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/startup/CopyImageStuff.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/startup/CopyImageStuff.java 2009-03-25 20:07:42 UTC (rev 13201) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/startup/CopyImageStuff.java 2009-03-25 23:09:12 UTC (rev 13202) @@ -32,6 +32,7 @@ static final String IMAGE_FOLDER = "/Upload"; + //TODO nick - change constant value to include "richfaces" static final String REALWORLD_FOLDER = "realworld"; @Out(scope = ScopeType.APPLICATION) @@ -52,12 +53,15 @@ @Destroy public void destroy()throws IOException { + //TODO nick - delete even data is stored in DB? FileUtils.deleteDirectory(uploadRoot); } void resolveImageFolder() { + //TODO nick - what this cast is for? URLClassLoader loader = (URLClassLoader)getClass().getClassLoader(); + //TODO nick - rewrite that URL path = loader.getResource(""); String classLoadPath = null; String realPath = null; Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/tags/RealworldCommandButtonRenderer.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/tags/RealworldCommandButtonRenderer.java 2009-03-25 20:07:42 UTC (rev 13201) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/tags/RealworldCommandButtonRenderer.java 2009-03-25 23:09:12 UTC (rev 13202) @@ -17,6 +17,7 @@ * @author Andrey Markavtsov * */ +//TODO nick - remove this public class RealworldCommandButtonRenderer extends AjaxCommandRendererBase { @Override Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/tags/RealworldImageScrollerRenderer.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/tags/RealworldImageScrollerRenderer.java 2009-03-25 20:07:42 UTC (rev 13201) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/tags/RealworldImageScrollerRenderer.java 2009-03-25 23:09:12 UTC (rev 13202) @@ -16,6 +16,7 @@ * @author Andrey Markavtsov * */ +//TODO nick - remove this public class RealworldImageScrollerRenderer extends DatascrollerTemplate { @Override Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/ui/FileItem.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/ui/FileItem.java 2009-03-25 20:07:42 UTC (rev 13201) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/ui/FileItem.java 2009-03-25 23:09:12 UTC (rev 13202) @@ -24,6 +24,7 @@ import org.richfaces.realworld.domain.Image; +//TODO nick - no references - dead class? public class FileItem { private Image image = new Image(); Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/ui/FileUploadBean.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/ui/FileUploadBean.java 2009-03-25 20:07:42 UTC (rev 13201) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/ui/FileUploadBean.java 2009-03-25 23:09:12 UTC (rev 13202) @@ -103,9 +103,11 @@ String newPath = "/" +model.getSelectedAlbum().getOwner().getLogin() + "/" + model.getSelectedAlbum().getId() + "/" +item.getFileName(); image.setPath(newPath); fileWrapper.getFiles().add(0, image); + //TODO nick - file manager should be protected from another users accessing images of another people if(!fileManager.addImage(newPath, item.getFile().getPath())){ Events.instance().raiseEvent(Constants.ADD_ERROR_EVENT, new Exception(FILE_SAVE_ERROR)); } + //TODO nick - what's the point of storing image if image file save failed? imageAction.addImage(image, flushStrategy.isDatabaseStoreStrategy()); Events.instance().raiseEvent("imageAdded"); item.getFile().delete(); @@ -124,6 +126,7 @@ private void extractMetadata(UploadItem item, Image image) throws JpegProcessingException, MetadataException, FileNotFoundException { InputStream in = new FileInputStream(item.getFile()); + //TODO nick - close "in"? Metadata metadata = JpegMetadataReader.readMetadata(in); Directory exifDirectory = metadata.getDirectory(ExifDirectory.class); Directory jpgDirectory = metadata.getDirectory(JpegDirectory.class); Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/ui/ImageSizeHelper.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/ui/ImageSizeHelper.java 2009-03-25 20:07:42 UTC (rev 13201) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/ui/ImageSizeHelper.java 2009-03-25 23:09:12 UTC (rev 13202) @@ -17,6 +17,7 @@ static final int DEFAULT_VALUE = 120; + //TODO nick - why not use dimension.x? int value = DEFAULT_VALUE; public static enum ImageDimension { @@ -38,6 +39,7 @@ this.x = x; this.bgStyle = String.format(IMAGE_BG_STYLE, x + 20); cssClass = CSS_CLASS + x; + //TODO nick - what if x == 160? imageBgSrc = String.format(IMAGE_BG, (x == 160) ? 200 : x); filePostfix = FILE_POSTFIX + x; @@ -71,6 +73,7 @@ } } + //TODO nick - does enum really have 120th value? return values()[DEFAULT_VALUE]; } }; Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/ui/UserPrefsHelper.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/ui/UserPrefsHelper.java 2009-03-25 20:07:42 UTC (rev 13201) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/ui/UserPrefsHelper.java 2009-03-25 23:09:12 UTC (rev 13202) @@ -37,6 +37,7 @@ @In FileManager fileManager; @In FlushStrategy flushStrategy; + //TODO nick - not used anywhere @In(create=true) @Out Model model; @In User user; @@ -53,13 +54,16 @@ public void uploadAvatar(UploadEvent event) { UploadItem item = event.getUploadItem(); avatarData = item.getFile(); + //TODO nick - persist user? user.setHasAvatar(true); } + //TODO nick - are we saving current user? public void saveUser(User user){ if (avatarData != null) { try { BufferedImage image = fileManager.bitmapToImage(avatarData.getPath(),"JPG" ); + //TODO nick - use fileManager methods to access user image fileManager.imageToBitmap(image, fileManager.getUploadRoot().getCanonicalPath()+ "/" + user.getLogin() + "/avatar.jpg", "JPG"); } catch (IOException e) { @@ -73,6 +77,7 @@ } public void cancel() { + //TODO nick - restore value of user.hasAvatar? avatarData = null; } Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/DirectLinkHelper.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/DirectLinkHelper.java 2009-03-25 20:07:42 UTC (rev 13201) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/DirectLinkHelper.java 2009-03-25 23:09:12 UTC (rev 13202) @@ -29,6 +29,7 @@ @Name("directLink") @Scope(ScopeType.SESSION) @AutoCreate +//TODO nick - session-scoped bean depending on request parameters? i'm in doubts... public class DirectLinkHelper { @In(value="entityManager") protected EntityManager em; @@ -48,7 +49,9 @@ public synchronized void paintImage(OutputStream out, Object data) throws IOException { Image im = em.find(Image.class, id); + //TODO nick - '&&' so only admins can see their own unshared images? if(im.getAlbum().getShelf().isShared() || (identity.hasRole("admin") && im.getAlbum().getOwner().getLogin().equals(identity.getUsername()))){ + //TODO nick - copy-pasted 99% from ImageLoader File imageResource = fileManager.getImage(im.getPath()); if (imageResource != null) { FileInputStream fileInputStream = new FileInputStream(imageResource); @@ -93,6 +96,7 @@ return null; } + //TODO nick - see org.ajax4jsf.renderkit.RendererUtils#encodeResourceURL(TemplateContext, Object) Object request = extc.getRequest(); if (request instanceof HttpServletRequest) { Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/ErrorHandlerBean.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/ErrorHandlerBean.java 2009-03-25 20:07:42 UTC (rev 13201) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/ErrorHandlerBean.java 2009-03-25 23:09:12 UTC (rev 13202) @@ -60,6 +60,7 @@ public void addToErrors(Exception e){ FacesContext context = FacesContext.getCurrentInstance(); AjaxContext ac = AjaxContext.getCurrentInstance(context); + //TODO nick - should add to AjaxContext#getAjaxAreasToRender() ac.addRenderedArea(Constants.ERROR_ID); errors.add(e); } Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/FileUtils.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/FileUtils.java 2009-03-25 20:07:42 UTC (rev 13201) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/FileUtils.java 2009-03-25 23:09:12 UTC (rev 13202) @@ -10,6 +10,8 @@ * * @author Marco Schmidt */ + +//TODO nick - COPYRIGHT!!! public class FileUtils { // constant values for the override option public static final int OVERWRITE_ALWAYS = 1; @@ -25,6 +27,7 @@ public static Long copyFile(File srcFile, File destFile) throws IOException { if(!srcFile.getPath().toLowerCase().endsWith("jpg") && !srcFile.getPath().toLowerCase().endsWith("jpeg")){ + //TODO nick - should be return null return -1L; } InputStream in = new FileInputStream(srcFile); @@ -43,6 +46,8 @@ } out.write(buffer, 0, bytesRead); } + + //TODO nick - this should be in finally block out.close(); in.close(); if (clock) { @@ -188,6 +193,7 @@ public static void copyDirectory(File srcDir, File dstDir) throws IOException { + //TODO nick - skip hidden/system directories if (".svn".equals(srcDir.getName())) { return; } @@ -212,6 +218,7 @@ if (dir.exists()) { File [] children = dir.listFiles(); for (int i = 0; i < children.length; i++) { + //TODO nick - add try/finally so that deletion continues if something failed deleteDirectory(children[i]); } } Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/ImageLoader.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/ImageLoader.java 2009-03-25 20:07:42 UTC (rev 13201) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/ImageLoader.java 2009-03-25 23:09:12 UTC (rev 13202) @@ -41,6 +41,7 @@ @Name("imageLoader") @Scope(ScopeType.CONVERSATION) +//TODO nick - why is this conversation-scoped? public class ImageLoader implements Serializable { private static final long serialVersionUID = -1572789608594870285L; Modified: trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/test.java =================================================================== --- trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/test.java 2009-03-25 20:07:42 UTC (rev 13201) +++ trunk/test-applications/realworld2/web/src/main/java/org/richfaces/realworld/util/test.java 2009-03-25 23:09:12 UTC (rev 13202) @@ -16,6 +16,7 @@ import javax.imageio.ImageWriter; import javax.imageio.stream.ImageInputStream; +//TODO nick - delete this class public class test { // private static String directory = "E:\\realworldnew\\web\\src\\main\\webapp\\WEB-INF\\Upload\\Viking\\15"; private static String directory = "E:\\temp\\out_images\\";