Alexander Wels has posted comments on this change.
Change subject: userportal, webadmin: branding support[WIP].
......................................................................
Patch Set 1: Verified
(39 inline comments)
Responding to Vojtechs comments.
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/branding/BrandingManager.java
Line 20: /**
Line 21: * The default style sheet name.
Line 22: */
Line 23: public static final String DEFAULT_STYLE_FILENAME =
Line 24: "default.css"; //$NON-NLS-1$
Done
Line 25: /**
Line 26: * The default branding path.
Line 27: */
Line 28: private static final String BRANDING_PATH = "/branding";
//$NON-NLS-1$
Line 30: /**
Line 31: * The prefix used for common messages.
Line 32: */
Line 33: private static final String COMMON_PREFIX = "common"; //$NON-NLS-1$
Line 34: /**
Done
Line 35: * A list of available {@code BrandingTheme}s.
Line 36: */
Line 37: private List<BrandingTheme> themes = null;
Line 38: /**
Line 37: private List<BrandingTheme> themes = null;
Line 38: /**
Line 39: * The root path of the branding themes.
Line 40: */
Line 41: private final File brandingRootPath = new
File(LocalConfig.getInstance().
Done
Line 42: getEtcDir() + BRANDING_PATH);
Line 43:
Line 44: /**
Line 45: * Get a list of available {@code BrandingTheme}s.
Line 44: /**
Line 45: * Get a list of available {@code BrandingTheme}s.
Line 46: * @return A {@code List} of {@code BrandingTheme}s.
Line 47: */
Line 48: public final List<BrandingTheme> getBrandingThemes() {
To make check style shut up. Its argument was that this class is not designed
to be sub classed and therefore your public methods should be final. I since
figured out how to turn off that rule, I will take the final off.
Line 49: if (themes == null) {
Line 50: if (brandingRootPath.exists() &&
brandingRootPath.isDirectory()
Line 51: && brandingRootPath.canRead()) {
Line 52: themes = new ArrayList<BrandingTheme>();
Line 46: * @return A {@code List} of {@code BrandingTheme}s.
Line 47: */
Line 48: public final List<BrandingTheme> getBrandingThemes() {
Line 49: if (themes == null) {
Line 50: if (brandingRootPath.exists() &&
brandingRootPath.isDirectory()
Done
Line 51: && brandingRootPath.canRead()) {
Line 52: themes = new ArrayList<BrandingTheme>();
Line 53: List<File> directories = Arrays.asList(
Line 54: brandingRootPath.listFiles(new
DirectoryFilter()));
Line 75: */
Line 76: public final String getMessages(final String prefix) {
Line 77: List<BrandingTheme> messageThemes = getBrandingThemes();
Line 78: //We need this map to remove potential duplicate strings from
the
Line 79: //resource bundles.
Done
Line 80: Map<String, String> keyValues = new HashMap<String, String>();
Line 81: for (BrandingTheme theme: messageThemes) {
Line 82: ResourceBundle messagesBundle = theme.getMessagesBundle();
Line 83: for (String key: messagesBundle.keySet()) {
Line 83: for (String key: messagesBundle.keySet()) {
Line 84: if (key.startsWith(prefix) ||
key.startsWith(COMMON_PREFIX)) {
Line 85: //We can potentially override existing values here
Line 86: //but this is fine as the themes are sorted in
order
Line 87: //And later messages should override earlier ones.
Done
Line 88: keyValues.put(key.replaceFirst(prefix + "\\.",
""). //$NON-NLS-1$
Line 89: replaceFirst(COMMON_PREFIX + "\\.", ""),
//$NON-NLS-1$
Line 90: messagesBundle.getString(key));
Line 91: }
Line 91: }
Line 92: }
Line 93: }
Line 94: //Turn the map into a string with the format:
Line 95: //{'key':'value',...}
Done
Line 96: return getMessagesFromMap(keyValues);
Line 97: }
Line 98:
Line 99: /**
Line 99: /**
Line 100: * @param keyValues The map to turn into the string.
Line 101: * @return A string of format {'key':'value',...}
Line 102: */
Line 103: private String getMessagesFromMap(final Map<String, String>
keyValues) {
Done
Line 104: boolean first = true;
Line 105: StringBuilder builder = new StringBuilder();
Line 106: builder.append('{'); //Open bracket //$NON-NLS-1$
Line 107: for (Map.Entry<String, String> entry : keyValues.entrySet()) {
Line 102: */
Line 103: private String getMessagesFromMap(final Map<String, String>
keyValues) {
Line 104: boolean first = true;
Line 105: StringBuilder builder = new StringBuilder();
Line 106: builder.append('{'); //Open bracket //$NON-NLS-1$
Not needed after doing previous comment.
Line 107: for (Map.Entry<String, String> entry : keyValues.entrySet()) {
Line 108: if (!first) {
Line 109: builder.append(','); //$NON-NLS-1$
Line 110: }
Line 114: builder.append("':'"); //$NON-NLS-1$
Line 115: builder.append(entry.getValue());
Line 116: builder.append('\''); //$NON-NLS-1$
Line 117: }
Line 118: builder.append('}'); //Close bracket //$NON-NLS-1$
Not needed after doing previous comment.
Line 119: return builder.toString();
Line 120: }
Line 121:
Line 122: /**
Line 129:
Line 130: /**
Line 131: * Comparator that sorts themes in their .
Line 132: */
Line 133: private class BrandingComparator implements
Comparator<BrandingTheme> {
There used be a lot more complicated comparisons for the BrandingThemes. I
eliminated them, and thus this is no longer needed.
Line 134: @Override
Line 135: public int compare(final BrandingTheme theme1,
Line 136: final BrandingTheme theme2) {
Line 137: return -1 * theme1.compareTo(theme2);
Line 140:
Line 141: /**
Line 142: * This class implements a {@code FileFilter} for directories.
Line 143: */
Line 144: private class DirectoryFilter implements FileFilter {
Inlined
Line 145: @Override
Line 146: public boolean accept(final File file) {
Line 147: return file.isDirectory();
Line 148: }
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/branding/BrandingTheme.java
Line 145: propertiesFile = new FileInputStream(brandingPath
Line 146: + "/" + BRANDING_PROPERTIES_NAME); //$NON-NLS-1$
Line 147: brandingProperties.load(propertiesFile);
Line 148: } catch (IOException e) {
Line 149: //Unable to load properties file, disable theme.
Done
Line 150: log.warn("Unable to load properties file for "
//$NON-NLS-1$
Line 151: + "theme located here:"//$NON-NLS-1$
Line 152: + brandingPath + "/" //$NON-NLS-1$
Line 153: + BRANDING_PROPERTIES_NAME);
Line 214: brandingProperties.getProperty(THEME_ORDER_KEY,
Line 215: "0")); //$NON-NLS-1$
Line 216: }
Line 217:
Line 218: @Override
Done
Line 219: /**
Line 220: * Compares two branding themes to each other using the following:
Line 221: * <ol>
Line 222: * <li>type, OEM comes first, then END_USER</li>
Line 218: @Override
Line 219: /**
Line 220: * Compares two branding themes to each other using the following:
Line 221: * <ol>
Line 222: * <li>type, OEM comes first, then END_USER</li>
Yes, I ammended the Javadoc for the enum.
Line 223: * <li>order, if exists, as a regular integer</li>
Line 224: * </ol>
Line 225: */
Line 226: public final int compareTo(final BrandingTheme other) {
Line 265: * Get the theme messages resouce bundle for the US locale.
Line 266: * @return A {@code ResourceBundle} containing messages.
Line 267: */
Line 268: public final ResourceBundle getMessagesBundle() {
Line 269: //Default to US Locale.
Done
Line 270: return getMessagesBundle(Locale.US);
Line 271: }
Line 272:
Line 273: /**
Line 278: public final ResourceBundle getMessagesBundle(final Locale
locale) {
Line 279: ResourceBundle result = null;
Line 280: try {
Line 281: File themeDirectory = new File(filePath);
Line 282: URLClassLoader urlLoader = new URLClassLoader(
I am loading files that are not in the normal class path, so I need a
URLClassloader in order to load files outside of the class path AND still be
able to use the ResourceBundle automagic Locale selection mechanism.
Line 283: new URL[]{themeDirectory.toURI().toURL()});
Line 284: result = ResourceBundle.getBundle(
Line 285: brandingProperties.getProperty(MESSAGES_KEY).
Line 286: replaceAll("\\.properties", ""), //$NON-NLS-1$
//$NON-NLS-2$
Line 285: brandingProperties.getProperty(MESSAGES_KEY).
Line 286: replaceAll("\\.properties", ""), //$NON-NLS-1$
//$NON-NLS-2$
Line 287: locale, urlLoader);
Line 288: } catch (IOException e) {
Line 289: //Unable to load messages resource bundle.
Done
Line 290: log.warn("Unable to read message resource " //$NON-NLS-1$
Line 291: + "bundle, returning null"); //$NON-NLS-1$
Line 292: }
Line 293: return result;
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/BrandingServlet.java
Line 19: * This class serves files from the branding themes to the browser.
This
Line 20: * includes images, style sheets and other files. It provides ETags so
Line 21: * browsers can cache the output of the servlet.
Line 22: */
Line 23: @WebServlet(name = "Branding", value = "/theme/*")
I have no strong feelings either way, moved this to the web-fragment.xml
Line 24: public class BrandingServlet extends HttpServlet {
Line 25:
Line 26: private static final long serialVersionUID = 8687185074759812924L;
Line 27:
Line 39: /**
Line 40: * Default constructor.
Line 41: */
Line 42: public BrandingServlet() {
Line 43: setBrandingManager(new BrandingManager());
Good point will do that.
Line 44: }
Line 45:
Line 46: /**
Line 47: * Set the branding manager.
Line 61: // Locate the requested file:
Line 62: String fullPath = getFullPath(path);
Line 63: if (fullPath != null) {
Line 64: File file = new File(fullPath);
Line 65: if (file == null || !file.exists() || !file.canRead()
Yeah good point, since I just created the object, it will never be null.
Line 66: || file.isDirectory()) {
Line 67: log.error("Unable to locate file, will send a "
//$NON-NLS-1$
Line 68: + "404 error response."); //$NON-NLS-1$
Line 69: response.sendError(HttpServletResponse.SC_NOT_FOUND);
Line 116: if (ServletUtils.isSane(path)) {
Line 117: List<BrandingTheme> brandingPaths = brandingManager.
Line 118: getBrandingThemes();
Line 119: for (BrandingTheme brandingTheme: brandingPaths) {
Line 120: if (path.startsWith(brandingTheme.getPath())) {
Looking at this some more, it makes absolutely no sense how I am doing this. I
look up a theme and check the path, then return the path... For no apparent
reason. I can just return the path, without looping over the theme
Line 121: result = brandingManager.getBrandingRootPath().
Line 122: getAbsolutePath() + path;
Line 123: }
Line 124: }
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/GwtDynamicHostPageServlet.java
Line 55: /**
Line 56: * Get the value of the attribute key.
Line 57: * @return A {@code String} containing the key.
Line 58: */
Line 59: public String getAttributeKey() {
Sure makes sense, less typing later.
Line 60: return attributeKey;
Line 61: }
Line 62: }
Line 63: private static final long serialVersionUID = 3946034162721073929L;
Line 72: private static final String UTF_CONTENT_TYPE = "text/html;
charset=UTF-8"; //$NON-NLS-1$
Line 73:
Line 74: private BackendLocal backend;
Line 75: private ObjectMapper mapper;
Line 76: private final BrandingManager brandingManager = new
BrandingManager();
Done
Line 77:
Line 78: @EJB(beanInterface = BackendLocal.class,
Line 79: mappedName =
"java:global/engine/bll/Backend!org.ovirt.engine.core.common.interfaces.BackendLocal")
Line 80: public void setBackend(BackendLocal backend) {
Line 91: // Set attribute for selector script
Line 92:
request.setAttribute(MD5Attributes.ATTR_SELECTOR_SCRIPT.getAttributeKey(),
getSelectorScriptName());
Line 93: // Set attribute for themes.
Line 94:
request.setAttribute(MD5Attributes.ATTR_STYLES.getAttributeKey(),
brandingManager.getBrandingThemes());
Line 95: //Set the messages that need to be replaced.
Done
Line 96:
request.setAttribute(MD5Attributes.ATTR_MESSAGES.getAttributeKey(),
getBrandingMessages());
Line 97: //Set the default theme path.
Line 98: request.setAttribute(ATTR_THEME_PATH, ATTR_THEME_PATH);
Line 99: // Set attribute for default style sheet location.
Line 93: // Set attribute for themes.
Line 94:
request.setAttribute(MD5Attributes.ATTR_STYLES.getAttributeKey(),
brandingManager.getBrandingThemes());
Line 95: //Set the messages that need to be replaced.
Line 96:
request.setAttribute(MD5Attributes.ATTR_MESSAGES.getAttributeKey(),
getBrandingMessages());
Line 97: //Set the default theme path.
Done
Line 98: request.setAttribute(ATTR_THEME_PATH, ATTR_THEME_PATH);
Line 99: // Set attribute for default style sheet location.
Line 100: request.setAttribute(ATTR_DEFAULT_STYLE, "default.css");
//$NON-NLS-1$
Line 101: // Set class of servlet
Line 94:
request.setAttribute(MD5Attributes.ATTR_STYLES.getAttributeKey(),
brandingManager.getBrandingThemes());
Line 95: //Set the messages that need to be replaced.
Line 96:
request.setAttribute(MD5Attributes.ATTR_MESSAGES.getAttributeKey(),
getBrandingMessages());
Line 97: //Set the default theme path.
Line 98: request.setAttribute(ATTR_THEME_PATH, ATTR_THEME_PATH);
I guess, I figured these wouldn't change and one less $NON-NLS-1$ I had to type.
Line 99: // Set attribute for default style sheet location.
Line 100: request.setAttribute(ATTR_DEFAULT_STYLE, "default.css");
//$NON-NLS-1$
Line 101: // Set class of servlet
Line 102:
request.setAttribute(MD5Attributes.ATTR_APPLICATION_TYPE.getAttributeKey(),
getApplicationType());
Line 212: * string representation of the MD5 sum.
Line 213: * @throws NoSuchAlgorithmException If the method cannot create
the digest
Line 214: * object.
Line 215: */
Line 216: protected MessageDigest getMd5Digest(final HttpServletRequest
request)
Thank you, I realized I had 5 of the same just different attribute keys, why
not put those in an enum and loop over them. Makes for easier extension, and if
we add another attribute, it is automatically calculated in the MD5.
Line 217: throws NoSuchAlgorithmException {
Line 218: MessageDigest digest = createMd5Digest();
Line 219: for (MD5Attributes attribute: MD5Attributes.values()) {
Line 220: if (request.getAttribute(attribute.getAttributeKey()) !=
null) {
....................................................
File
frontend/webadmin/modules/frontend/src/main/resources/META-INF/resources/GwtHostPage.jsp
Line 5: <head>
Line 6: <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
Line 7: <meta http-equiv="X-UA-Compatible" content="IE=edge">
Line 8: <c:if test="${requestScope['defaultStyle'] != null}">
Line 9: <link rel="stylesheet" type="text/css"
href="${requestScope['defaultStyle']}">
The reason it is indented this way, is so that in the output html the link tag
is properly indented.
Line 10: </c:if>
Line 11: <c:if test="${requestScope['brandingStyle'] != null}">
Line 12: <c:forEach items="${requestScope['brandingStyle']}"
var="theme">
Line 13: <link rel="stylesheet" type="text/css"
href="${pageContext.request.contextPath}/${requestScope['theme']}${theme.path}/${theme.commonStyleSheetName}">
....................................................
File
frontend/webadmin/modules/frontend/src/main/resources/META-INF/web-fragment.xml
Line 12: <!-- * ClientBundle composite images: <md5>.cache.png |
<md5>.cache.gif -->
Line 13: <!-- * split points: deferredjs/<md5>/<n>.cache.js -->
Line 14: <init-param>
Line 15: <param-name>cache</param-name>
Line 16:
<param-value>.*\.cache\.html|.*\.cache\.png|.*\.cache\.gif|.*\.cache\.js|.*\.css</param-value>
Done
Line 17: </init-param>
Line 18:
Line 19: <!-- Resources which always need to be checked for
changes: -->
Line 20: <!-- * application host page: WebAdmin.html |
UserPortal.html -->
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/utils/DynamicConstants.java
Line 43: * Protected default constructor.
Line 44: * @param gwtConstants The fall back {@code
CommonApplicationConstants}.
Line 45: */
Line 46: @Inject
Line 47: protected DynamicConstants(final CommonApplicationConstants
gwtConstants) {
Not sure, must have seemed appropriate at the time. Fixed.
Line 48: this.constants = gwtConstants;
Line 49: dictionary = null;
Line 50: try {
Line 51: dictionary =
Dictionary.getDictionary(MESSAGES_DICTIONARY_NAME);
Line 49: dictionary = null;
Line 50: try {
Line 51: dictionary =
Dictionary.getDictionary(MESSAGES_DICTIONARY_NAME);
Line 52: } catch (MissingResourceException mre) {
Line 53: //Do nothing, the dictionary doesn't exist.
Done
Line 54: mre.printStackTrace();
Line 55: }
Line 56: }
Line 57:
Line 50: try {
Line 51: dictionary =
Dictionary.getDictionary(MESSAGES_DICTIONARY_NAME);
Line 52: } catch (MissingResourceException mre) {
Line 53: //Do nothing, the dictionary doesn't exist.
Line 54: mre.printStackTrace();
I had that in there as debugging, removed.
Line 55: }
Line 56: }
Line 57:
Line 58: /**
Line 68: if (dictionary != null) {
Line 69: result = dictionary.get(key);
Line 70: }
Line 71: } catch (MissingResourceException mre) {
Line 72: //Do nothing, the key doesn't exist.
Done
Line 73: }
Line 74: return result;
Line 75: }
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/dialog/SimpleDialogPanel.ui.xml
Line 108:
Line 109: <g:FlowPanel>
Line 110: <g:FlowPanel addStyleNames="{style.header}">
Line 111: <g:SimplePanel ui:field="logoPanel"
addStyleNames="{style.headerLeftPanel}">
Line 112: <g:Image styleName='dialogLogoImage'
url="clear.cache.gif" />
Because an image without a url parameter doesn't show up, and I can't use a
style sheet to style the image at that point. I am actually NOT showing an
image, I am setting the background of the image to be the actual image and
using the clear.cache.gif (which is already loaded anyway) to give myself a
transparent image foreground.
Line 113: </g:SimplePanel>
Line 114: <g:FlowPanel
addStyleNames="{style.headerCenterPanel}">
Line 115: <g:FlowPanel
ui:field="headerContainerPanel">
Line 116: <g:SimplePanel
ui:field="headerTitlePanel" addStyleNames="{style.headerTitle}" />
....................................................
File
frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/ApplicationDynamicConstants.java
Line 12: /**
Line 13: * Default constructor for UiBinder.
Line 14: */
Line 15: public ApplicationDynamicConstants() {
Line 16: this((CommonApplicationConstants)
No, GWT.create returns an object, and I am passing that to the other
constructor so it needs the case.
Line 17: GWT.create(CommonApplicationConstants.class));
Line 18: }
Line 19:
Line 20: /**
Line 35: */
Line 36: public final String applicationTitle() {
Line 37: String result = getString(APPLICATION_TITLE);
Line 38: if (result == null) {
Line 39: result = ((ApplicationConstants)
constants).applicationTitle();
I feel there is a better way to do this too, but I am not sure how.
Line 40: }
Line 41: return result;
Line 42: }
Line 43:
....................................................
File
frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/webadmin/WebAdmin.gwt.xml
Line 1
Line 2
Line 3
Line 4
Line 5
Don't know I think I added an inherits at some point, but removed it later. I
will revert that change.
--
To view, visit http://gerrit.ovirt.org/13181
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a8a426ce7d688d33c5ae2b70632c836843106b2
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
