diff --git a/config/alfresco/quickshare-services-context.xml b/config/alfresco/quickshare-services-context.xml index 8e34dc4d0a..490f484dd9 100644 --- a/config/alfresco/quickshare-services-context.xml +++ b/config/alfresco/quickshare-services-context.xml @@ -59,7 +59,7 @@ - + diff --git a/config/alfresco/templates/quickshare-email-templates/quickshare-email.default.template.ftl b/config/alfresco/templates/quickshare-email-templates/quickshare-email.default.template.ftl index 1bf90e535d..d76eb876b8 100644 --- a/config/alfresco/templates/quickshare-email-templates/quickshare-email.default.template.ftl +++ b/config/alfresco/templates/quickshare-email-templates/quickshare-email.default.template.ftl @@ -322,7 +322,9 @@ middle;">

${shared_node_name} shared with you

${sender_first_name} ${sender_last_name} has shared ${shared_node_name} with you.

-

${sender_message}

+<#if sender_message??> +

${sender_message}

+
diff --git a/source/java/org/alfresco/repo/quickshare/ClientAppConfig.java b/source/java/org/alfresco/repo/quickshare/ClientAppConfig.java index de7346d765..aae859b831 100644 --- a/source/java/org/alfresco/repo/quickshare/ClientAppConfig.java +++ b/source/java/org/alfresco/repo/quickshare/ClientAppConfig.java @@ -19,6 +19,7 @@ package org.alfresco.repo.quickshare; +import org.alfresco.util.PropertyCheck; import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -36,15 +37,15 @@ import java.util.concurrent.ConcurrentMap; /** * This class picks up all the loaded properties passed to it and uses a naming - * convention to isolate the client's name and related values. - * So, if a new client (e.g. MyClientName) is required to send shared-link email, then the following + * convention to isolate the client's name and the related values. + * So, if a new client (e.g. MyClientName) is required to send a shared-link email, then the following * needs to be put into a properties file. * * The default property file is alfresco/quickshare/quickshare-clients.properties which - * could be overridden by alfresco-global properties file. + * could be overridden (or add new clients) by alfresco-global properties file. * * @author Jamal Kaabi-Mofrad */ @@ -75,10 +76,37 @@ public class ClientAppConfig extends AbstractLifecycleBean this.globalProperties = globalProperties; } + public void init() + { + PropertyCheck.mandatory(this, "defaultProperties", defaultProperties); + PropertyCheck.mandatory(this, "globalProperties", globalProperties); + } + + /** + * Returns an unmodifiable view of the clients map. Never null. + */ + public Map getClients() + { + return Collections.unmodifiableMap(clients); + } + + /** + * Returns the named client or null if no client exists with the given name. + * + * @param name the name of the client to retrieve + */ + public ClientApp getClient(String name) + { + return clients.get(name); + } + @Override protected void onBootstrap(ApplicationEvent event) { - load(); + Map mergedProperties = getAndMergeProperties(); + Set clientsNames = processPropertyKeys(mergedProperties); + clients.putAll(processClients(clientsNames, mergedProperties)); + if (logger.isDebugEnabled()) { logger.debug("All bootstrapped quickShare clients: " + clients); @@ -91,37 +119,12 @@ public class ClientAppConfig extends AbstractLifecycleBean // nothing to do } - public void load() - { - Map mergedProperties = getAndMergeProperties(); - Set clientsNames = processPropertyKeys(mergedProperties); - clients.putAll(processClients(clientsNames, mergedProperties)); - } - - public Map getClients() - { - return Collections.unmodifiableMap(clients); - } - - public ClientApp getClient(String name) - { - return clients.get(name); - } - - public void setClient(ClientApp client) - { - if (client != null) - { - clients.put(client.getName(), client); - } - } - - public boolean removeClient(String name) - { - ClientApp client = clients.remove(name); - return (client != null); - } - + /** + * Processes the property's key and extracts the clients' names. + * + * @param allProps the merged properties + * @return a set of clients' names + */ protected Set processPropertyKeys(Map allProps) { Set clientsNames = new HashSet<>(); @@ -164,6 +167,14 @@ public class ClientAppConfig extends AbstractLifecycleBean return clientsNames; } + /** + * Processes the given properties and if the properties' values are valid, creates + * a map of {@code ClientApp} with the client's name as the key. + * + * @param clientsNames the processed clients' names + * @param allProps the merged properties + * @return a map of {@code ClientApp} with the client's name as the key. + */ protected Map processClients(Set clientsNames, Map allProps) { Map clientApps = new HashMap<>(clientsNames.size()); @@ -171,7 +182,7 @@ public class ClientAppConfig extends AbstractLifecycleBean { String propKey = getPropertyKey(name, PROP_SHARED_LINK_BASE_URL); String sharedLinkBaseUrl = allProps.get(propKey); - if (StringUtils.isEmpty(sharedLinkBaseUrl)) + if (isValidString(sharedLinkBaseUrl)) { logInvalidPropertyValue(propKey, sharedLinkBaseUrl); continue; @@ -179,33 +190,42 @@ public class ClientAppConfig extends AbstractLifecycleBean propKey = getPropertyKey(name, PROP_TEMPLATE_ASSETS_URL); String templateAssetsUrl = allProps.get(propKey); - if (StringUtils.isEmpty(templateAssetsUrl)) + if (isValidString(templateAssetsUrl)) { logInvalidPropertyValue(propKey, templateAssetsUrl); continue; } - // Create the client data + // As the required values are valid, create the client data ClientApp client = new ClientApp(name, sharedLinkBaseUrl, templateAssetsUrl); clientApps.put(name, client); } return clientApps; } + /** + * Converts and merges the given Java properties into a {@code java.util.Map}. + */ protected Map getAndMergeProperties() { Map allProperties = new HashMap<>(); for (String propKey : defaultProperties.stringPropertyNames()) { allProperties.put(propKey, defaultProperties.getProperty(propKey)); - } - //override default values from other property files + // Add new clients or override the default values from other properties files for (String propKey : globalProperties.stringPropertyNames()) { if (propKey.startsWith(PREFIX)) { - allProperties.put(propKey, globalProperties.getProperty(propKey)); + String value = globalProperties.getProperty(propKey); + // before overriding the key, validate the property value + if (isValidString(value)) + { + logInvalidPropertyValue(propKey, value); + continue; + } + allProperties.put(propKey, value); } } @@ -227,6 +247,11 @@ public class ClientAppConfig extends AbstractLifecycleBean return PREFIX + clientName + '.' + clientProp; } + private boolean isValidString(String str) + { + return StringUtils.isEmpty(str); + } + public static class ClientApp { private final String name; diff --git a/source/test-java/org/alfresco/Repository01TestSuite.java b/source/test-java/org/alfresco/Repository01TestSuite.java index 408ef2182c..550ec34b57 100644 --- a/source/test-java/org/alfresco/Repository01TestSuite.java +++ b/source/test-java/org/alfresco/Repository01TestSuite.java @@ -268,6 +268,7 @@ public class Repository01TestSuite extends TestSuite static void tests40(TestSuite suite) { + suite.addTest(new JUnit4TestAdapter(org.alfresco.repo.quickshare.ClientAppConfigTest.class)); suite.addTest(new JUnit4TestAdapter(org.alfresco.repo.quickshare.QuickShareServiceIntegrationTest.class)); suite.addTest(new JUnit4TestAdapter(org.alfresco.repo.rating.RatingServiceIntegrationTest.class)); suite.addTest(new JUnit4TestAdapter(org.alfresco.repo.remotecredentials.RemoteCredentialsServicesTest.class)); diff --git a/source/test-java/org/alfresco/repo/quickshare/ClientAppConfigTest.java b/source/test-java/org/alfresco/repo/quickshare/ClientAppConfigTest.java new file mode 100644 index 0000000000..8ecba6965e --- /dev/null +++ b/source/test-java/org/alfresco/repo/quickshare/ClientAppConfigTest.java @@ -0,0 +1,102 @@ +/* + * Copyright (C) 2005-2016 Alfresco Software Limited. + * + * This file is part of Alfresco + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + */ + +package org.alfresco.repo.quickshare; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.fail; +import org.alfresco.repo.quickshare.ClientAppConfig.ClientApp; +import org.alfresco.util.ApplicationContextHelper; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; +import org.springframework.context.support.ClassPathXmlApplicationContext; + +import java.util.Map; + +/** + * This class contains tests for the class {@link ClientAppConfig} + * + * @author Jamal Kaabi-Mofrad + */ +public class ClientAppConfigTest +{ + private static ClassPathXmlApplicationContext context; + private static ClientAppConfig clientAppConfig; + + @BeforeClass + public static void setUp() throws Exception + { + context = new ClassPathXmlApplicationContext(new String[] { "classpath:org/alfresco/repo/quickshare/test-quickshare-clients-context.xml" }, + ApplicationContextHelper.getApplicationContext()); + + clientAppConfig = context.getBean("quickShareClientsConfigTest", ClientAppConfig.class); + + } + + @AfterClass + public static void tearDown() throws Exception + { + context.close(); + } + + @Test + public void testClients() throws Exception + { + Map clients = clientAppConfig.getClients(); + assertNotNull(clients); + assertEquals("Incorrect number of clients", 3, clients.size()); + + // loaded from org/alfresco/repo/quickshare/test-quickshare-clients-config.properties + ClientApp client1 = clientAppConfig.getClient("test-client1"); + assertNotNull(client1); + assertEquals("test-client1", client1.getName()); + assertEquals("http://localhost:8081/test-client1/o", client1.getSharedLinkBaseUrl()); + assertEquals("http://localhost:8081/test-client1", client1.getTemplateAssetsUrl()); + + // loaded from org/alfresco/repo/quickshare/test-quickshare-clients-config.properties and overridden by + // org/alfresco/repo/quickshare/test-global-properties.properties + ClientApp client2 = clientAppConfig.getClient("test-client2"); + assertNotNull(client2); + assertEquals("test-client2", client2.getName()); + assertEquals("https://127.0.0.1:8082/test-client2/t", client2.getSharedLinkBaseUrl()); + assertEquals("https://127.0.0.1:8082/test-client2", client2.getTemplateAssetsUrl()); + + // loaded from org/alfresco/repo/quickshare/test-global-properties.properties + ClientApp client3 = clientAppConfig.getClient("test-client5"); + assertNotNull(client3); + assertEquals("test-client5", client3.getName()); + assertEquals("http://localhost:8085/test-client5/f", client3.getSharedLinkBaseUrl()); + assertEquals("http://localhost:8085/test-client5", client3.getTemplateAssetsUrl()); + + // Try to add a client into an unmodifiable map + ClientApp newClient = new ClientApp("testClient" + System.currentTimeMillis(), "http://localhost:8085/test-client/s", + "http://localhost:8085/testclient"); + try + { + clients.put(newClient.getName(), newClient); + fail("Shouldn't be able to modify the clients map."); + } + catch (UnsupportedOperationException ex) + { + // expected + } + } +} diff --git a/source/test-resources/org/alfresco/repo/quickshare/test-global-properties.properties b/source/test-resources/org/alfresco/repo/quickshare/test-global-properties.properties new file mode 100644 index 0000000000..5a6bbf5697 --- /dev/null +++ b/source/test-resources/org/alfresco/repo/quickshare/test-global-properties.properties @@ -0,0 +1,41 @@ +# Simulate the alfresco global properties file + +# Override the default properties of client2 +quickshare.client.test-client2.sharedLinkBaseUrl=https://127.0.0.1:8082/test-client2/t +quickshare.client.test-client2.templateAssetsUrl=https://127.0.0.1:8082/test-client2 + +# Add a new client +quickshare.client.test-client5.sharedLinkBaseUrl=http://localhost:8085/test-client5/f +quickshare.client.test-client5.templateAssetsUrl=http://localhost:8085/test-client5 + +# Try to override the default properties of client1 with invalid values +quickshare.client.test-client1.sharedLinkBaseUrl= +quickshare.client.test-client1.templateAssetsUrl= + +# Invalid keys - undefined key structure +quickshare.client.invalid.test-client6.sharedLinkBaseUrl=http://localhost:8086/test-client6/s +quickshare.client.invalid.test-client6.templateAssetsUrl=http://localhost:8086/test-client6 + +# Invalid keys - undefined key structure +invalid.test-client7.sharedLinkBaseUrl=http://localhost:8087/test-client7/s +invalid.test-client7.templateAssetsUrl=http://localhost:8087/test-client7 + +# Invalid keys - undefined key structure +quickshare.client.test-client8.invalid.sharedLinkBaseUrl=http://localhost:8088/test-client8/e +quickshare.client.test-client8.invalid.templateAssetsUrl=http://localhost:8088/test-client8 + +# Invalid key - missing the required 'sharedLinkBaseUrl' and 'templateAssetsUrl' after the dot +quickshare.client.test-client9.=http://localhost:8089/test-client9 + +# Invalid key - missing the required 'sharedLinkBaseUrl' and 'templateAssetsUrl' +quickshare.client.test-client10=http://localhost:8810/test-client10 + +# Invalid keys - undefined 'sharedLink' and 'templateUrl' +quickshare.client.test-client11.sharedLink=http://localhost:8811/test-client11/e +quickshare.client.test-client11.templateUrl=http://localhost:8811/test-client11 + +# Invalid client config - missing the required 'templateAssetsUrl' config +quickshare.client.test-client12.sharedLinkBaseUrl=http://localhost:8812/test-client12/t + +# Invalid client config - missing the required 'sharedLinkBaseUrl' config +quickshare.client.test-client13.templateAssetsUrl=http://localhost:8812/test-client13 \ No newline at end of file diff --git a/source/test-resources/org/alfresco/repo/quickshare/test-quickshare-clients-config.properties b/source/test-resources/org/alfresco/repo/quickshare/test-quickshare-clients-config.properties new file mode 100644 index 0000000000..406ff23d83 --- /dev/null +++ b/source/test-resources/org/alfresco/repo/quickshare/test-quickshare-clients-config.properties @@ -0,0 +1,15 @@ +# registry of clients that are able to email shared links + +quickshare.client.test-client1.sharedLinkBaseUrl=http://localhost:8081/test-client1/o +quickshare.client.test-client1.templateAssetsUrl=http://localhost:8081/test-client1 + +quickshare.client.test-client2.sharedLinkBaseUrl=http://localhost:8082/test-client2/t +quickshare.client.test-client2.templateAssetsUrl=http://localhost:8082/test-client2 + +# Invalid keys - undefined key structure +invalid.test-client3.sharedLinkBaseUrl=http://localhost:8083/test-client3/s +invalid.test-client3.templateAssetsUrl=http://localhost:8083/test-client3 + +# Invalid values +quickshare.client.test-client4.sharedLinkBaseUrl= +quickshare.client.test-client4.templateAssetsUrl= \ No newline at end of file diff --git a/source/test-resources/org/alfresco/repo/quickshare/test-quickshare-clients-context.xml b/source/test-resources/org/alfresco/repo/quickshare/test-quickshare-clients-context.xml new file mode 100644 index 0000000000..445921f110 --- /dev/null +++ b/source/test-resources/org/alfresco/repo/quickshare/test-quickshare-clients-context.xml @@ -0,0 +1,26 @@ + + + + + + + + + classpath*:org/alfresco/repo/quickshare/test-quickshare-clients-config.properties + + + + + + + + classpath*:org/alfresco/repo/quickshare/test-global-properties.properties + + + + + + + + + \ No newline at end of file