Blobstore image upload fetch #35
Conversation
…ureException, courtesy of neelgandhi@
| @@ -19,7 +19,7 @@ | |||
| * | |||
| * @param loggedIn true if a user is logged in, else false | |||
| * @param userId a unique String identifying the user, if logged in; else null | |||
| * @param userEmail | |||
| * @param userEmail | |||
| * @param redirectURL the URL to be redirected to after logging in or out | |||
| */ | |||
| public LoginStatus(boolean loggedIn, String userId, String userEmail, String redirectURL) { | |||
There was a problem hiding this comment.
I missed this is previous reviews, but something to think about if you have time to come back to this code:
Create a User object to hold userId, userEmail. That way you can do LoginStatus(User user, boolean loggedIn, String redirectUrl)
Loose rule of thumb if you are passing 2 or more params that are similar, think about creating an object to hold the values.
|
|
||
| @Override | ||
| public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException { | ||
| BlobstoreService blobstoreService = BlobstoreServiceFactory.getBlobstoreService(); |
There was a problem hiding this comment.
Does BlobstoreService need to be initialized on every Get? Can this be a global constant variable?
| @Override | ||
| public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException { | ||
| BlobstoreService blobstoreService = BlobstoreServiceFactory.getBlobstoreService(); | ||
| String uploadUrl = blobstoreService.createUploadUrl("/data"); |
There was a problem hiding this comment.
If you don't plan to use uploadUrl in any other way, you can combine it with line 34.
response.getWriter().println(blobstoreService.createUploadUrl("/data"));
|
|
||
| // User submitted form without selecting a file, so we can't get a URL. (dev server) | ||
| if (blobKeys == null || blobKeys.isEmpty()) { | ||
| return null; |
There was a problem hiding this comment.
Given this method can return null, you should think about using @Nullable annotation on the method. This is a good way of getting compiler errors on incompatibile Nullability and help prevent issues at compile time instead of runtime.
| // https://stackoverflow.com/q/10779564/873165 | ||
|
|
||
| // Use ImagesService to get a URL that points to the uploaded file. | ||
| ImagesService imagesService = ImagesServiceFactory.getImagesService(); |
There was a problem hiding this comment.
Similar comment to above, can this be made into a global constant?
|
|
||
| @Override | ||
| public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException { | ||
| BlobKey blobKey = new BlobKey(request.getParameter("blob-key")); |
There was a problem hiding this comment.
Make "blob-key" into a constant. This is more preemptive since it is only be used once currently.
| fetch(SERVLET_BLOBSTORE).then(response => response.text()).then(result => { | ||
| commentForm.action = result; | ||
| }) | ||
| .catch(error => console.error(error.message)); |
There was a problem hiding this comment.
In the future think about how you would display this error to the user, if it is human readable.
…nd introduce Nullable annotations
Enables site visitors to upload up to 1 image with each comment, which is displayed as a thumbnail