Skip to content

Commit 5149824

Browse files
ErmiasGErmiasG
authored
[HWORKS-991] add spotbugs (#1486)
1 parent 71d0882 commit 5149824

File tree

78 files changed

+400
-692
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

78 files changed

+400
-692
lines changed

‎.github/workflows/hopsworks-tests.yml

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
name: pre-commit tests
2+
3+
on:
4+
pull_request_review:
5+
types: [submitted]
6+
7+
jobs:
8+
hopsworks-unit-tests:
9+
name: Hopsworks Unit Tests
10+
runs-on: ubuntu-22.04
11+
12+
steps:
13+
- name: Checkout
14+
uses: actions/checkout@v3
15+
16+
- name: Set up JDK 8
17+
uses: actions/setup-java@v3
18+
with:
19+
java-version: '8'
20+
distribution: 'adopt'
21+
cache: 'maven'
22+
23+
- name: Run unit tests
24+
run: mvn --batch-mode test
25+
26+
hopsworks-vulnerability-checker:
27+
name: Hopsworks Vulnerability Checker
28+
runs-on: ubuntu-22.04
29+
30+
steps:
31+
- name: Checkout
32+
uses: actions/checkout@v3
33+
34+
- name: Set up JDK 8
35+
uses: actions/setup-java@v3
36+
with:
37+
java-version: '8'
38+
distribution: 'adopt'
39+
cache: 'maven'
40+
41+
- name: Run vulnerability checker
42+
run: mvn clean install -Powasp-dependency-check,spot-bugs -DskipTests

‎alerting/pom.xml

+4
Original file line numberDiff line numberDiff line change
@@ -66,5 +66,9 @@
6666
<groupId>junit</groupId>
6767
<artifactId>junit</artifactId>
6868
</dependency>
69+
<dependency>
70+
<groupId>com.github.spotbugs</groupId>
71+
<artifactId>spotbugs-annotations</artifactId>
72+
</dependency>
6973
</dependencies>
7074
</project>

‎alerting/src/main/java/io/hops/hopsworks/alerting/api/util/Settings.java

+2
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.logicalclocks.servicediscoverclient.resolvers.Type;
2323
import com.logicalclocks.servicediscoverclient.service.Service;
2424
import com.logicalclocks.servicediscoverclient.service.ServiceQuery;
25+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
2526

2627
import javax.ws.rs.core.UriBuilder;
2728
import java.net.InetAddress;
@@ -38,6 +39,7 @@
3839
import java.util.logging.Logger;
3940
import java.util.stream.Collectors;
4041

42+
@SuppressFBWarnings(justification="Used in a singleton", value="AT_OPERATION_SEQUENCE_ON_CONCURRENT_ABSTRACTION")
4143
public class Settings {
4244
private final static Logger LOGGER = Logger.getLogger(Settings.class.getName());
4345
public static final String MANAGEMENT_API_HEALTH = "/-/healthy";

‎alerting/src/main/java/io/hops/hopsworks/alerting/config/AlertManagerConfigController.java

+2
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
2323
import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator;
2424
import com.google.common.base.Strings;
25+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
2526
import io.hops.hopsworks.alerting.api.AlertManagerClient;
2627
import io.hops.hopsworks.alerting.config.dto.AlertManagerConfig;
2728
import io.hops.hopsworks.alerting.exceptions.AlertManagerConfigFileNotFoundException;
@@ -33,6 +34,7 @@
3334
import java.io.File;
3435
import java.io.IOException;
3536

37+
@SuppressFBWarnings(justification="Used only as default", value="DMI_HARDCODED_ABSOLUTE_FILENAME")
3638
public class AlertManagerConfigController {
3739
private static final String CONFIG_FILE_PATH = "/srv/hops/alertmanager/alertmanager/alertmanager.yml";
3840

‎alerting/src/main/java/io/hops/hopsworks/alerting/config/dto/Responder.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,13 @@ public class Responder {
2828
public Responder() {
2929
}
3030

31-
public Responder(String id, String name, String username) {
31+
private Responder(String id, String name, String username) {
3232
this.id = id;
3333
this.name = name;
3434
this.username = username;
35+
}
36+
37+
public static Responder getInstance(String id, String name, String username) {
3538
String errorMsg = "Exactly one of id, name or username fields should be defined.";
3639
if (!Strings.isNullOrEmpty(id) && (!Strings.isNullOrEmpty(name) || !Strings.isNullOrEmpty(username))) {
3740
throw new IllegalStateException(errorMsg);
@@ -42,6 +45,7 @@ public Responder(String id, String name, String username) {
4245
if (!Strings.isNullOrEmpty(username) && (!Strings.isNullOrEmpty(name) || !Strings.isNullOrEmpty(id))) {
4346
throw new IllegalStateException(errorMsg);
4447
}
48+
return new Responder(id, name, username);
4549
}
4650

4751
public String getId() {

‎hopsworks-alert/src/main/java/io/hops/hopsworks/alert/AMClient.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ void tryBuildClient() {
128128
}
129129

130130
void createRetryTimer() {
131-
long duration = Constants.RETRY_SECONDS * 1000;
131+
long duration = Constants.RETRY_SECONDS * 1000L;
132132
if (count > Constants.NUM_RETRIES) {
133133
duration *= Constants.NUM_RETRIES;
134134
} else {

‎hopsworks-api/src/main/java/io/hops/hopsworks/api/admin/dto/MaterializerStateResponse.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public void setMaterialKeyLocks(Map<String, Boolean> materialKeyLocks) {
9898
this.materialKeyLocks = materialKeyLocks;
9999
}
100100

101-
public static class CryptoMaterial {
101+
public static class CryptoMaterial implements Serializable {
102102
private String user;
103103
private String path;
104104
private Integer references;

‎hopsworks-api/src/main/java/io/hops/hopsworks/api/dataset/tags/InodeTagUri.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
import javax.ws.rs.core.UriInfo;
2525

2626
public class InodeTagUri implements TagUri {
27-
private static UriInfo uriInfo;
27+
private final UriInfo uriInfo;
2828

2929
public InodeTagUri(UriInfo uriInfo) {
3030
this.uriInfo = uriInfo;

‎hopsworks-api/src/main/java/io/hops/hopsworks/api/filter/ApiKeyFilter.java

+2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package io.hops.hopsworks.api.filter;
1818

19+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
1920
import io.hops.hopsworks.api.auth.Configuration;
2021
import io.hops.hopsworks.api.auth.UserStatusValidator;
2122
import io.hops.hopsworks.api.auth.UserUtilities;
@@ -41,6 +42,7 @@
4142
@Provider
4243
@ApiKeyRequired
4344
@Priority(Priorities.AUTHENTICATION - 1)
45+
@SuppressFBWarnings(justification = "Should be renamed", value = "NM_SAME_SIMPLE_NAME_AS_SUPERCLASS")
4446
public class ApiKeyFilter extends io.hops.hopsworks.api.auth.key.ApiKeyFilter {
4547
@EJB
4648
private UserStatusValidator userStatusValidator;

‎hopsworks-api/src/main/java/io/hops/hopsworks/api/jwt/JWTHelper.java

+1-5
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@
5454
import javax.ws.rs.core.SecurityContext;
5555
import java.io.IOException;
5656
import java.security.NoSuchAlgorithmException;
57-
import java.util.ArrayList;
5857
import java.util.Arrays;
5958
import java.util.Collections;
6059
import java.util.Date;
@@ -78,10 +77,7 @@
7877
public class JWTHelper {
7978
private static final Logger LOGGER = Logger.getLogger(JWTHelper.class.getName());
8079

81-
public static final List<String> SERVICE_RENEW_JWT_AUDIENCE = new ArrayList<>(1);
82-
static {
83-
SERVICE_RENEW_JWT_AUDIENCE.add(Audience.SERVICES);
84-
}
80+
public static final List<String> SERVICE_RENEW_JWT_AUDIENCE = Collections.singletonList(Audience.SERVICES);
8581

8682
@EJB
8783
private JWTController jwtController;

‎hopsworks-api/src/main/java/io/hops/hopsworks/api/jwt/JWTRequestDTO.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import javax.xml.bind.annotation.XmlAccessorType;
2323
import javax.xml.bind.annotation.XmlRootElement;
2424
import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter;
25+
import java.util.Arrays;
2526
import java.util.Date;
2627

2728
@XmlRootElement
@@ -125,9 +126,9 @@ public void setExpLeeway(int expLeeway) {
125126

126127
@Override
127128
public String toString() {
128-
return "JWTRequestDTO{" + "subject=" + subject + ", audiences=" + audiences + ", keyName=" + keyName
129-
+ ", expiresAt=" + expiresAt + ", notBefore=" + nbf + ", renewable=" + renewable + ", expLeeway="
130-
+ expLeeway + '}';
129+
return "JWTRequestDTO{" + "subject=" + subject + ", audiences=" + Arrays.toString(audiences) + ", keyName=" +
130+
keyName + ", expiresAt=" + expiresAt + ", notBefore=" + nbf + ", renewable=" + renewable + ", expLeeway="
131+
+ expLeeway + '}';
131132
}
132133

133134
}

‎hopsworks-api/src/main/java/io/hops/hopsworks/api/project/util/PathValidator.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ private void buildFullPath(Project project, String path, DsPath dsPath) throws D
123123
while (path.startsWith("/")) {
124124
path = path.substring(1);
125125
}
126-
String[] pathComponents = path.split(File.separator);
126+
String[] pathComponents = path.split("/");
127127

128128
String dsName = pathComponents[0];
129129
boolean shared = false;

‎hopsworks-api/src/main/java/io/hops/hopsworks/api/proxy/ProxyServlet.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ protected void consumeQuietly(HttpEntity entity) {
437437
* I use an HttpClient HeaderGroup class instead of Set<String> because this
438438
* approach does case insensitive lookup faster.
439439
*/
440-
protected static HeaderGroup hopByHopHeaders;
440+
protected static final HeaderGroup hopByHopHeaders;
441441

442442
static {
443443
hopByHopHeaders = new HeaderGroup();

‎hopsworks-api/src/main/java/io/hops/hopsworks/api/python/library/LibraryDTO.java

+8-7
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package io.hops.hopsworks.api.python.library;
1717

18+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
1819
import io.hops.hopsworks.api.python.command.CommandDTO;
1920
import io.hops.hopsworks.common.api.RestDTO;
2021
import io.hops.hopsworks.common.python.library.PackageSource;
@@ -103,6 +104,8 @@ public void setPackageSource(PackageSource packageSource) {
103104
}
104105

105106
@Override
107+
@SuppressFBWarnings(justification = "Can be compared to PythonDep",
108+
value = "EQ_CHECK_FOR_OPERAND_NOT_COMPATIBLE_WITH_THIS")
106109
public boolean equals(Object o) {
107110
if (o instanceof LibraryDTO) {
108111
LibraryDTO pd = (LibraryDTO) o;
@@ -116,13 +119,11 @@ public boolean equals(Object o) {
116119
}
117120
if (o instanceof PythonDep) {
118121
PythonDep pd = (PythonDep) o;
119-
if (pd.getRepoUrl().compareToIgnoreCase(this.channel) == 0
120-
&& pd.getInstallType().name().equalsIgnoreCase(this.getPackageSource().name())
121-
&& pd.getDependency().compareToIgnoreCase(this.library) == 0
122-
&& pd.getVersion().compareToIgnoreCase(this.version) == 0
123-
&& Boolean.toString(pd.isPreinstalled()).compareToIgnoreCase(this.preinstalled) == 0) {
124-
return true;
125-
}
122+
return pd.getRepoUrl().compareToIgnoreCase(this.channel) == 0
123+
&& pd.getInstallType().name().equalsIgnoreCase(this.getPackageSource().name())
124+
&& pd.getDependency().compareToIgnoreCase(this.library) == 0
125+
&& pd.getVersion().compareToIgnoreCase(this.version) == 0
126+
&& Boolean.toString(pd.isPreinstalled()).compareToIgnoreCase(this.preinstalled) == 0;
126127
}
127128
return false;
128129
}

‎hopsworks-api/src/main/java/io/hops/hopsworks/api/user/apiKey/ApiKeyResource.java

+4
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package io.hops.hopsworks.api.user.apiKey;
1717

18+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
1819
import io.hops.hopsworks.api.auth.key.ApiKeyUtilities;
1920
import io.hops.hopsworks.api.filter.AllowedProjectRoles;
2021
import io.hops.hopsworks.api.filter.Audience;
@@ -230,6 +231,9 @@ public Response checkSession(@Context SecurityContext sc,
230231
// For a strange reason the Set of user supplied ApiScope(s) is marshalled
231232
// to String even though it's a Set of ApiScope. We need to explicitly convert
232233
// them to ApiScope
234+
@SuppressFBWarnings(justification = "For a strange reason the Set of user supplied ApiScope(s) is marshalled to " +
235+
"String even though it's a Set of ApiScope. We need to explicitly convert them to ApiScope",
236+
value = "BC_IMPOSSIBLE_CAST")
233237
private Set<ApiScope> validateScopes(Users user, Set<ApiScope> scopes) throws ApiKeyException {
234238
Set<ApiScope> validScopes = getScopesForUser(user);
235239
Set<ApiScope> validatedScopes = new HashSet<>(scopes.size());

‎hopsworks-api/src/main/java/io/hops/hopsworks/api/util/UploadService.java

+2-7
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,11 @@
3838
*/
3939
package io.hops.hopsworks.api.util;
4040

41+
import io.hops.hopsworks.api.auth.key.ApiKeyRequired;
4142
import io.hops.hopsworks.api.filter.AllowedProjectRoles;
4243
import io.hops.hopsworks.api.filter.Audience;
43-
import io.hops.hopsworks.api.auth.key.ApiKeyRequired;
4444
import io.hops.hopsworks.api.jwt.JWTHelper;
4545
import io.hops.hopsworks.common.dao.project.ProjectFacade;
46-
import io.hops.hopsworks.common.dataset.DatasetController;
4746
import io.hops.hopsworks.common.dataset.util.DatasetHelper;
4847
import io.hops.hopsworks.common.dataset.util.DatasetPath;
4948
import io.hops.hopsworks.common.hdfs.HdfsUsersController;
@@ -75,7 +74,6 @@
7574
import javax.ws.rs.core.MediaType;
7675
import javax.ws.rs.core.Response;
7776
import javax.ws.rs.core.SecurityContext;
78-
import java.io.IOException;
7977
import java.io.InputStream;
8078
import java.util.logging.Level;
8179
import java.util.logging.Logger;
@@ -86,8 +84,6 @@ public class UploadService {
8684

8785
private static final Logger LOGGER = Logger.getLogger(UploadService.class.getName());
8886

89-
@EJB
90-
private DatasetController datasetController;
9187
@EJB
9288
private JWTHelper jWTHelper;
9389
@EJB
@@ -177,8 +173,7 @@ public Response testMethod(@QueryParam("flowChunkNumber") String flowChunkNumber
177173
@QueryParam("flowRelativePath") String flowRelativePath,
178174
@QueryParam("flowTotalChunks") String flowTotalChunks,
179175
@QueryParam("flowTotalSize") String flowTotalSize,
180-
@Context HttpServletRequest request, @Context SecurityContext sc) throws IOException, DatasetException,
181-
ProjectException {
176+
@Context HttpServletRequest request, @Context SecurityContext sc) throws DatasetException, ProjectException {
182177
configureUploader(sc);
183178
RESTApiJsonResponse json = new RESTApiJsonResponse();
184179
FlowInfo flowInfo = new FlowInfo(

‎hopsworks-common/src/main/java/io/hops/hopsworks/common/agent/AgentController.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import javax.ejb.Stateless;
3333
import javax.ejb.TransactionAttribute;
3434
import javax.ejb.TransactionAttributeType;
35+
import java.io.Serializable;
3536
import java.util.ArrayList;
3637
import java.util.Comparator;
3738
import java.util.Date;
@@ -240,7 +241,7 @@ public ServiceStatus getStatus() {
240241
}
241242
}
242243

243-
private static class CommandsComparator<T> implements Comparator<T> {
244+
private static class CommandsComparator<T> implements Comparator<T>, Serializable {
244245

245246
@Override
246247
public int compare(T t, T t1) {

‎hopsworks-common/src/main/java/io/hops/hopsworks/common/alert/AlertController.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -330,10 +330,10 @@ public PostableAlert getPostableFeatureMonitorAlert(Project project, FeatureStor
330330
.withFeatureMonitorConfig(fmConfigName, fmResultId)
331331
.withSummary(summary)
332332
.withDescription(description);
333-
if (resourceName.equals(ResourceRequest.Name.FEATUREGROUPS)) {
333+
if (resourceName.equals(ResourceRequest.Name.FEATUREGROUPS) && featureStoreAlert instanceof FeatureGroupAlert) {
334334
builder.withFeatureGroupName(((FeatureGroupAlert) featureStoreAlert).getFeatureGroup().getName())
335335
.withFeatureGroupVersion(((FeatureGroupAlert) featureStoreAlert).getFeatureGroup().getVersion());
336-
} else if (resourceName.equals(ResourceRequest.Name.FEATUREVIEW)) {
336+
} else if (resourceName.equals(ResourceRequest.Name.FEATUREVIEW) && featureStoreAlert instanceof FeatureViewAlert) {
337337
builder.withFeatureViewVersion(((FeatureViewAlert) featureStoreAlert).getFeatureView().getVersion())
338338
.withFeatureViewName(((FeatureViewAlert) featureStoreAlert).getFeatureView().getName());
339339
}

‎hopsworks-common/src/main/java/io/hops/hopsworks/common/arrowflight/ArrowFlightController.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ private FlightClient initFlightClient(Project project, Users user)
124124
// register client certificates
125125
ArrowFlightCredentialDTO arrowFlightCredentials = new ArrowFlightCredentialDTO(accessCredentialsDTO);
126126
flightClient.doAction(new Action("register-client-certificates",
127-
objectMapper.writeValueAsString(arrowFlightCredentials).getBytes()))
127+
objectMapper.writeValueAsString(arrowFlightCredentials).getBytes(StandardCharsets.UTF_8)))
128128
.hasNext();
129129

130130
return flightClient;

‎hopsworks-common/src/main/java/io/hops/hopsworks/common/dao/AbstractFacade.java

+3
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,13 @@
3939

4040
package io.hops.hopsworks.common.dao;
4141

42+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
43+
4244
/*
4345
* Only for compatibility purposes
4446
*/
4547
@Deprecated
48+
@SuppressFBWarnings(justification = "Should be removed", value = "NM_SAME_SIMPLE_NAME_AS_SUPERCLASS")
4649
public abstract class AbstractFacade<T> extends io.hops.hopsworks.persistence.entity.util.AbstractFacade<T> {
4750

4851
public AbstractFacade(Class<T> entityClass) {

‎hopsworks-common/src/main/java/io/hops/hopsworks/common/dao/hdfs/HdfsLeDescriptorsFacade.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,15 @@
4444
import javax.persistence.EntityManager;
4545
import javax.persistence.NoResultException;
4646
import javax.persistence.PersistenceContext;
47+
4748
import io.hops.hopsworks.common.dao.AbstractFacade;
4849
import io.hops.hopsworks.persistence.entity.hdfs.HdfsLeDescriptors;
4950

5051
import java.util.Random;
5152

5253
@Stateless
5354
public class HdfsLeDescriptorsFacade extends AbstractFacade<HdfsLeDescriptors> {
54-
55+
private static final Random RANDOM = new Random();
5556
@PersistenceContext(unitName = "kthfsPU")
5657
private EntityManager em;
5758

@@ -78,7 +79,7 @@ public String getRPCEndpoint() {
7879
return null;
7980
} else {
8081
// Get a random NN Address
81-
int randomNNIndex = new Random().nextInt(hdfsLeDescriptorsList.size());
82+
int randomNNIndex = RANDOM.nextInt(hdfsLeDescriptorsList.size());
8283
HdfsLeDescriptors randomNN = hdfsLeDescriptorsList.get(randomNNIndex);
8384
String rpcAddresses = randomNN.getRpcAddresses();
8485
rpcAddresses = rpcAddresses.trim();

‎hopsworks-common/src/main/java/io/hops/hopsworks/common/dao/tensorflow/config/TensorBoardProcessMgr.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ public TensorBoardDTO startTensorBoard(Project project, Users user, HdfsUsers hd
133133

134134
String anacondaEnvironmentPath = settings.getAnacondaProjectDir();
135135
int retries = 3;
136-
while(retries > 0) {
136+
while(retries >= 0) {
137137
try {
138138
if(retries == 0) {
139139
throw new IOException("Failed to start TensorBoard for project=" + project.getName() + ", user="

0 commit comments

Comments
 (0)