Alexander Wels has posted comments on this change. Change subject: engine: root.war cleanup ......................................................................
Patch Set 13: (9 comments) .................................................... File backend/manager/modules/branding/src/main/java/org/ovirt/engine/core/branding/BrandingManager.java Line 127: * @param type a string naming the application type to load the theme for. Line 128: * @return A {@code List} of {@code BrandingTheme}s. Line 129: */ Line 130: public synchronized List<BrandingTheme> getBrandingThemes(final String type) { Line 131: this.applicationType = type; You make a good point, and I will make it happen. Line 132: if (themes == null && brandingRootPath.exists() && brandingRootPath.isDirectory() Line 133: && brandingRootPath.canRead()) { Line 134: themes = new ArrayList<BrandingTheme>(); Line 135: List<File> directories = Arrays.asList( .................................................... File backend/manager/modules/branding/src/main/java/org/ovirt/engine/core/branding/BrandingTheme.java Line 170: * Get the style sheet type based on the passed in {@code ApplicationType}. Line 171: * @param type The application type to get the style sheet string for. Line 172: * @return A {@code String} representation of the style sheet name. Line 173: */ Line 174: public String getThemeStyleSheet(String applicationType) { Actually we don't need the application type in the constructor, as you point out it is never used anywhere. So I removed it there. This application type is used to filter the messages available in the theme to the correct section. Line 175: return brandingProperties.getProperty(applicationType + CSS_POST_FIX); Line 176: } Line 177: Line 178: /** Line 331: return null; Line 332: } Line 333: Line 334: @Override Line 335: public String toString() { See above Line 336: return "Path to theme: " + getPath() + ", User portal css: " //$NON-NLS-1$ //$NON-NLS-2$ Line 337: + getThemeStyleSheet("userportal") + ", Web admin css: " //$NON-NLS-1$ //$NON-NLS-2$ Line 338: + getThemeStyleSheet("webadmin") + ", Welcome page css: " //$NON-NLS-1$ //$NON-NLS-2$ Line 339: + getThemeStyleSheet("welcome"); //$NON-NLS-1$ .................................................... File backend/manager/modules/root/src/main/java/org/ovirt/engine/core/DocsServlet.java Line 63: if (!languagePageShown) { Line 64: setLangPageShown(response, true); Line 65: request.setAttribute(LocaleFilter.LOCALE, locale); Line 66: request.setAttribute(ENGLISH_HREF, redirect); Line 67: final ServletContext forwardContext = getServletContext().getContext(localeDocsMissing); Any suggestions on what to do if it is null? besides die horribly? Line 68: final RequestDispatcher dispatcher = forwardContext.getRequestDispatcher(localeDocsMissing); Line 69: dispatcher.forward(request, response); Line 70: } else { Line 71: //Redirect to English version of the document Line 64: setLangPageShown(response, true); Line 65: request.setAttribute(LocaleFilter.LOCALE, locale); Line 66: request.setAttribute(ENGLISH_HREF, redirect); Line 67: final ServletContext forwardContext = getServletContext().getContext(localeDocsMissing); Line 68: final RequestDispatcher dispatcher = forwardContext.getRequestDispatcher(localeDocsMissing); Any suggestions on what to do if it is null? besides die horribly? Line 69: dispatcher.forward(request, response); Line 70: } else { Line 71: //Redirect to English version of the document Line 72: response.sendRedirect(redirect); .................................................... File backend/manager/modules/root/src/main/java/org/ovirt/engine/core/WelcomeServlet.java Line 23 Line 24 Line 25 Line 26 Line 27 That is a matter of taste, in this case to be consistent with the rest of the applications it made a lot of sense to move everything into the webapp descriptor. .................................................... File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/servlet/PageNotFoundForwardServlet.java Line 69: @Override Line 70: protected void doGet(final HttpServletRequest request, final HttpServletResponse response) throws IOException, Line 71: ServletException { Line 72: //Forward to the page not found URI of the target context, which should have all the proper branding/messages. Line 73: final ServletContext forwardContext = getServletContext().getContext(targetContext); Done Line 74: final RequestDispatcher dispatcher = forwardContext.getRequestDispatcher(pageNotFoundURI); Line 75: dispatcher.forward(request, response); Line 76: } Line 77: Line 70: protected void doGet(final HttpServletRequest request, final HttpServletResponse response) throws IOException, Line 71: ServletException { Line 72: //Forward to the page not found URI of the target context, which should have all the proper branding/messages. Line 73: final ServletContext forwardContext = getServletContext().getContext(targetContext); Line 74: final RequestDispatcher dispatcher = forwardContext.getRequestDispatcher(pageNotFoundURI); Done Line 75: dispatcher.forward(request, response); Line 76: } Line 77: .................................................... File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/GwtDynamicHostPageServlet.java Line 70: public static final String ETAG_HEADER = "Etag"; //$NON-NLS-1$ Line 71: Line 72: private static final String HOST_JSP = "/GwtHostPage.jsp"; //$NON-NLS-1$ Line 73: private static final String UTF_CONTENT_TYPE = "text/html; charset=UTF-8"; //$NON-NLS-1$ Line 74: private static final String APPLICATION_TYPE = "applicationType"; //$NON-NLS-1$ Turns out I don't really need to do any of this, check out the latest patch when I push it. Line 75: Line 76: private BackendLocal backend; Line 77: Line 78: private ObjectMapper mapper; -- To view, visit http://gerrit.ovirt.org/19848 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iceae0ebf671efc951522c464db1a5b2b95dd5637 Gerrit-PatchSet: 13 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches