Bug fixes for UI tools - handing ordinal 0 and propagating exception …

…to client browser (#587) * Bugfix: ordinal value 0 handling in velocity template since major version upgrade * Bugfix: Propagate exception back to browser * Explorer UI velocity template- Harden null check --------- Co-authored-by: Sunjeet Singh <[email protected]>
Netflix · Feb 15, 2023 · e63ffb2 · e63ffb2
1 parent a73b65b
commit e63ffb2
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 40 deletions.
diff --git a/hollow-diff-ui/src/main/java/com/netflix/hollow/diff/ui/HollowDiffUIRouter.java b/hollow-diff-ui/src/main/java/com/netflix/hollow/diff/ui/HollowDiffUIRouter.java
@@ -37,13 +37,13 @@ public HollowDiffUIRouter(String baseUrlPath) {
         this.diffUIs = new LinkedHashMap<>();
     }
 
-    public void handle(String target, HttpServletRequest req, HttpServletResponse resp)
+    public boolean handle(String target, HttpServletRequest req, HttpServletResponse resp)
             throws IOException {
         String diffUIKey = getTargetRootPath(target);
 
         if ("resource".equals(diffUIKey)) {
             if (serveResource(req, resp, getResourceName(target, diffUIKey)))
-                return;
+                return true;
         } else {
             HollowDiffUI ui = diffUIs.get(diffUIKey);
             if (ui == null) {
@@ -55,14 +55,10 @@ public void handle(String target, HttpServletRequest req, HttpServletResponse re
 
             if (ui != null) {
                 if (ui.serveRequest(getResourceName(target, diffUIKey), req, resp))
-                    return;
+                    return true;
             }
         }
-    }
-
-    @Override
-    public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
-        handle(req.getPathInfo(), req, resp);
+        return false;
     }
 
     public Map<String, HollowDiffUI> getDiffUIs() {

diff --git a/hollow-diff-ui/src/main/java/com/netflix/hollow/history/ui/HollowHistoryUI.java b/hollow-diff-ui/src/main/java/com/netflix/hollow/history/ui/HollowHistoryUI.java
@@ -161,56 +161,57 @@ public HollowHistory getHistory() {
         return history;
     }
 
-    public void handle(String target, HttpServletRequest req, HttpServletResponse resp)
+    @Override
+    public boolean handle(String target, HttpServletRequest req, HttpServletResponse resp)
             throws IOException {
         String pageName = getTargetRootPath(target);
 
         if("diffrowdata".equals(pageName)) {
             diffViewOutputGenerator.uncollapseRow(req, resp);
-            return;
+            return true;
         } else if("collapsediffrow".equals(pageName)) {
             diffViewOutputGenerator.collapseRow(req, resp);
-            return;
+            return true;
         }
 
         resp.setContentType("text/html");
 
-
         if("resource".equals(pageName)) {
             if(serveResource(req, resp, getResourceName(target)))
-                return;
+                return true;
         } else if("".equals(pageName) || "overview".equals(pageName)) {
             if(req.getParameter("format") != null && req.getParameter("format").equals("json")) {
                 overviewPage.sendJson(req, resp);
-                return;
+                return true;
             }
             overviewPage.render(req, getSession(req, resp), resp.getWriter());
         } else if("state".equals(pageName)) {
             if(req.getParameter("format") != null && req.getParameter("format").equals("json")) {
                 statePage.sendJson(req, resp);
-                return;
+                return true;
             }
             statePage.render(req, getSession(req, resp), resp.getWriter());
+            return true;
         } else if("statetype".equals(pageName)) {
             if(req.getParameter("format") != null && req.getParameter("format").equals("json")) {
                 stateTypePage.sendJson(req, getSession(req, resp),  resp);
-                return;
+                return true;
             }
             stateTypePage.render(req, getSession(req, resp), resp.getWriter());
+            return true;
         } else if("statetypeexpand".equals(pageName)) {
             stateTypeExpandPage.render(req, getSession(req, resp), resp.getWriter());
+            return true;
         } else if("query".equals(pageName)) {
             queryPage.render(req, getSession(req, resp), resp.getWriter());
+            return true;
         } else if("historicalObject".equals(pageName)) {
             objectDiffPage.render(req, getSession(req, resp), resp.getWriter());
+            return true;
         }
+        return false;
     }
 
-    @Override
-    public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
-        handle(req.getPathInfo(), req, resp);
-    }
-
     public void addCustomHollowRecordNamer(String typeName, HollowHistoryRecordNamer recordNamer) {
         customHollowRecordNamers.put(typeName, recordNamer);
     }

diff --git a/hollow-explorer-ui/src/main/java/com/netflix/hollow/explorer/ui/HollowExplorerUI.java b/hollow-explorer-ui/src/main/java/com/netflix/hollow/explorer/ui/HollowExplorerUI.java
@@ -71,30 +71,27 @@ private HollowExplorerUI(String baseUrlPath, HollowConsumer consumer, HollowClie
         this.queryPage = new QueryPage(this);
     }
 
-    public void handle(String target, HttpServletRequest req, HttpServletResponse resp)
+    @Override
+    public boolean handle(String target, HttpServletRequest req, HttpServletResponse resp)
             throws IOException {
         String pageName = getTargetRootPath(target);
 
         HollowUISession session = HollowUISession.getSession(req, resp);
 
         if("".equals(pageName) || "home".equals(pageName)) {
             showAllTypesPage.render(req, resp, session);
-            return;
+            return true;
         } else if("type".equals(pageName)) {
             browseTypePage.render(req, resp, session);
-            return;
+            return true;
         } else if("schema".equals(pageName)) {
             browseSchemaPage.render(req, resp, session);
-            return;
+            return true;
         } else if("query".equals(pageName)) {
             queryPage.render(req, resp, session);
-            return;
+            return true;
         }
-    }
-
-    @Override
-    public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
-        handle(req.getPathInfo(), req, resp);
+        return false;
     }
 
     public long getCurrentStateVersion() {

diff --git a/hollow-explorer-ui/src/main/resources/browse-selected-type-bottom.vm b/hollow-explorer-ui/src/main/resources/browse-selected-type-bottom.vm
@@ -1,5 +1,5 @@
 #**
- *  Copyright 2018 Netflix, Inc.
+ *  Copyright 2023 Netflix, Inc.
  *
  *     Licensed under the Apache License, Version 2.0 (the "License");
  *     you may not use this file except in compliance with the License.
@@ -13,7 +13,7 @@
  *     See the License for the specific language governing permissions and
  *     limitations under the License.
 *#
-#if($ordinal)</pre>#end
+#if($ordinal != $null)</pre>#end
 						</div>
 					</td>
 				</tr>

diff --git a/hollow-explorer-ui/src/main/resources/browse-selected-type-top.vm b/hollow-explorer-ui/src/main/resources/browse-selected-type-top.vm
@@ -1,5 +1,5 @@
 #**
- *  Copyright 2017 Netflix, Inc.
+ *  Copyright 2023 Netflix, Inc.
  *
  *     Licensed under the Apache License, Version 2.0 (the "License");
  *     you may not use this file except in compliance with the License.
@@ -37,7 +37,7 @@ NOTE: Results are filtered by query <span style="font-family: monospace;">$esc.h
 				<a href="$basePath/type?type=$esc.url($type)&key=$esc.url($key)&ordinal=$ordinal&display=$display&page=$nextPage&pageSize=$pageSize">NEXT</a>
 			#end
 		</td>
-		#if($ordinal)
+		#if($ordinal != $null)
 			<td class="nav">
 				FORMAT: 
 				<a href="$basePath/type?type=$esc.url($type)&key=$esc.url($key)&ordinal=$ordinal&page=$page&pageSize=$pageSize&display=text">text</a> 
@@ -89,12 +89,12 @@ NOTE: Results are filtered by query <span style="font-family: monospace;">$esc.h
 						</form>
 					</td>
 				</tr>
-				#if($ordinal)
+				#if($ordinal != $null)
 					<tr valign=top>
 						<th align=right nowrap>Showing Record: </th>
 						<td id="recLink">
 							ordinal: $ordinal
-							#if($key && !$key.isEmpty())
+							#if($key)	<!-- not null and not empty -->
 								<br/>key: $esc.html($key)
 							#end
 						</td>
@@ -106,4 +106,4 @@ NOTE: Results are filtered by query <span style="font-family: monospace;">$esc.h
 					</th>
 					<td>
 						<div style="min-height:400px">
-							#if($ordinal)<pre>#end
+							#if($ordinal != $null)<pre>#end
diff --git a/hollow-ui-tools/src/main/java/com/netflix/hollow/ui/HollowUIRouter.java b/hollow-ui-tools/src/main/java/com/netflix/hollow/ui/HollowUIRouter.java
@@ -18,6 +18,8 @@
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.PrintWriter;
+import java.io.StringWriter;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -32,8 +34,18 @@ public abstract class HollowUIRouter extends HttpServlet {
     protected final VelocityEngine velocityEngine;
 
     @Override
-    public abstract void doGet(HttpServletRequest request,
-        HttpServletResponse response) throws IOException;
+    public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException {
+        try {
+            handle(request.getPathInfo(), request, response);
+        } catch (Exception ex) {
+            StringWriter stringWriter = new StringWriter();
+            PrintWriter printWriter = new PrintWriter(stringWriter);
+            ex.printStackTrace(printWriter);
+            response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, stringWriter.toString());
+        }
+    }
+
+    public abstract boolean handle(String target, HttpServletRequest req, HttpServletResponse resp) throws IOException;
 
     public HollowUIRouter(String baseUrlPath) {
         if(!baseUrlPath.startsWith("/"))