From dd0ad1253a58ff130c0327045ea5b9e5db7eea17 Mon Sep 17 00:00:00 2001 From: BVier <26220150+BVier@users.noreply.github.com> Date: Wed, 6 Jun 2018 10:55:08 +0200 Subject: [PATCH] TSK-520: Replaced code smells found by FindBugs --- .../pro/taskana/impl/ClassificationServiceImpl.java | 11 +++++++++-- .../main/java/pro/taskana/impl/TaskServiceImpl.java | 2 +- .../java/pro/taskana/security/GroupPrincipal.java | 2 ++ .../java/pro/taskana/database/TestDataGenerator.java | 9 ++++++--- .../pro/taskana/rest/AbstractPagingController.java | 4 +++- 5 files changed, 21 insertions(+), 7 deletions(-) diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/ClassificationServiceImpl.java b/lib/taskana-core/src/main/java/pro/taskana/impl/ClassificationServiceImpl.java index e153bed43..51f07b3c4 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/impl/ClassificationServiceImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/impl/ClassificationServiceImpl.java @@ -98,7 +98,7 @@ public class ClassificationServiceImpl implements ClassificationService { } private void addClassificationToRootDomain(ClassificationImpl classificationImpl) { - if (classificationImpl.getDomain() != "") { + if (!classificationImpl.getDomain().equals("")) { boolean doesExist = true; String idBackup = classificationImpl.getId(); String domainBackup = classificationImpl.getDomain(); @@ -178,7 +178,14 @@ public class ClassificationServiceImpl implements ClassificationService { } classificationMapper.update(classificationImpl); boolean priorityChanged = oldClassification.getPriority() != classification.getPriority(); - boolean serviceLevelChanged = oldClassification.getServiceLevel() != classification.getServiceLevel(); + boolean serviceLevelChanged = true; + if (oldClassification.getServiceLevel() == null) { + if (classification.getServiceLevel() == null) { + serviceLevelChanged = false; + } + } else if (oldClassification.getServiceLevel().equals(classification.getServiceLevel())) { + serviceLevelChanged = false; + } if (priorityChanged || serviceLevelChanged) { Map args = new HashMap<>(); diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/TaskServiceImpl.java b/lib/taskana-core/src/main/java/pro/taskana/impl/TaskServiceImpl.java index 22632929f..9780aacd4 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/impl/TaskServiceImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/impl/TaskServiceImpl.java @@ -297,7 +297,7 @@ public class TaskServiceImpl implements TaskService { TaskImpl task = (TaskImpl) taskToCreate; try { taskanaEngine.openConnection(); - if (task.getId() != "" && task.getId() != null) { + if (task.getId() != null && !task.getId().equals("")) { throw new TaskAlreadyExistException(task.getId()); } else { LOGGER.debug("Task {} cannot be be found, so it can be created.", task.getId()); diff --git a/lib/taskana-core/src/main/java/pro/taskana/security/GroupPrincipal.java b/lib/taskana-core/src/main/java/pro/taskana/security/GroupPrincipal.java index dab882869..dad8a4729 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/security/GroupPrincipal.java +++ b/lib/taskana-core/src/main/java/pro/taskana/security/GroupPrincipal.java @@ -4,6 +4,7 @@ import java.security.Principal; import java.security.acl.Group; import java.util.Collections; import java.util.Enumeration; +import java.util.HashSet; import java.util.Set; /** @@ -16,6 +17,7 @@ public class GroupPrincipal implements Group { public GroupPrincipal(String name) { this.name = name; + this.members = new HashSet(); } @Override diff --git a/lib/taskana-core/src/test/java/pro/taskana/database/TestDataGenerator.java b/lib/taskana-core/src/test/java/pro/taskana/database/TestDataGenerator.java index 2a1f3d628..a7d1d1246 100644 --- a/lib/taskana-core/src/test/java/pro/taskana/database/TestDataGenerator.java +++ b/lib/taskana-core/src/test/java/pro/taskana/database/TestDataGenerator.java @@ -79,7 +79,9 @@ public class TestDataGenerator { .forEach(runner::runScript); } finally { - runner.closeConnection(); + if (runner != null) { + runner.closeConnection(); + } LOGGER.debug(outWriter.toString()); if (!errorWriter.toString().trim().isEmpty()) { LOGGER.error(errorWriter.toString()); @@ -109,8 +111,9 @@ public class TestDataGenerator { new ByteArrayInputStream( sqlReplacer.monitoringTestDataSql.getBytes(StandardCharsets.UTF_8)))); } finally { - - runner.closeConnection(); + if (runner != null) { + runner.closeConnection(); + } LOGGER.debug(outWriter.toString()); if (!errorWriter.toString().trim().isEmpty()) { LOGGER.error(errorWriter.toString()); diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/AbstractPagingController.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/AbstractPagingController.java index dae803857..3e89f4f4d 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/AbstractPagingController.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/rest/AbstractPagingController.java @@ -34,7 +34,9 @@ public abstract class AbstractPagingController { protected String[] extractCommaSeparatedFields(List list) { List values = new ArrayList<>(); - list.forEach(item -> values.addAll(Arrays.asList(item.split(",")))); + if (list != null) { + list.forEach(item -> values.addAll(Arrays.asList(item.split(",")))); + } return values.toArray(new String[0]); }