Merge pull request #5 from mwoodiupui/DSRV-22

[DSRV-22] Detect indirect circular property references
mwoodiupui · Sep 14, 2012 · 83e8e9d · 83e8e9d
2 parents dd5f35c + 38be7a0
commit 83e8e9d
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 21 deletions.
diff --git a/impl/src/main/java/org/dspace/servicemanager/config/DSpaceConfigurationService.java b/impl/src/main/java/org/dspace/servicemanager/config/DSpaceConfigurationService.java
@@ -491,17 +491,20 @@ protected void replaceVariables(Map<String, DSpaceConfig> dsConfiguration) {
                     int end = value.indexOf('}', start);
                     if (end > -1) {
                         String newKey = value.substring(start+2, end);
-                        if (newKey.equals(entry.getKey())) {
-                            log.warn("Found circular reference for key ("+newKey+") in config value: " + value);
-                            break;
-                        }
                         DSpaceConfig dsc = dsConfiguration.get(newKey);
                         if (dsc == null) {
                             log.warn("Could not find key ("+newKey+") for replacement in value: " + value);
                             break;
                         }
                         String newVal = dsc.getValue();
+                        String oldValue = value;
                         value = value.replace("${"+newKey+"}", newVal);
+                        if (value.equals(oldValue)) {
+                            log.warn("No change after variable replacement -- is "
+                                    + newKey + " = " + newVal +
+                                    " a circular reference?");
+                            break;
+                        }
                         entry.setValue( new DSpaceConfig(entry.getValue().getKey(), value) );
                     } else {
                         log.warn("Found '${' but could not find a closing '}' in the value: " + value);

diff --git a/impl/src/test/java/org/dspace/servicemanager/config/DSpaceConfigurationServiceTest.java b/impl/src/test/java/org/dspace/servicemanager/config/DSpaceConfigurationServiceTest.java
@@ -54,7 +54,7 @@ public void tearDown() {
     /**
      * Test method for {@link org.dspace.servicemanager.config.DSpaceConfigurationService#replaceVariables(java.util.Map)}.
      */
-    @Test
+    @Test(timeout=10000)
     public void testReplaceVariables() {
 
         List<DSpaceConfig> l = new ArrayList<DSpaceConfig>();
@@ -64,20 +64,29 @@ public void testReplaceVariables() {
         l.add( new DSpaceConfig("test.key1", "This is a value") );
         l.add( new DSpaceConfig("test.key2", "This is key1=${test.key1}") );
         l.add( new DSpaceConfig("test.key3", "This is key2=${test.key2}") );
+        int dirIdx = l.size();
+        l.add( new DSpaceConfig("circular", "${circular}"));
+        int indirIdx = l.size();
+        l.add( new DSpaceConfig("indirect.circular", "${circular} square"));
 
         Map<String, DSpaceConfig> configMap = new HashMap<String, DSpaceConfig>();
         for (DSpaceConfig config : l) {
             configMap.put(config.getKey(), config);
         }
         configurationService.replaceVariables(configMap);
 
-        assertEquals(6, configMap.size());
+        assertEquals("all configuration list members should be map members",
+                l.size(), configMap.size());
         assertEquals("DSpace", configMap.get("service.name").getValue());
         assertEquals("Aaron Zeckoski", configMap.get("aaronz").getValue());
         assertEquals("Aaron Zeckoski", configMap.get("current.user").getValue());
         assertEquals("This is a value", configMap.get("test.key1").getValue());
         assertEquals("This is key1=This is a value", configMap.get("test.key2").getValue());
         assertEquals("This is key2=This is key1=This is a value", configMap.get("test.key3").getValue());
+        assertEquals("Direct circular reference should not be replaced",
+                configMap.get("circular").getValue(), l.get(dirIdx).getValue());
+        assertEquals("Indirect circular reference should not be replaced",
+                configMap.get("indirect.circular").getValue(), l.get(indirIdx).getValue());
     }
 
     /**

diff --git a/pom.xml b/pom.xml
@@ -3,6 +3,7 @@
     <modelVersion>4.0.0</modelVersion>
     <groupId>org.dspace</groupId>
     <artifactId>dspace-services</artifactId>
+    <version>2.0.5-SNAPSHOT</version>
     <packaging>pom</packaging>
     <name>DSpace Services Modules</name>
     <url>http://projects.dspace.org/dspace-services</url>
@@ -13,12 +14,71 @@
     </organization>
 
     <parent>
-        <artifactId>dspace-pom</artifactId>
-        <groupId>org.dspace</groupId>
-        <version>10</version>
+        <groupId>org.sonatype.oss</groupId>
+        <artifactId>oss-parent</artifactId>
+        <version>7</version>
     </parent>
-
-    <version>2.0.5-SNAPSHOT</version>
+
+    <properties>
+        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+        <maven.compiler.source>1.6</maven.compiler.source>
+        <maven.compiler.target>1.6</maven.compiler.target>
+    </properties>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>com.mycila.maven-license-plugin</groupId>
+                <artifactId>maven-license-plugin</artifactId>
+                <version>1.9.0</version>
+                <configuration>
+                    <!-- external and absolute URL for use in external addon builds -->
+                    <header>https://raw.github.com/DSpace/DSpace/master/LICENSE_HEADER</header>
+                    <!--Just check headers of everything in the /src directory -->
+                    <includes>
+                        <include>src/**</include>
+                    </includes>
+                    <!--Use all default exclusions for IDE files & Maven files, see:
+                        http://code.google.com/p/maven-license-plugin/wiki/Configuration#Default_excludes -->
+                    <useDefaultExcludes>true</useDefaultExcludes>
+                    <!-- Add some default DSpace exclusions not covered by <useDefaultExcludes>
+                         Individual Maven projects may choose to override these defaults. -->
+                    <excludes>
+                        <exclude>**/src/test/resources/**</exclude>
+                        <exclude>**/src/test/data/**</exclude>
+                        <exclude>**/META-INF/**</exclude>
+                        <exclude>**/robots.txt</exclude>
+                        <exclude>**/*.LICENSE</exclude>
+                        <exclude>**/LICENSE*</exclude>
+                        <exclude>**/README*</exclude>
+                        <exclude>**/readme*</exclude>
+                        <exclude>**/.gitignore</exclude>
+                    </excludes>
+                    <mapping>
+                        <!-- Custom DSpace file extensions which are not recognized by maven-release-plugin:
+                             *.xmap, *.xslt, *.wsdd, *.wsdl, *.LICENSE -->
+                        <xmap>XML_STYLE</xmap>
+                        <xslt>XML_STYLE</xslt>
+                        <wsdd>XML_STYLE</wsdd>
+                        <wsdl>XML_STYLE</wsdl>
+                        <LICENSE>TEXT</LICENSE>
+                    </mapping>
+                    <encoding>UTF-8</encoding>
+                    <!-- maven-license-plugin recommends a strict check (e.g. check spaces/tabs too) -->
+                    <strictCheck>true</strictCheck>
+                </configuration>
+                <executions>
+                    <execution>
+                        <id>check-headers</id>
+                        <phase>verify</phase>
+                        <goals>
+                            <goal>check</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
+        </plugins>
+    </build>
 
     <modules>
         <module>api</module>
@@ -70,16 +130,10 @@
         </contributor>
     </contributors>
 
-    <!--
-        The Subversion repository location is used by Continuum to
-        update against when changes have occured, this spawns a new
-        build cycle and releases snapshots into the snapshot repository
-        below.
-    -->
     <scm>
-        <connection>scm:svn:http://scm.dspace.org/svn/repo/modules/dspace-services/trunk</connection>
-        <developerConnection>scm:svn:https://scm.dspace.org/svn/repo/modules/dspace-services/trunk</developerConnection>
-        <url>http://scm.dspace.org/svn/repo/modules/dspace-services/trunk</url>
+      <connection>scm:git:git://github.com/DSpace/dspace-services.git</connection>
+      <developerConnection>scm:git:git@github.com:DSpace/dspace-services.git</developerConnection>
+      <url>https://github.com/DSpace/dspace-services</url>
     </scm>
 
-</project>
+</project>