TSK-59 ACCESS_ID should always be treated as lowercase - add taskanaEngineConfiguration.getUseContainerManagedTransactions

BerndBreier · holgerhagen · commit 00340044e2e4 · 2017-12-21T15:14:17.000+01:00
diff --git a/lib/taskana-core/src/main/java/pro/taskana/configuration/TaskanaEngineConfiguration.java b/lib/taskana-core/src/main/java/pro/taskana/configuration/TaskanaEngineConfiguration.java
@@ -7,12 +7,12 @@
 import org.apache.ibatis.datasource.pooled.PooledDataSource;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+
 import pro.taskana.TaskanaEngine;
 import pro.taskana.impl.TaskanaEngineImpl;
 
 /**
- * This central class creates the TaskanaEngine and needs all the information
- * about DB and Security.
+ * This central class creates the TaskanaEngine and holds all the information about DB and Security.
  */
 public class TaskanaEngineConfiguration {
 
@@ -29,19 +29,19 @@ public class TaskanaEngineConfiguration {
     // global switch to enable JAAS based authentication and Taskana
     // authorizations
     protected boolean securityEnabled;
-    protected boolean useContainerManagedTransactions;
+    protected boolean useManagedTransactions;
 
     public TaskanaEngineConfiguration() {
     }
 
     public TaskanaEngineConfiguration(DataSource dataSource, boolean useContainerManagedTransactions)
-            throws SQLException {
+        throws SQLException {
         this(dataSource, useContainerManagedTransactions, true);
     }
 
     public TaskanaEngineConfiguration(DataSource dataSource, boolean useContainerManagedTransactions,
-            boolean securityEnabled) throws SQLException {
-        this.useContainerManagedTransactions = useContainerManagedTransactions;
+        boolean securityEnabled) throws SQLException {
+        this.useManagedTransactions = useContainerManagedTransactions;
 
         if (dataSource != null) {
             this.dataSource = dataSource;
@@ -57,25 +57,30 @@ public TaskanaEngineConfiguration(DataSource dataSource, boolean useContainerMan
 
     public static DataSource createDefaultDataSource() {
         LOGGER.warn("No datasource is provided. A inmemory db is used: "
-                + "'org.h2.Driver', 'jdbc:h2:mem:taskana', 'sa', 'sa'");
+            + "'org.h2.Driver', 'jdbc:h2:mem:taskana', 'sa', 'sa'");
         return createDatasource(H2_DRIVER, JDBC_H2_MEM_TASKANA, USER_NAME, USER_PASSWORD);
     }
 
     /**
      * This method creates the TaskanaEngine without an sqlSessionFactory.
+     *
      * @return the TaskanaEngine
-     * @throws SQLException TODO
      */
-    public TaskanaEngine buildTaskanaEngine() throws SQLException {
+    public TaskanaEngine buildTaskanaEngine() {
         return new TaskanaEngineImpl(this);
     }
 
     /**
      * This method creates a PooledDataSource, if the needed properties are provided.
-     * @param driver TODO
-     * @param jdbcUrl TODO
-     * @param username TODO
-     * @param password TODO
+     *
+     * @param driver
+     *            the name of the jdbc driver
+     * @param jdbcUrl
+     *            the url to which the jdbc driver connects
+     * @param username
+     *            the user name for database access
+     * @param password
+     *            the password for database access
      * @return DataSource
      */
     public static DataSource createDatasource(String driver, String jdbcUrl, String username, String password) {
@@ -90,8 +95,17 @@ public DataSource getDatasource() {
         return this.dataSource;
     }
 
-    public boolean getUseContainerManagedTransactions() {
-        return this.useContainerManagedTransactions;
+    public boolean getUseManagedTransactions() {
+        return this.useManagedTransactions;
+    }
+
+    /**
+     * Helper method to determine whether all access ids (user Id and group ids) should be used in lower case.
+     *
+     * @return true if all access ids should be used in lower case, false otherwise
+     */
+    public static boolean shouldUseLowerCaseForAccessIds() {
+        return true;
     }
 
 }
diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/TaskanaEngineImpl.java b/lib/taskana-core/src/main/java/pro/taskana/impl/TaskanaEngineImpl.java
@@ -57,7 +57,7 @@ public class TaskanaEngineImpl implements TaskanaEngine {
 
     public TaskanaEngineImpl(TaskanaEngineConfiguration taskanaEngineConfiguration) {
         this.taskanaEngineConfiguration = taskanaEngineConfiguration;
-        createTransactionFactory(taskanaEngineConfiguration.getUseContainerManagedTransactions());
+        createTransactionFactory(taskanaEngineConfiguration.getUseManagedTransactions());
         this.sessionManager = createSqlSessionManager();
     }
 
diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketQueryImpl.java b/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketQueryImpl.java
@@ -11,6 +11,7 @@
 import pro.taskana.TaskanaEngine;
 import pro.taskana.Workbasket;
 import pro.taskana.WorkbasketQuery;
+import pro.taskana.configuration.TaskanaEngineConfiguration;
 import pro.taskana.exceptions.InvalidArgumentException;
 import pro.taskana.exceptions.NotAuthorizedException;
 import pro.taskana.impl.util.LoggerUtils;
@@ -117,13 +118,14 @@ public WorkbasketQuery access(WorkbasketAuthorization permission, String... acce
         }
         this.authorization = permission;
         this.accessId = accessIds;
-        for (int i = 0; i < accessIds.length; i++) {
-            String id = accessIds[i];
-            if (id != null) {
-                accessIds[i] = id.toLowerCase();
+        if (TaskanaEngineConfiguration.shouldUseLowerCaseForAccessIds()) {
+            for (int i = 0; i < accessIds.length; i++) {
+                String id = accessIds[i];
+                if (id != null) {
+                    accessIds[i] = id.toLowerCase();
+                }
             }
         }
-
         return this;
     }
 
diff --git a/lib/taskana-core/src/main/java/pro/taskana/model/WorkbasketAccessItem.java b/lib/taskana-core/src/main/java/pro/taskana/model/WorkbasketAccessItem.java
@@ -1,5 +1,7 @@
 package pro.taskana.model;
 
+import pro.taskana.configuration.TaskanaEngineConfiguration;
+
 /**
  * WorkbasketAccessItem entity.
  */
@@ -39,11 +41,19 @@ public void setWorkbasketKey(String workbasketKey) {
     }
 
     public String getAccessId() {
-        return accessId != null ? accessId.toLowerCase() : null;
+        if (TaskanaEngineConfiguration.shouldUseLowerCaseForAccessIds()) {
+            return accessId != null ? accessId.toLowerCase() : null;
+        } else {
+            return accessId;
+        }
     }
 
     public void setAccessId(String accessId) {
-        this.accessId = accessId != null ? accessId.toLowerCase() : null;
+        if (TaskanaEngineConfiguration.shouldUseLowerCaseForAccessIds()) {
+            this.accessId = accessId != null ? accessId.toLowerCase() : null;
+        } else {
+            this.accessId = accessId;
+        }
     }
 
     public boolean isPermRead() {
diff --git a/lib/taskana-core/src/main/java/pro/taskana/security/CurrentUserContext.java b/lib/taskana-core/src/main/java/pro/taskana/security/CurrentUserContext.java
@@ -13,6 +13,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import pro.taskana.configuration.TaskanaEngineConfiguration;
+
 /**
  * Provides the context information about the current (calling) user. The context is gathered from the JAAS subject.
  *
@@ -63,7 +65,10 @@ private static String getUseridFromWSSubject() {
                         (Object[]) null);
                     LOGGER.debug("Returning the unique security name of first public credential: {}", o);
                     String userIdFound = o.toString();
-                    String userIdUsed = userIdFound != null ? userIdFound.toLowerCase() : null;
+                    String userIdUsed = userIdFound;
+                    if (TaskanaEngineConfiguration.shouldUseLowerCaseForAccessIds() && userIdFound != null) {
+                        userIdUsed = userIdFound.toLowerCase();
+                    }
                     LOGGER.trace("Found User id {}. Returning User id {} ", userIdFound, userIdUsed);
                     return userIdUsed;
                 }
@@ -102,7 +107,10 @@ private static String getUseridFromJAASSubject() {
             for (Principal pC : principals) {
                 if (!(pC instanceof Group)) {
                     String userIdFound = pC.getName();
-                    String userIdUsed = userIdFound != null ? userIdFound.toLowerCase() : null;
+                    String userIdUsed = userIdFound;
+                    if (TaskanaEngineConfiguration.shouldUseLowerCaseForAccessIds() && userIdFound != null) {
+                        userIdUsed = userIdFound.toLowerCase();
+                    }
                     LOGGER.trace("Found User id {}. Returning User id {} ", userIdFound, userIdUsed);
                     return userIdUsed;
                 }
@@ -121,7 +129,10 @@ public static List<String> getGroupIds() {
             LOGGER.trace("Public groups of caller: {}", groups);
             for (Principal group : groups) {
                 String groupNameFound = group.getName();
-                String groupNameReturned = groupNameFound != null ? groupNameFound.toLowerCase() : null;
+                String groupNameReturned = groupNameFound;
+                if (TaskanaEngineConfiguration.shouldUseLowerCaseForAccessIds() && groupNameFound != null) {
+                    groupNameReturned = groupNameFound.toLowerCase();
+                }
                 LOGGER.trace("Found group id {}. Returning group Id: {}", groupNameFound, groupNameReturned);
                 groupIds.add(groupNameReturned);
             }
diff --git a/lib/taskana-core/src/test/java/pro/taskana/impl/WorkbasketServiceImplTest.java b/lib/taskana-core/src/test/java/pro/taskana/impl/WorkbasketServiceImplTest.java
@@ -335,7 +335,11 @@ public void should_ReturnWorkbasketAuthorization_when_WorkbasketAccessItemIsUpda
         accessItem.setAccessId("Zaphod Beeblebrox");
         workbasketServiceImpl.updateWorkbasketAuthorization(accessItem);
 
-        Assert.assertEquals("zaphod beeblebrox", accessItem.getAccessId());
+        if (TaskanaEngineConfiguration.shouldUseLowerCaseForAccessIds()) {
+            Assert.assertEquals("zaphod beeblebrox", accessItem.getAccessId());
+        } else {
+            Assert.assertEquals("Zaphod Beeblebrox", accessItem.getAccessId());
+        }
     }
 
     @Test(expected = NotAuthorizedException.class)
diff --git a/lib/taskana-core/src/test/java/pro/taskana/impl/integration/WorkbasketServiceImplIntAutocommitTest.java b/lib/taskana-core/src/test/java/pro/taskana/impl/integration/WorkbasketServiceImplIntAutocommitTest.java
@@ -299,8 +299,13 @@ public void testUpdateWorkbasketAccessUser() throws NotAuthorizedException {
         accessItem.setAccessId("Zaphod Beeblebrox");
         workBasketService.updateWorkbasketAuthorization(accessItem);
 
-        Assert.assertEquals("zaphod beeblebrox",
-            workBasketService.getWorkbasketAuthorization(accessItem.getId()).getAccessId());
+        if (TaskanaEngineConfiguration.shouldUseLowerCaseForAccessIds()) {
+            Assert.assertEquals("zaphod beeblebrox",
+                workBasketService.getWorkbasketAuthorization(accessItem.getId()).getAccessId());
+        } else {
+            Assert.assertEquals("zaphod beeblebrox",
+                workBasketService.getWorkbasketAuthorization(accessItem.getId()).getAccessId());
+        }
     }
 
     @Test
diff --git a/lib/taskana-core/src/test/java/pro/taskana/impl/integration/WorkbasketServiceImplIntExplicitTest.java b/lib/taskana-core/src/test/java/pro/taskana/impl/integration/WorkbasketServiceImplIntExplicitTest.java
@@ -251,7 +251,10 @@ public void testUpdateWorkbasket() throws Exception {
         workbasket2.getDistributionTargets().add(workbasket0);
         workbasket2.getDistributionTargets().add(workbasket1);
         workBasketService.createWorkbasket(workbasket2);
-        Workbasket workbasket3 = workBasketService.newWorkbasket();
+
+        WorkbasketImpl workbasket3 = (WorkbasketImpl) workBasketService.newWorkbasket();
+        String id3 = IdGenerator.generateWithPrefix("TWB");
+        workbasket3.setId(id3);
         workbasket3.setKey("key3");
         workbasket3.setName("hm ... irgend ein basket");
         workbasket3.setType(WorkbasketType.GROUP);
diff --git a/lib/taskana-spring/src/main/java/pro/taskana/configuration/SpringTaskanaEngineConfiguration.java b/lib/taskana-spring/src/main/java/pro/taskana/configuration/SpringTaskanaEngineConfiguration.java
@@ -23,7 +23,7 @@ public class SpringTaskanaEngineConfiguration extends TaskanaEngineConfiguration
 	 * @return the TaskanaEngine
 	 */
 	public TaskanaEngine buildTaskanaEngine() {
-		this.useContainerManagedTransactions = true;
+		this.useManagedTransactions = true;
 
 		dbScriptRunner = new DbScriptRunner(this.dataSource);
 		try {
diff --git a/qa/checkstyle/checkstyle.xml b/qa/checkstyle/checkstyle.xml
@@ -105,7 +105,6 @@
         <module name="EqualsHashCode"/>
         <module name="IllegalInstantiation"/>
         <module name="InnerAssignment"/>
-        <module name="MagicNumber"/>
         <module name="MissingSwitchDefault"/>
         <module name="SimplifyBooleanExpression"/>
         <module name="SimplifyBooleanReturn"/>
diff --git a/qa/eclipseFormatter/taskana_formatter.xml b/qa/eclipseFormatter/taskana_formatter.xml
@@ -52,7 +52,7 @@
 <setting id="org.eclipse.jdt.core.formatter.insert_new_line_in_empty_block" value="insert"/>
 <setting id="org.eclipse.jdt.core.formatter.insert_space_after_prefix_operator" value="do not insert"/>
 <setting id="org.eclipse.jdt.core.formatter.blank_lines_between_type_declarations" value="1"/>
-<setting id="org.eclipse.jdt.core.formatter.insert_space_before_closing_brace_in_array_initializer" value="insert"/>
+<setting id="org.eclipse.jdt.core.formatter.insert_space_before_closing_brace_in_array_initializer" value="do not insert"/>
 <setting id="org.eclipse.jdt.core.formatter.insert_space_before_closing_paren_in_for" value="do not insert"/>
 <setting id="org.eclipse.jdt.core.formatter.insert_space_after_opening_paren_in_catch" value="do not insert"/>
 <setting id="org.eclipse.jdt.core.formatter.insert_space_after_opening_angle_bracket_in_type_arguments" value="do not insert"/>
@@ -205,7 +205,7 @@
 <setting id="org.eclipse.jdt.core.formatter.insert_space_before_comma_in_superinterfaces" value="do not insert"/>
 <setting id="org.eclipse.jdt.core.formatter.insert_space_after_comma_in_method_declaration_parameters" value="insert"/>
 <setting id="org.eclipse.jdt.core.formatter.insert_new_line_after_type_annotation" value="do not insert"/>
-<setting id="org.eclipse.jdt.core.formatter.insert_space_after_opening_brace_in_array_initializer" value="insert"/>
+<setting id="org.eclipse.jdt.core.formatter.insert_space_after_opening_brace_in_array_initializer" value="do not insert"/>
 <setting id="org.eclipse.jdt.core.formatter.insert_space_before_opening_paren_in_parenthesized_expression" value="do not insert"/>
 <setting id="org.eclipse.jdt.core.formatter.comment.format_html" value="true"/>
 <setting id="org.eclipse.jdt.core.formatter.insert_space_after_at_in_annotation_type_declaration" value="do not insert"/>

Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,7 @@ public class TaskanaEngineImpl implements TaskanaEngine {`
`57`	`57`
`58`	`58`	`public TaskanaEngineImpl(TaskanaEngineConfiguration taskanaEngineConfiguration) {`
`59`	`59`	`this.taskanaEngineConfiguration = taskanaEngineConfiguration;`
`60`		`- createTransactionFactory(taskanaEngineConfiguration.getUseContainerManagedTransactions());`
	`60`	`+ createTransactionFactory(taskanaEngineConfiguration.getUseManagedTransactions());`
`61`	`61`	`this.sessionManager = createSqlSessionManager();`
`62`	`62`	`}`
`63`	`63`