Alexander Wels has posted comments on this change.

Change subject: userportal,webadmin: update branding manager
......................................................................


Patch Set 5: (8 inline comments)

....................................................
File 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/branding/BrandingManager.java
Line 72:     /**
Line 73:      * Constructor that takes a {@code File} object to configure the 
brandingRootPath.
Line 74:      * @param etcDir A {@code File} pointing to the branding root path.
Line 75:      */
Line 76:     private BrandingManager(final File etcDir) {
Because we are using getInstance() to call the constructor. Nothing should be 
calling this constructor outside of the class.
Line 77:         brandingRootPath = new File(etcDir, BRANDING_PATH);
Line 78:     }
Line 79: 
Line 80:     /**


Line 80:     /**
Line 81:      * Get an instance of the {@code BrandingManager} with the default 
ETC_DIR.
Line 82:      * @return A {@code BrandingManager}
Line 83:      */
Line 84:     public static synchronized BrandingManager getInstance() {
Except that as explained by Vojtechs link the above code does not work. I was 
in a little bit of a rush when I did this, I will go ahead and clean it up. But 
we will have to pay the synchronization penalty. Which really shouldn't be that 
bad.
Line 85:         if (instance == null) {
Line 86:             instance = new BrandingManager();
Line 87:         }
Line 88:         return instance;


Line 92:      * Get an instance of the {@code BrandingManager} with the passed 
in ETC_DIR.
Line 93:      * @param etcDir a {@code File} pointing to the engine etc 
directory.
Line 94:      * @return A {@code BrandingManager}
Line 95:      */
Line 96:     public static synchronized BrandingManager getInstance(final File 
etcDir) {
One of the two is redudant, I will clean it up.
Line 97:         if (instance == null) {
Line 98:             instance = new BrandingManager(etcDir);
Line 99:         }
Line 100:         return instance;


Line 131:      * Get the message associated with the passed in key.
Line 132:      * @param key The key to get the message for. For instance 
obrand.common.copy_right_notice.
Line 133:      * @return The associated message in the default locale.
Line 134:      */
Line 135:     public static String getMessage(final String key) {
Don't know, wasn't thinking when I did this I think.
Line 136:         return getMessage(key, LocaleFilter.DEFAULT_LOCALE);
Line 137:     }
Line 138: 
Line 139:     /**


Line 141:      * @param key The key to get the message for. For instance 
obrand.common.copy_right_notice.
Line 142:      * @param locale The locale to use to look up the message.
Line 143:      * @return The associated message in the passed in locale.
Line 144:      */
Line 145:     public static String getMessage(final String key, final Locale 
locale) {
Don't know, wasn't thinking when I did this I think.
Line 146:         String result = "";
Line 147:         // key needs to start with obrand.
Line 148:         if (key != null && key.startsWith(BRAND_PREFIX + ".")) {
Line 149:             String[] splitString = key.split("\\.");


Line 151:             if (splitString.length >= 2) {
Line 152:                 prefix = key.split("\\.")[1];
Line 153:             }
Line 154:             if (prefix != null && prefix.length() > 0) {
Line 155:                 result = getInstance().getMessageMap(prefix, locale).
Done
Line 156:                         get(key.substring(key.indexOf(prefix) + 
prefix.length() + 1));
Line 157:             }
Line 158:         }
Line 159:         return result;


Line 164:      * @param prefix The prefix to use for getting the keys.
Line 165:      * @param locale The locale to get the messages for.
Line 166:      * @return A {@code Map} of keys and values.
Line 167:      */
Line 168:     private Map<String, String> getMessageMap(final String prefix, 
final Locale locale) {
I did consider that, but concluded that the cost of reading the files and 
processing was low enough to not make it worth the extra complexity of caching 
the results.
Line 169:         List<BrandingTheme> messageThemes = getBrandingThemes();
Line 170:         // We need this map to remove potential duplicate strings 
from the resource bundles.
Line 171:         Map<String, String> keyValues = new HashMap<String, String>();
Line 172:         if (messageThemes != null) {


Line 205:      * @param keyValues The map to turn into the string.
Line 206:      * @return A string of format {"key":"value",...}
Line 207:      */
Line 208:     String getMessagesFromMap(final Map<String, String> keyValues) {
Line 209:         ObjectNode node = new ObjectMapper().createObjectNode();
Done
Line 210:         for (Map.Entry<String, String> entry : keyValues.entrySet()) {
Line 211:             node.put(entry.getKey(), entry.getValue());
Line 212:         }
Line 213:         return node.size() > 0 ? node.toString() : null;


-- 
To view, visit http://gerrit.ovirt.org/15579
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I87f5a6684701c9d5f603bf79459a3a5b6614b24f
Gerrit-PatchSet: 5
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: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to