Reduce code smells (#2089)

Reduce code smells found by deepsource.io. We focused on the low-hanging fruits and not breaking any api.
This commit is contained in:
Eduard Heimbuch
2022-07-15 15:33:37 +02:00
committed by GitHub
parent f61d0c113f
commit 67c083ee54
15 changed files with 149 additions and 443 deletions

View File

@@ -0,0 +1,2 @@
- type: fixed
description: Reduce code smells ([#2089](https://github.com/scm-manager/scm-manager/pull/2089))

View File

@@ -36,17 +36,21 @@ import sonia.scm.util.ServiceUtil;
*
* @author Sebastian Sdorra
*/
public final class SCMContext
{
public final class SCMContext {
/** Default java package for finding extensions */
/**
* Default java package for finding extensions
*/
public static final String DEFAULT_PACKAGE = "sonia.scm";
/** Name of the anonymous user */
/**
* Name of the anonymous user
*/
public static final String USER_ANONYMOUS = "_anonymous";
/**
* the anonymous user
*
* @since 1.21
*/
public static final User ANONYMOUS = new User(
@@ -58,42 +62,36 @@ public final class SCMContext
true
);
/** Singleton instance of {@link SCMContextProvider} */
private static volatile SCMContextProvider provider;
/**
* Singleton instance of {@link SCMContextProvider}
*/
private static SCMContextProvider provider = null;
//~--- constructors ---------------------------------------------------------
/**
* Constructs ...
*
*/
private SCMContext() {}
private SCMContext() {
}
//~--- get methods ----------------------------------------------------------
/**
* Returns the singleton instance of {@link SCMContextProvider}
*
*
* @return singleton instance of {@link SCMContextProvider}
*/
public static SCMContextProvider getContext()
{
if (provider == null)
{
synchronized (SCMContext.class)
{
if (provider == null)
{
public static SCMContextProvider getContext() {
synchronized (SCMContext.class) {
if (provider == null) {
provider = ServiceUtil.getService(SCMContextProvider.class);
if (provider == null)
{
if (provider == null) {
provider = new BasicContextProvider();
}
}
}
}
return provider;
}

View File

@@ -43,17 +43,17 @@ import java.util.ServiceLoader;
* themselves. The ScmEventBus searches its implementation with the
* {@link ServiceLoader}.
*
* @see EventBus
* @author Sebastian Sdorra
* @since 1.23
*
* @apiviz.landmark
* @see EventBus
* @since 1.23
*/
public abstract class ScmEventBus
{
public abstract class ScmEventBus {
/** Field description */
private volatile static ScmEventBus instance;
/**
* Field description
*/
private static ScmEventBus instance;
/**
* the logger for ScmEventBus
@@ -66,32 +66,22 @@ public abstract class ScmEventBus
/**
* Returns the singleton instance of the ScmEventBus
*
*
* @return singleton instance
*/
public static ScmEventBus getInstance()
{
if (instance == null)
{
synchronized (ScmEventBus.class)
{
if (instance == null)
{
public static ScmEventBus getInstance() {
synchronized (ScmEventBus.class) {
if (instance == null) {
instance = ServiceUtil.getService(ScmEventBus.class);
if (instance == null)
{
if (instance == null) {
throw new IllegalStateException(
"could not find a event bus implementation");
}
else
{
} else {
logger.info("use {} as event bus implementation",
instance.getClass().getName());
}
}
}
}
return instance;
}

View File

@@ -39,13 +39,13 @@ import java.io.IOException;
import java.util.Scanner;
/**
*
* @author Sebastian Sdorra
*/
public final class RegistryUtil
{
public final class RegistryUtil {
/** the logger for RegistryUtil */
/**
* the logger for RegistryUtil
*/
private static final Logger logger =
LoggerFactory.getLogger(RegistryUtil.class);
@@ -53,90 +53,72 @@ public final class RegistryUtil
/**
* Constructs ...
*
*/
private RegistryUtil() {}
private RegistryUtil() {
}
//~--- get methods ----------------------------------------------------------
/**
* Method description
*
*
* @param key
*
* @return
*/
public static String getRegistryValue(String key)
{
public static String getRegistryValue(String key) {
return getRegistryValue(key, null, null);
}
/**
* Method description
*
*
* @param key
* @param defaultValue
*
* @return
*/
public static String getRegistryValue(String key, String defaultValue)
{
public static String getRegistryValue(String key, String defaultValue) {
return getRegistryValue(key, null, defaultValue);
}
/**
* Method description
*
*
* @param key
* @param subKey
* @param defaultValue
*
* @return
*/
public static String getRegistryValue(String key, String subKey,
String defaultValue)
{
String defaultValue) {
String value = defaultValue;
SimpleCommand command = null;
if (subKey != null)
{
if (subKey != null) {
command = new SimpleCommand("reg", "query", key, "/v", subKey);
}
else
{
} else {
command = new SimpleCommand("reg", "query", key, "/ve");
}
try
{
try {
SimpleCommandResult result = command.execute();
if (result.isSuccessfull())
{
if (result.isSuccessfull()) {
String output = result.getOutput();
Scanner scanner = new Scanner(output);
try (Scanner scanner = new Scanner(output)) {
while (scanner.hasNextLine())
{
while (scanner.hasNextLine()) {
String line = scanner.nextLine();
int index = line.indexOf("REG_SZ");
if (index > 0)
{
if (index > 0) {
value = line.substring(index + "REG_SZ".length()).trim();
if (value.startsWith("\""))
{
if (value.startsWith("\"")) {
value = value.substring(1);
value = value.substring(0, value.indexOf('"'));
}
if (logger.isDebugEnabled())
{
if (logger.isDebugEnabled()) {
logger.debug("registry value {} at {}", value, key);
}
@@ -145,8 +127,7 @@ public final class RegistryUtil
}
}
}
catch (IOException ex)
{
} catch (IOException ex) {
logger.error(ex.getMessage(), ex);
}

View File

@@ -66,8 +66,8 @@ public class PathBasedRepositoryLocationResolver extends BasicRepositoryLocation
private final Clock clock;
private Long creationTime;
private Long lastModified;
private long creationTime;
private long lastModified;
@Inject
public PathBasedRepositoryLocationResolver(SCMContextProvider contextProvider, InitialRepositoryLocationResolver initialRepositoryLocationResolver, FileSystem fileSystem) {
@@ -162,7 +162,7 @@ public class PathBasedRepositoryLocationResolver extends BasicRepositoryLocation
}
}
private void onLoadDates(Long creationTime, Long lastModified) {
private void onLoadDates(long creationTime, long lastModified) {
this.creationTime = creationTime;
this.lastModified = lastModified;
}

View File

@@ -63,7 +63,7 @@ class PathDatabase {
this.storePath = storePath;
}
void write(Long creationTime, Long lastModified, Map<String, Path> pathDatabase) {
void write(long creationTime, long lastModified, Map<String, Path> pathDatabase) {
ensureParentDirectoryExists();
LOG.trace("write repository path database to {}", storePath);
@@ -107,7 +107,7 @@ class PathDatabase {
}
}
private void writeRepositoriesStart(XMLStreamWriter writer, Long creationTime, Long lastModified) throws XMLStreamException {
private void writeRepositoriesStart(XMLStreamWriter writer, long creationTime, long lastModified) throws XMLStreamException {
writer.writeStartElement(ELEMENT_REPOSITORIES);
writer.writeAttribute(ATTRIBUTE_CREATION_TIME, String.valueOf(creationTime));
writer.writeAttribute(ATTRIBUTE_LAST_MODIFIED, String.valueOf(lastModified));

View File

@@ -92,11 +92,11 @@ public class RepositoryUtil {
* @throws IOException
*/
public static Changeset commitMultipleFileModifications(RepositoryClient repositoryClient, String username, Map<String, String> addedFiles, Map<String, String> modifiedFiles, List<String> removedFiles) throws IOException {
for (String fileName : addedFiles.keySet()) {
writeAndAddFile(repositoryClient, fileName, addedFiles.get(fileName));
for (Map.Entry<String,String> entry : addedFiles.entrySet()) {
writeAndAddFile(repositoryClient, entry.getKey(), entry.getValue());
}
for (String fileName : modifiedFiles.keySet()) {
writeAndAddFile(repositoryClient, fileName, modifiedFiles.get(fileName));
for (Map.Entry<String,String> entry : modifiedFiles.entrySet()) {
writeAndAddFile(repositoryClient, entry.getKey(), entry.getValue());
}
for (String fileName : removedFiles) {
deleteFileAndApplyRemoveCommand(repositoryClient, fileName);

View File

@@ -80,7 +80,7 @@ public class ScmFileTransferServlet extends HttpServlet {
* Gson is used because the implementation was based on the jgit implementation. However the {@link LfsProtocolServlet} (which we do use in
* {@link ScmLfsProtocolServlet}) also uses Gson, which currently ties us to Gson anyway.
*/
private static Gson gson = createGson();
private static final Gson gson = createGson();
private final BlobStore blobStore;

View File

@@ -99,7 +99,7 @@ public class GitHookTagProviderTest {
public void testGetCreatedTags() {
try (MockedStatic<GitUtil> dummy = Mockito.mockStatic(GitUtil.class)) {
String revision = REVISION_1;
Long timestamp = 1339416344000L;
long timestamp = 1339416344000L;
String tagName = "1.0.0";
String ref = "refs/tags/" + tagName;
@@ -121,7 +121,7 @@ public class GitHookTagProviderTest {
public void testGetDeletedTags() {
try (MockedStatic<GitUtil> dummy = Mockito.mockStatic(GitUtil.class)) {
String revision = REVISION_1;
Long timestamp = 1339416344000L;
long timestamp = 1339416344000L;
String tagName = "1.0.0";
String ref = "refs/tags/" + tagName;
@@ -157,7 +157,7 @@ public class GitHookTagProviderTest {
String oldRevision = REVISION_2;
String newRevision = REVISION_1;
Long timestamp = 1339416344000L;
long timestamp = 1339416344000L;
String tagName = "1.0.0";
String ref = "refs/tags/" + tagName;
@@ -183,7 +183,7 @@ public class GitHookTagProviderTest {
String oldRevision = REVISION_2;
String newRevision = REVISION_1;
Long timestamp = 1339416344000L;
long timestamp = 1339416344000L;
String tagName = "1.0.0";
String ref = "refs/tags/" + tagName;
@@ -205,8 +205,8 @@ public class GitHookTagProviderTest {
try (MockedStatic<GitUtil> dummy = Mockito.mockStatic(GitUtil.class)) {
String revisionOfTag = REVISION_1;
String revisionOfCommit = REVISION_2;
Long timestampOfTag = 6666666000L;
Long timestampOfCommit = 1339416344000L;
long timestampOfTag = 6666666000L;
long timestampOfCommit = 1339416344000L;
String tagName = "1.0.0";
String ref = "refs/tags/" + tagName;

View File

@@ -973,36 +973,6 @@ public class SCMSvnDiffGenerator implements ISvnDiffGenerator {
}
}
private String getGitDiffLabel1(SvnDiffCallback.OperationKind operationKind, String path1, String path2, String copyFromPath, String revision) {
if (operationKind == SvnDiffCallback.OperationKind.Deleted) {
return getLabel("a/" + path1, revision);
} else if (operationKind == SvnDiffCallback.OperationKind.Copied) {
return getLabel("a/" + copyFromPath, revision);
} else if (operationKind == SvnDiffCallback.OperationKind.Added) {
return getLabel("/dev/null", revision);
} else if (operationKind == SvnDiffCallback.OperationKind.Modified) {
return getLabel("a/" + path1, revision);
} else if (operationKind == SvnDiffCallback.OperationKind.Moved) {
return getLabel("a/" + copyFromPath, revision);
}
throw new IllegalArgumentException("Unsupported operation: " + operationKind);
}
private String getGitDiffLabel2(SvnDiffCallback.OperationKind operationKind, String path1, String path2, String copyFromPath, String revision) {
if (operationKind == SvnDiffCallback.OperationKind.Deleted) {
return getLabel("/dev/null", revision);
} else if (operationKind == SvnDiffCallback.OperationKind.Copied) {
return getLabel("b/" + path2, revision);
} else if (operationKind == SvnDiffCallback.OperationKind.Added) {
return getLabel("b/" + path2, revision);
} else if (operationKind == SvnDiffCallback.OperationKind.Modified) {
return getLabel("b/" + path2, revision);
} else if (operationKind == SvnDiffCallback.OperationKind.Moved) {
return getLabel("b/" + path2, revision);
}
throw new IllegalArgumentException("Unsupported operation: " + operationKind);
}
private void displayGitDiffHeader(OutputStream outputStream, SvnDiffCallback.OperationKind operationKind, String path1, String path2, String copyFromPath) throws SVNException {
if (operationKind == SvnDiffCallback.OperationKind.Deleted) {
displayGitDiffHeaderDeleted(outputStream, path1, path2, copyFromPath);
@@ -1126,11 +1096,6 @@ public class SCMSvnDiffGenerator implements ISvnDiffGenerator {
displayString(outputStream, path1);
}
private String getAdjustedPathWithLabel(String displayPath, String path, String revision, String commonAncestor) {
String adjustedPath = getAdjustedPath(displayPath, path, commonAncestor);
return getLabel(adjustedPath, revision);
}
private String getAdjustedPath(String displayPath, String path1, String commonAncestor) {
String adjustedPath = getRelativePath(path1, commonAncestor);

View File

@@ -24,17 +24,12 @@
package sonia.scm.web;
//~--- non-JDK imports --------------------------------------------------------
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableList.Builder;
import com.google.common.collect.Ordering;
import com.google.inject.Inject;
import com.google.inject.Provider;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.tmatesoft.svn.core.SVNDirEntry;
import org.tmatesoft.svn.core.SVNErrorMessage;
import org.tmatesoft.svn.core.SVNException;
@@ -43,8 +38,6 @@ import org.tmatesoft.svn.core.internal.server.dav.CollectionRenderer;
import org.tmatesoft.svn.core.internal.server.dav.DAVPathUtil;
import org.tmatesoft.svn.core.internal.server.dav.DAVResource;
import org.tmatesoft.svn.core.internal.server.dav.DAVResourceURI;
import sonia.scm.config.ScmConfiguration;
import sonia.scm.repository.Repository;
import sonia.scm.repository.RepositoryProvider;
import sonia.scm.template.Template;
@@ -52,122 +45,67 @@ import sonia.scm.template.TemplateEngine;
import sonia.scm.template.TemplateEngineFactory;
import sonia.scm.util.HttpUtil;
//~--- JDK imports ------------------------------------------------------------
import java.io.IOException;
import java.io.StringWriter;
import java.util.Iterator;
import java.util.List;
import javax.servlet.http.HttpServletRequest;
/**
*
* @author Sebastian Sdorra
*/
public class SvnCollectionRenderer implements CollectionRenderer
{
public class SvnCollectionRenderer implements CollectionRenderer {
/** Field description */
private static final String RESOURCE_SVNINDEX =
"/sonia/scm/svn.index.mustache";
private final TemplateEngineFactory templateEngineFactory;
private final RepositoryProvider repositoryProvider;
private static final String RESOURCE_SVNINDEX = "/sonia/scm/svn.index.mustache";
/**
* the logger for SvnCollectionRenderer
*/
private static final Logger logger =
LoggerFactory.getLogger(SvnCollectionRenderer.class);
private static final Logger LOG = LoggerFactory.getLogger(SvnCollectionRenderer.class);
//~--- constructors ---------------------------------------------------------
/**
* Constructs ...
*
*
*
* @param configuration
* @param templateEngineFactory
* @param repositoryProvider
* @param requestProvider
*/
@Inject
public SvnCollectionRenderer(ScmConfiguration configuration,
TemplateEngineFactory templateEngineFactory,
RepositoryProvider repositoryProvider,
Provider<HttpServletRequest> requestProvider)
{
this.configuration = configuration;
public SvnCollectionRenderer(TemplateEngineFactory templateEngineFactory,
RepositoryProvider repositoryProvider) {
this.templateEngineFactory = templateEngineFactory;
this.repositoryProvider = repositoryProvider;
this.requestProvider = requestProvider;
}
//~--- methods --------------------------------------------------------------
/**
* Method description
*
*
* @param buffer
* @param resource
*
* @throws SVNException
*/
@Override
@SuppressWarnings("java:S2139")
public void renderCollection(StringBuilder buffer, DAVResource resource)
throws SVNException
{
throws SVNException {
TemplateEngine engine = templateEngineFactory.getDefaultEngine();
StringWriter writer = new StringWriter();
try
{
try {
Template template = engine.getTemplate(RESOURCE_SVNINDEX);
template.execute(writer, createRepositoryWrapper(resource));
}
catch (IOException ex)
{
logger.error("could not render directory", ex);
} catch (IOException ex) {
LOG.error("could not render directory", ex);
throw new SVNException(SVNErrorMessage.UNKNOWN_ERROR_MESSAGE, ex);
}
writer.flush();
buffer.append(writer.toString());
buffer.append(writer);
}
/**
* Method description
*
*
* @param resource
*
* @return
*
* @throws SVNException
*/
private RepositoryWrapper createRepositoryWrapper(DAVResource resource)
throws SVNException
{
throws SVNException {
Builder<DirectoryEntry> entries = ImmutableList.builder();
DAVResourceURI uri = resource.getResourceURI();
String path = uri.getPath();
if (!HttpUtil.SEPARATOR_PATH.equals(path))
{
if (!HttpUtil.SEPARATOR_PATH.equals(path)) {
String completePath = HttpUtil.append(uri.getContext(), path);
String parent = DAVPathUtil.removeTail(completePath, true);
entries.add(new DirectoryEntry("..", parent, true));
}
for (Iterator iterator = resource.getEntries().iterator();
iterator.hasNext(); )
{
SVNDirEntry entry = (SVNDirEntry) iterator.next();
for (Object o : resource.getEntries()) {
SVNDirEntry entry = (SVNDirEntry) o;
entries.add(new DirectoryEntry(resource, entry));
}
@@ -175,276 +113,109 @@ public class SvnCollectionRenderer implements CollectionRenderer
//J-
return new RepositoryWrapper(
repositoryProvider.get(),
resource,
new DirectoryOrdering().immutableSortedCopy(entries.build())
);
//J+
}
private static class DirectoryEntry {
private final boolean directory;
private final String name;
private final String url;
private String getBaseUrl() {
return HttpUtil.getCompleteUrl(requestProvider.get());
}
//~--- inner classes --------------------------------------------------------
/**
* Class description
*
*
* @version Enter version here..., 13/11/10
* @author Enter your name here...
*/
private static class DirectoryEntry
{
/**
* Constructs ...
*
*
* @param resource
* @param entry
*/
public DirectoryEntry(DAVResource resource, SVNDirEntry entry)
{
public DirectoryEntry(DAVResource resource, SVNDirEntry entry) {
this.name = entry.getName();
this.url = createUrl(resource, entry);
this.directory = entry.getKind() == SVNNodeKind.DIR;
}
/**
* Constructs ...
*
*
* @param name
* @param url
* @param directory
*/
public DirectoryEntry(String name, String url, boolean directory)
{
public DirectoryEntry(String name, String url, boolean directory) {
this.name = name;
this.url = url;
this.directory = directory;
}
//~--- get methods --------------------------------------------------------
/**
* Method description
*
*
* @return
*/
public String getName()
{
public String getName() {
return name;
}
/**
* Method description
*
*
* @return
*/
public String getUrl()
{
public String getUrl() {
return url;
}
/**
* Method description
*
*
* @return
*/
public boolean isDirectory()
{
public boolean isDirectory() {
return directory;
}
//~--- methods ------------------------------------------------------------
/**
* Method description
*
*
* @param resource
* @param entry
*
* @return
*/
private String createUrl(DAVResource resource, SVNDirEntry entry)
{
private String createUrl(DAVResource resource, SVNDirEntry entry) {
StringBuilder buffer = new StringBuilder();
buffer.append(resource.getResourceURI().getContext());
String path = resource.getResourceURI().getPath();
if (!HttpUtil.SEPARATOR_PATH.equals(path))
{
if (!HttpUtil.SEPARATOR_PATH.equals(path)) {
buffer.append(path);
}
buffer.append(DAVPathUtil.standardize(entry.getName()));
if (isDirectory())
{
if (isDirectory()) {
buffer.append(HttpUtil.SEPARATOR_PATH);
}
return buffer.toString();
}
//~--- fields -------------------------------------------------------------
/** Field description */
private final boolean directory;
/** Field description */
private final String name;
/** Field description */
private final String url;
}
private static class DirectoryOrdering extends Ordering<DirectoryEntry> {
/**
* Class description
*
*
* @version Enter version here..., 13/11/10
* @author Enter your name here...
*/
private static class DirectoryOrdering extends Ordering<DirectoryEntry>
{
/**
* Method description
*
*
* @param left
* @param right
*
* @return
*/
@Override
public int compare(DirectoryEntry left, DirectoryEntry right)
{
int result;
public int compare(DirectoryEntry left, DirectoryEntry right) {
int result = 0;
if (left.isDirectory() &&!right.isDirectory())
{
if (left == null || right == null) {
return result;
}
if (left.isDirectory() && !right.isDirectory()) {
result = -1;
}
else if (!left.isDirectory() && right.isDirectory())
{
} else if (!left.isDirectory() && right.isDirectory()) {
result = 1;
}
else
{
if ("..".equals(left.getName()))
{
} else {
if ("..".equals(left.getName())) {
result = -1;
}
else if ("..".equals(right.getName()))
{
} else if ("..".equals(right.getName())) {
result = 1;
}
else
{
} else {
result = left.getName().compareTo(right.getName());
}
}
return result;
}
}
private static class RepositoryWrapper {
private final List<DirectoryEntry> entries;
/**
* Class description
*
*
* @version Enter version here..., 13/11/10
* @author Enter your name here...
*/
private static class RepositoryWrapper
{
private final Repository repository;
/**
* Constructs ...
*
*
*
* @param repository
* @param resource
* @param entries
*/
public RepositoryWrapper(Repository repository, DAVResource resource, List<DirectoryEntry> entries)
{
public RepositoryWrapper(Repository repository, List<DirectoryEntry> entries) {
this.repository = repository;
this.resource = resource;
this.entries = entries;
}
//~--- get methods --------------------------------------------------------
/**
* Method description
*
*
* @return
*/
public List<DirectoryEntry> getEntries()
{
public List<DirectoryEntry> getEntries() {
return entries;
}
/**
* Method description
*
*
* @return
*/
public String getName()
{
public String getName() {
return repository.getName();
}
/**
* Method description
*
*
* @return
*/
public Repository getRepository()
{
public Repository getRepository() {
return repository;
}
//~--- fields -------------------------------------------------------------
/** Field description */
private final List<DirectoryEntry> entries;
/** Field description */
private final Repository repository;
/** Field description */
private final DAVResource resource;
}
//~--- fields ---------------------------------------------------------------
private final Provider<HttpServletRequest> requestProvider;
/** Field description */
private final ScmConfiguration configuration;
/** Field description */
private final RepositoryProvider repositoryProvider;
/** Field description */
private final TemplateEngineFactory templateEngineFactory;
}

View File

@@ -92,7 +92,7 @@ public abstract class ZippedRepositoryTestBase extends AbstractTestBase
*
* @param date
*/
protected void checkDate(Long date)
protected void checkDate(long date)
{
assertNotNull(date);
assertTrue("Date should not be older than current date",

View File

@@ -42,6 +42,7 @@ import java.io.OutputStream;
import java.net.HttpURLConnection;
import java.net.URL;
import java.util.Arrays;
import java.util.Map;
import java.util.Set;
/**
@@ -169,10 +170,8 @@ public class DefaultAdvancedHttpClient extends AdvancedHttpClient {
private void applyHeaders(BaseHttpRequest<?> request, HttpURLConnection connection) {
Multimap<String, String> headers = request.getHeaders();
for (String key : headers.keySet()) {
for (String value : headers.get(key)) {
connection.addRequestProperty(key, value);
}
for (Map.Entry<String, String> entry : headers.entries()) {
connection.addRequestProperty(entry.getKey(), entry.getValue());
}
}

View File

@@ -65,10 +65,10 @@ class InvalidFormatExceptionMapperTest {
}
static class ObjectWithEnum {
public Enum e;
public TestEnum e;
}
enum Enum {
enum TestEnum {
ONE, TWO
}
}

View File

@@ -162,7 +162,7 @@ public class DefaultRepositoryManagerPerfTest {
private long calculateAverage(List<Long> times) {
Long sum = 0L;
if(!times.isEmpty()) {
for (Long time : times) {
for (long time : times) {
sum += time;
}
return Math.round(sum.doubleValue() / times.size());