SHIRO-593 - Added getFrameworkIni() method to IniWebEnvironment Which will allow downstream integrations to provide default configuration that will be merged with the existing configuration. Default functionality is unchanged.
Project: http://git-wip-us.apache.org/repos/asf/shiro/repo Commit: http://git-wip-us.apache.org/repos/asf/shiro/commit/6e9a20ab Tree: http://git-wip-us.apache.org/repos/asf/shiro/tree/6e9a20ab Diff: http://git-wip-us.apache.org/repos/asf/shiro/diff/6e9a20ab Branch: refs/heads/master Commit: 6e9a20abc65acf9db25c2667d114230d74513416 Parents: 4af5e18 Author: Brian Demers <bdem...@apache.org> Authored: Fri Sep 30 16:40:56 2016 -0400 Committer: Brian Demers <bdem...@apache.org> Committed: Tue Oct 18 13:18:45 2016 -0400 ---------------------------------------------------------------------- .../main/java/org/apache/shiro/config/Ini.java | 51 +++++++++ .../org/apache/shiro/config/IniTest.groovy | 103 +++++++++++++++++++ .../apache/shiro/web/env/IniWebEnvironment.java | 84 ++++++++++++++- .../shiro/web/env/IniWebEnvironmentTest.groovy | 56 +++++++++- 4 files changed, 285 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/shiro/blob/6e9a20ab/config/core/src/main/java/org/apache/shiro/config/Ini.java ---------------------------------------------------------------------- diff --git a/config/core/src/main/java/org/apache/shiro/config/Ini.java b/config/core/src/main/java/org/apache/shiro/config/Ini.java index a71bc20..46bdce4 100644 --- a/config/core/src/main/java/org/apache/shiro/config/Ini.java +++ b/config/core/src/main/java/org/apache/shiro/config/Ini.java @@ -303,6 +303,57 @@ public class Ini implements Map<String, Ini.Section> { } } + /** + * Merges the contents of <code>m</code>'s {@link Section} objects into self. + * This differs from {@link Ini#putAll(Map)}, in that each section is merged with the existing one. + * For example the following two ini blocks are merged and the result is the third<BR/> + * <p> + * Initial: + * <pre> + * <code>[section1] + * key1 = value1 + * + * [section2] + * key2 = value2 + * </code> </pre> + * + * To be merged: + * <pre> + * <code>[section1] + * foo = bar + * + * [section2] + * key2 = new value + * </code> </pre> + * + * Result: + * <pre> + * <code>[section1] + * key1 = value1 + * foo = bar + * + * [section2] + * key2 = new value + * </code> </pre> + * + * </p> + * + * @param m map to be merged + * @since 1.4 + */ + public void merge(Map<String, Section> m) { + + if (m != null) { + for (Entry<String, Section> entry : m.entrySet()) { + Section section = this.getSection(entry.getKey()); + if (section == null) { + section = addSection(entry.getKey()); + } + section.putAll(entry.getValue()); + } + } + } + private void addSection(String name, StringBuilder content) { if (content.length() > 0) { String contentString = content.toString(); http://git-wip-us.apache.org/repos/asf/shiro/blob/6e9a20ab/config/core/src/test/groovy/org/apache/shiro/config/IniTest.groovy ---------------------------------------------------------------------- diff --git a/config/core/src/test/groovy/org/apache/shiro/config/IniTest.groovy b/config/core/src/test/groovy/org/apache/shiro/config/IniTest.groovy index 431e520..a17a181 100644 --- a/config/core/src/test/groovy/org/apache/shiro/config/IniTest.groovy +++ b/config/core/src/test/groovy/org/apache/shiro/config/IniTest.groovy @@ -21,6 +21,9 @@ package org.apache.shiro.config; import static org.junit.Assert.*; import org.junit.Test; + +import static org.hamcrest.Matchers.*; + /** * Unit test for the {@link Ini} class. * @@ -158,4 +161,104 @@ public class IniTest { assertEquals("value 4", section.get("prop4")); assertEquals("some long value", section.get("prop5")); } + + /** + * @since 1.4 + */ + @Test + public void testPutAll() { + + Ini ini1 = new Ini(); + ini1.setSectionProperty("section1", "key1", "value1"); + + Ini ini2 = new Ini(); + ini2.setSectionProperty("section2", "key2", "value2"); + + ini1.putAll(ini2); + + assertThat(ini1.getSectionNames(), allOf( + hasItem("section1"), + hasItem("section2") + )); + + // two sections each with one property + assertThat(ini1.getSectionNames(), hasSize(2)); + assertThat(ini1.getSection("section2"), aMapWithSize(1)); + assertThat(ini1.getSection("section1"), aMapWithSize(1)); + + // adding a value directly to ini2's section will update ini1 + ini2.setSectionProperty("section2", "key2.2", "value2.2"); + assertThat(ini1.getSection("section2"), aMapWithSize(2)); + + Ini ini3 = new Ini(); + ini3.setSectionProperty("section1", "key1.3", "value1.3"); + + // this will replace the whole section + ini1.putAll(ini3); + assertThat(ini1.getSection("section1"), aMapWithSize(1)); + + } + + /** + * @since 1.4 + */ + @Test + public void testMerge() { + + Ini ini1 = new Ini(); + ini1.setSectionProperty("section1", "key1", "value1"); + + Ini ini2 = new Ini(); + ini2.setSectionProperty("section2", "key2", "value2"); + + ini1.merge(ini2); + + assertThat(ini1.getSectionNames(), allOf( + hasItem("section1"), + hasItem("section2") + )); + + // two sections each with one property + assertThat(ini1.getSectionNames(), hasSize(2)); + assertThat(ini1.getSection("section2"), aMapWithSize(1)); + assertThat(ini1.getSection("section1"), aMapWithSize(1)); + + // updating the original ini2, will NOT effect ini1 + ini2.setSectionProperty("section2", "key2.2", "value2.2"); + assertThat(ini1.getSection("section2"), aMapWithSize(1)); + + Ini ini3 = new Ini(); + ini3.setSectionProperty("section1", "key1.3", "value1.3"); + + // after merging the section will contain 2 values + ini1.merge(ini3); + assertThat(ini1.getSection("section1"), aMapWithSize(2)); + } + + /** + * @since 1.4 + */ + @Test + public void testCreateWithDefaults() { + + Ini ini1 = new Ini(); + ini1.setSectionProperty("section1", "key1", "value1"); + + Ini ini2 = new Ini(ini1); + ini2.setSectionProperty("section2", "key2", "value2"); + + assertThat(ini2.getSectionNames(), allOf( + hasItem("section1"), + hasItem("section2") + )); + + // two sections each with one property + assertThat(ini2.getSectionNames(), hasSize(2)); + assertThat(ini2.getSection("section2"), aMapWithSize(1)); + assertThat(ini2.getSection("section1"), aMapWithSize(1)); + + // updating the original ini1, will NOT effect ini2 + ini1.setSectionProperty("section1", "key1.1", "value1.1"); + assertThat(ini2.getSection("section1"), allOf(aMapWithSize(1), hasEntry("key1", "value1"))); + } } http://git-wip-us.apache.org/repos/asf/shiro/blob/6e9a20ab/web/src/main/java/org/apache/shiro/web/env/IniWebEnvironment.java ---------------------------------------------------------------------- diff --git a/web/src/main/java/org/apache/shiro/web/env/IniWebEnvironment.java b/web/src/main/java/org/apache/shiro/web/env/IniWebEnvironment.java index afb15ba..f07406a 100644 --- a/web/src/main/java/org/apache/shiro/web/env/IniWebEnvironment.java +++ b/web/src/main/java/org/apache/shiro/web/env/IniWebEnvironment.java @@ -60,6 +60,20 @@ public class IniWebEnvironment extends ResourceBasedWebEnvironment implements In * configuration and calling {@link #configure() configure} for actual instance configuration. */ public void init() { + + setIni(parseConfig()); + + configure(); + } + + /** + * Loads configuration {@link Ini} from {@link #getConfigLocations()} if set, otherwise falling back + * to the {@link #getDefaultConfigLocations()}. Finally any Ini objects will be merged with the value returned + * from {@link #getFrameworkIni()} + * @return Ini configuration to be used by this Environment. + * @since 1.4 + */ + protected Ini parseConfig() { Ini ini = getIni(); String[] configLocations = getConfigLocations(); @@ -82,14 +96,15 @@ public class IniWebEnvironment extends ResourceBasedWebEnvironment implements In ini = getDefaultIni(); } + // Allow for integrations to provide default that will be merged other configuration. + // to retain backwards compatibility this must be a different method then 'getDefaultIni()' + ini = mergeIni(getFrameworkIni(), ini); + if (CollectionUtils.isEmpty(ini)) { String msg = "Shiro INI configuration was either not found or discovered to be empty/unconfigured."; throw new ConfigurationException(msg); } - - setIni(ini); - - configure(); + return ini; } protected void configure() { @@ -105,6 +120,50 @@ public class IniWebEnvironment extends ResourceBasedWebEnvironment implements In } } + /** + * Extension point to allow subclasses to provide an {@link Ini} configuration that will be merged into the + * users configuration. The users configuration will override anything set here. + * <p> + * <strong>NOTE:</strong> Framework developers should use with caution. It is possible a user could provide + * configuration that would conflict with the frameworks configuration. For example: if this method returns an + * Ini object with the following configuration: + * <pre><code> + * [main] + * realm = com.myco.FoobarRealm + * realm.foobarSpecificField = A string + * </code></pre> + * And the user provides a similar configuration: + * <pre><code> + * [main] + * realm = net.differentco.MyCustomRealm + * </code></pre> + * + * This would merge into: + * <pre><code> + * [main] + * realm = net.differentco.MyCustomRealm + * realm.foobarSpecificField = A string + * </code></pre> + * + * This may cause a configuration error if <code>MyCustomRealm</code> does not contain the field <code>foobarSpecificField</code>. + * This can be avoided if the Framework Ini uses more unique names, such as <code>foobarRealm</code>. which would result + * in a merged configuration that looks like: + * <pre><code> + * [main] + * foobarRealm = com.myco.FoobarRealm + * foobarRealm.foobarSpecificField = A string + * realm = net.differentco.MyCustomRealm + * </code></pre> + * + * </p> + * + * @return Ini configuration used by the framework integrations. + * @since 1.4 + */ + protected Ini getFrameworkIni() { + return null; + } + protected Ini getSpecifiedIni(String[] configLocations) throws ConfigurationException { Ini ini = null; @@ -124,6 +183,23 @@ public class IniWebEnvironment extends ResourceBasedWebEnvironment implements In return ini; } + protected Ini mergeIni(Ini ini1, Ini ini2) { + + if (ini1 == null) { + return ini2; + } + + if (ini2 == null) { + return ini1; + } + + // at this point we have two valid ini objects, create a new one and merge the contents of 2 into 1 + Ini iniResult = new Ini(ini1); + iniResult.merge(ini2); + + return iniResult; + } + protected Ini getDefaultIni() { Ini ini = null; http://git-wip-us.apache.org/repos/asf/shiro/blob/6e9a20ab/web/src/test/groovy/org/apache/shiro/web/env/IniWebEnvironmentTest.groovy ---------------------------------------------------------------------- diff --git a/web/src/test/groovy/org/apache/shiro/web/env/IniWebEnvironmentTest.groovy b/web/src/test/groovy/org/apache/shiro/web/env/IniWebEnvironmentTest.groovy index 08b2bc0..84b698d 100644 --- a/web/src/test/groovy/org/apache/shiro/web/env/IniWebEnvironmentTest.groovy +++ b/web/src/test/groovy/org/apache/shiro/web/env/IniWebEnvironmentTest.groovy @@ -18,18 +18,25 @@ */ package org.apache.shiro.web.env +import org.apache.shiro.config.CompositeBean import org.apache.shiro.config.Ini +import org.apache.shiro.config.SimpleBean import org.apache.shiro.web.filter.mgt.DefaultFilter +import org.junit.Test + +import static org.junit.Assert.* /** * Unit tests for the {@link IniWebEnvironment} implementation. - * + * * @since 1.2 */ -class IniWebEnvironmentTest extends GroovyTestCase { - - - //asserts SHIRO-306 +class IniWebEnvironmentTest { + + /** + * asserts SHIRO-306 + */ + @Test void testObjectsAfterSecurityManagerCreation() { def ini = new Ini() @@ -48,4 +55,43 @@ class IniWebEnvironmentTest extends GroovyTestCase { assertNotNull env.objects['securityManager'] assertNotNull env.objects['compositeBean'] } + + /** + * @since 1.4 + */ + @Test + void testFrameworkConfigAdded() { + + def ini = new Ini() + ini.load(""" + [main] + compositeBean = org.apache.shiro.config.CompositeBean + compositeBean.simpleBean = \$simpleBean + """) + + def env = new IniWebEnvironment() { + @Override + protected Ini getFrameworkIni() { + def frameworkIni = new Ini() + frameworkIni.setSectionProperty("main", "simpleBean", "org.apache.shiro.config.SimpleBean") + return frameworkIni; + } + } + env.ini = ini + env.init() + + assertNotNull env.objects + //asserts that the objects size = securityManager (1) + the event bus (1) + filterChainResolverFactory (1) + num custom objects + num default filters + def expectedSize = 5 + DefaultFilter.values().length + assertEquals expectedSize, env.objects.size() + assertNotNull env.objects['securityManager'] + + def compositeBean = (CompositeBean) env.objects['compositeBean'] + def simpleBean = (SimpleBean) env.objects['simpleBean'] + + assertNotNull compositeBean + assertNotNull simpleBean + + assertSame(compositeBean.simpleBean, simpleBean) + } }