-
Notifications
You must be signed in to change notification settings - Fork 1
add blobstore to webapp #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| // Copyright 2019 Google LLC | ||
| // | ||
| // 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 | ||
| // | ||
| // https://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 com.google.sps.servlets; | ||
|
|
||
| import com.google.appengine.api.blobstore.BlobstoreService; | ||
| import com.google.appengine.api.blobstore.BlobstoreServiceFactory; | ||
| import java.io.IOException; | ||
| import javax.servlet.annotation.WebServlet; | ||
| import javax.servlet.http.HttpServlet; | ||
| import javax.servlet.http.HttpServletRequest; | ||
| import javax.servlet.http.HttpServletResponse; | ||
|
|
||
| /** | ||
| * When the fetch() function requests the /blobstore-upload-url URL, the content of the response is | ||
| * the URL that allows a user to upload a file to Blobstore. If this sounds confusing, try running a | ||
| * dev server and navigating to /blobstore-upload-url to see the Blobstore URL. | ||
| */ | ||
| @WebServlet("/blob-url") | ||
| public class BlobUrlServlet extends HttpServlet { | ||
| @Override | ||
| public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException { | ||
|
|
||
| BlobstoreService blobstoreService = BlobstoreServiceFactory.getBlobstoreService(); | ||
| String uploadUrl = blobstoreService.createUploadUrl("/file-handler"); | ||
|
|
||
| response.setContentType("text/html"); | ||
| response.getWriter().println(uploadUrl); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,7 +23,8 @@ | |
| import com.google.gson.Gson; | ||
| import com.google.sps.data.Post; | ||
| import java.io.IOException; | ||
| import java.util.*; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import javax.servlet.annotation.WebServlet; | ||
| import javax.servlet.http.HttpServlet; | ||
| import javax.servlet.http.HttpServletRequest; | ||
|
|
@@ -32,7 +33,7 @@ | |
| /** Servlet that returns some example content. TODO: modify this file to handle comments data */ | ||
| @WebServlet("/Comments") | ||
| public class DataServlet extends HttpServlet { | ||
| private int max=5; | ||
| private int maxNumberOfComments=5; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm confused as to why this isn't a constant. Either it's configurable (possibly with a constant default) in which case it should be a local variable or it isn't and should be final. Seeing this random shared mutable value is a big code smell.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: keep final values separate from non-final values to draw attention to the fact that they have different mutability. In this case, I would put |
||
| private static final String COMMENTS_ENTITY_NAME = "Comments"; | ||
| private static final String TIMESTAMP_PROPERTY_KEY = "timestamp"; | ||
| private static final String COMMENT_PROPERTY_KEY = "comment"; | ||
|
|
@@ -46,7 +47,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro | |
| PreparedQuery results = datastore.prepare(query); | ||
|
|
||
| List<Post> comments = new ArrayList<>(); | ||
| FetchOptions fetchOptions = FetchOptions.Builder.withLimit(max); | ||
| FetchOptions fetchOptions = FetchOptions.Builder.withLimit(maxNumberOfComments); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This read needs to be synchronized. |
||
| for (Entity entity : results.asIterable(fetchOptions)) { | ||
| long id = entity.getKey().getId(); | ||
| String comment = (String) entity.getProperty(COMMENT_PROPERTY_KEY); | ||
|
|
@@ -68,7 +69,7 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) thr | |
| String userComment = request.getParameter(COMMENT_PARAMETER_NAME); | ||
| long timestamp = System.currentTimeMillis(); | ||
| String maxString = request.getParameter(MAX_COMMENTS_PARAMETER_NAME); | ||
| if (userComment != null) { | ||
| if (userComment != null ) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. where's my formatter meme? |
||
| Entity taskEntity = new Entity(COMMENTS_ENTITY_NAME); | ||
| taskEntity.setProperty(COMMENT_PROPERTY_KEY, userComment); | ||
| taskEntity.setProperty(TIMESTAMP_PROPERTY_KEY, timestamp); | ||
|
|
@@ -77,11 +78,10 @@ public void doPost(HttpServletRequest request, HttpServletResponse response) thr | |
| datastore.put(taskEntity); | ||
| } else if (maxString != null) { | ||
| try { | ||
| max = Integer.parseInt(maxString); | ||
|
|
||
| maxNumberOfComments = Integer.parseInt(maxString); | ||
| } catch (NumberFormatException e) { | ||
| System.out.println("Could not convert to int: " + maxString); | ||
| max = 1; | ||
| System.out.println("Could not convert to int|" + maxString+"|"); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use System.out.println(String.format("Could not convert to int: \"%s\".", maxString);
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moreover I'm going to cut you a bug to switch to a real logger. |
||
| maxNumberOfComments = 1; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You might want to return an error code to the caller (probably
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This write should be synchronized. |
||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| package com.google.sps.servlets; | ||
|
|
||
| import com.google.appengine.api.blobstore.BlobInfo; | ||
| import com.google.appengine.api.blobstore.BlobInfoFactory; | ||
| import com.google.appengine.api.blobstore.BlobKey; | ||
| import com.google.appengine.api.blobstore.BlobstoreService; | ||
| import com.google.appengine.api.blobstore.BlobstoreServiceFactory; | ||
| import com.google.appengine.api.images.ImagesService; | ||
| import com.google.appengine.api.images.ImagesServiceFactory; | ||
| import com.google.appengine.api.images.ServingUrlOptions; | ||
| import java.io.IOException; | ||
| import java.io.PrintWriter; | ||
| import java.net.MalformedURLException; | ||
| import java.net.URL; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import javax.servlet.annotation.WebServlet; | ||
| import javax.servlet.http.HttpServlet; | ||
| import javax.servlet.http.HttpServletRequest; | ||
| import javax.servlet.http.HttpServletResponse; | ||
|
|
||
| /** | ||
| * When the user submits the form, Blobstore processes the file upload and then forwards the request | ||
| * to this servlet. This servlet can then process the request using the file URL we get from | ||
| * Blobstore. | ||
| */ | ||
| @WebServlet("/file-handler") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be nice if you could reference this path from a static constant, but I don't believe that's possible with annotations. This is another benefit of using a Guice filter to resolve serving paths. (No need to do this here, but consider adopting this in a future change.) |
||
| public class FileHandlerServlet extends HttpServlet { | ||
| @Override | ||
| public void doPost(HttpServletRequest request, HttpServletResponse response) throws IOException { | ||
|
|
||
| // Get the URL of the image that the user uploaded to Blobstore. | ||
| String imageUrl = getUploadedFileUrl(request, "images"); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that |
||
|
|
||
| //Will change when walkthrough is finished | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you tho? |
||
| System.out.println(imageUrl); | ||
| PrintWriter out = response.getWriter(); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. /formattermeme |
||
| out.println("<p>Here's the image you uploaded:</p>"); | ||
| out.println("<a href=\"" + imageUrl + "\">"); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's risky to manually escape values within raw HTML like this. I'll try to figure out if there's a safer way to render this (e.g., using JSPs).
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're using JSPs now? I'm ded. |
||
| out.println("<img src=\"" + imageUrl + "\" />"); | ||
| out.println("</a>"); | ||
|
|
||
|
|
||
| } | ||
| /** Returns a URL that points to the uploaded file, or null if the user didn't upload a file. */ | ||
| private String getUploadedFileUrl(HttpServletRequest request, String formInputElementName) { | ||
| BlobstoreService blobstoreService = BlobstoreServiceFactory.getBlobstoreService(); | ||
| Map<String, List<BlobKey>> blobs = blobstoreService.getUploads(request); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What fresh hell is? This API takes an HttpServletRequest?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also you don't need to actually do anything for this. Just commenting on how much I hate this API. |
||
| List<BlobKey> blobKeys = blobs.get("images"); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make this a constant instead of inlining it if it's a keyword. |
||
|
|
||
| // User submitted form without selecting a file, so we can't get a URL. (dev server) | ||
| if (blobKeys == null || blobKeys.isEmpty()) { | ||
| return null; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you're returning null values as part of the API, you should add the |
||
| } | ||
|
|
||
| // Our form only contains a single file input, so get the first index. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. The form only contains a single file input, but what if the user hijacked the form or manually submitted a request to submit multiple files? We probably don't need to worry about that here, but we may want to consider that during upload. |
||
| BlobKey blobKey = blobKeys.get(0); | ||
|
|
||
| // User submitted form without selecting a file, so we can't get a URL. (live server) | ||
| BlobInfo blobInfo = new BlobInfoFactory().loadBlobInfo(blobKey); | ||
| if (blobInfo.getSize() == 0) { | ||
| blobstoreService.delete(blobKey); | ||
| System.out.println("Nothing here"); | ||
| return null; | ||
| } | ||
|
|
||
|
|
||
| // Use ImagesService to get a URL that points to the uploaded file. | ||
| ImagesService imagesService = ImagesServiceFactory.getImagesService(); | ||
| ServingUrlOptions options = ServingUrlOptions.Builder.withBlobKey(blobKey); | ||
|
|
||
|
|
||
| try { | ||
| URL url = new URL(imagesService.getServingUrl(options)); | ||
| return url.getPath(); | ||
| } catch (MalformedURLException e) { | ||
| return imagesService.getServingUrl(options); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,31 +7,36 @@ | |
| <script src="script.js"></script> | ||
|
|
||
| </head> | ||
| <body onload="getComments()"> | ||
| <div id="content"> | ||
| <body onload="getComments()" > | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you reverse format this file? |
||
| <div id="content" > | ||
| <h1>Charles Lovett's Portfolio</h1> | ||
|
|
||
| <form action="/Comments" method="POST"> | ||
|
|
||
|
|
||
| <p>Post a comment</p> | ||
| <input type="text" name="comment"> | ||
| <input type="text" name="comment" > | ||
| <br/><br/> | ||
| <input type="submit" /> | ||
|
|
||
| <input type="submit" /> | ||
| </form> | ||
|
|
||
| <form action="/Comments" method="POST"> | ||
| <p>Maxium comments</p> | ||
| <input type="number" name="maxnum"> | ||
| <input type="number" name="maxnum" > | ||
| <br/><br/> | ||
| <input type="submit" /> | ||
| <input type="submit" /> | ||
| </form> | ||
|
|
||
| <br/><br/> | ||
|
|
||
| <ul id="fetch-comment"></ul> | ||
|
|
||
| <form id="uploaded-image" method="POST" enctype="multipart/form-data" > | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you need to set an |
||
| <br/><br/> | ||
| <p>Upload Image</p> | ||
| <input type="file" name="images"> | ||
| <br/><br/> | ||
| <input type="submit" /> | ||
| </form> | ||
|
|
||
| <p>This Portfolio contains my life growing up and during college. Hope you enjoy learning a little bit about me.</p> | ||
| <p>Real Quick if you want to know some fun facts about me press the button below:</p> | ||
| <button onclick="addRandomFunFact()">Fun Facts</button> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,15 +31,18 @@ function addRandomFunFact() { | |
| } | ||
|
|
||
| function getComments() { | ||
|
|
||
|
|
||
| fetch('/Comments').then(response => response.json()).then((com) => { | ||
| const commentList = document.getElementById('fetch-comment'); | ||
| com.forEach((comment) => { | ||
| commentList.appendChild(createTaskElement(comment)); | ||
| commentList.appendChild(createCommentElement(comment)); | ||
| }) | ||
| }); | ||
| getImage(); | ||
| } | ||
|
|
||
| function createTaskElement(comment) { | ||
| function createCommentElement(comment) { | ||
| const taskElement = document.createElement('li'); | ||
|
|
||
| taskElement.className = 'comment'; | ||
|
|
@@ -50,7 +53,7 @@ function createTaskElement(comment) { | |
| const deleteButtonElement = document.createElement('button'); | ||
| deleteButtonElement.innerText = 'Delete'; | ||
| deleteButtonElement.addEventListener('click', () => { | ||
| deleteTask(comment); | ||
| deleteComment(comment); | ||
|
|
||
| // Remove the task from the DOM. | ||
| taskElement.remove(); | ||
|
|
@@ -62,13 +65,21 @@ function createTaskElement(comment) { | |
| } | ||
|
|
||
|
|
||
| function createListElement(text) { | ||
| const liElement = document.createElement('li'); | ||
| liElement.innerText = text; | ||
| return liElement; | ||
| } | ||
| function deleteTask(task) { | ||
| function deleteComment(task) { | ||
| const params = new URLSearchParams(); | ||
| params.append('id', task.id); | ||
| fetch('/delete-data', {method: 'POST', body: params}); | ||
| } | ||
|
|
||
| function getImage(){ | ||
|
|
||
| fetch('/blob-url') | ||
| .then((response) => { | ||
| return response.text(); | ||
| }) | ||
| .then((imageUploadUrl) => { | ||
| const messageForm = document.getElementById('uploaded-image'); | ||
| messageForm.action = imageUploadUrl; | ||
| // messageForm.classList.remove('hidden'); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. // |
||
| }); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please put
"/file-handler"into a shared constant.