Skip to content

Commit

Permalink
DSND-2703: Fix sonar issues (#115)
Browse files Browse the repository at this point in the history
* DSND-2703: Upgrade to Java 21

* DSND-2703: Get docker working with cucumber tests

* The cucumber tests would not run due to testcontainers not
  finding a "valid docker environment". The reason for this was
  because data-sync-api-sdk-java had the transitive dependency
  docker-java-api which is out of date (presumeably out of date
  with the current spring or testcontainers version. Therefore,
  adding the docker-java-api dependency to the pom with the most
  recent version got the cucumber tests running, albeit not all
  pass.
* In addition to this, I've added docker-java-api as an exclusion
  to data-sync-api-sdk-java to be even more explicit - this might
  get removed going forward as it may be deemed unneccessary.

* DSND-2703: WIP get cucumber 503s working

* TestRestTemplate was timing out due to waiting for Mongo to respond.
  This was caused by an out of date transitive dep.

* DSND-2703: Remove unused dep

* DSND-2703: Remove unused property from pom

* DSND-2703: Clean up pom

* DSND-2703: Fix failing GET list cucumber tests

* DSND-2703: Add autocloseable to steps

* DSND-2703: Optimise imports

* DSND-2703: Address majority of sonar lint complaints

* DSND-2703: Refactor switch statement

* DSND-2703: WIP Trying to fix checkstyle plugin issue

* DSND-2703: Remove checkstyle plugiin

* DSND-2703: Remove unused exception class

* DSND-2703: Remove unused get/set
  • Loading branch information
sthompsonCH authored Jul 8, 2024
1 parent 91e9971 commit 0605cb8
Show file tree
Hide file tree
Showing 45 changed files with 427 additions and 478 deletions.
43 changes: 0 additions & 43 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -176,22 +176,6 @@
</dependencies>

<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-checkstyle-plugin</artifactId>
<version>2.17</version>
<dependencies>
<dependency>
<groupId>com.puppycrawl.tools</groupId>
<artifactId>checkstyle</artifactId>
<version>7.5.1</version>
</dependency>
</dependencies>
</plugin>
</plugins>
</pluginManagement>
<plugins>
<plugin>
<groupId>org.springframework.boot</groupId>
Expand Down Expand Up @@ -227,33 +211,6 @@
</environmentVariables>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-checkstyle-plugin</artifactId>
<version>2.17</version>
<executions>
<execution>
<phase>process-sources</phase>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
<configuration>
<violationSeverity>warning</violationSeverity>
<logViolationsToConsole>true</logViolationsToConsole>
<failsOnError>true</failsOnError>
<failOnViolation>true</failOnViolation>
<configLocation>companieshouse_checks.xml</configLocation>
</configuration>
<dependencies>
<dependency>
<groupId>uk.gov.companieshouse</groupId>
<artifactId>java-checkstyle-config</artifactId>
<version>1.1</version>
</dependency>
</dependencies>
</plugin>
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import io.cucumber.junit.CucumberOptions;
import io.cucumber.spring.CucumberContextConfiguration;
import org.junit.runner.RunWith;

import uk.gov.companieshouse.pscdataapi.config.AbstractIntegrationTest;

@RunWith(Cucumber.class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.ActiveProfiles;
import uk.gov.companieshouse.api.InternalApiClient;
import uk.gov.companieshouse.api.api.CompanyMetricsApiService;
import uk.gov.companieshouse.pscdataapi.api.ChsKafkaApiService;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import java.util.HashMap;
import java.util.Map;
import org.springframework.http.ResponseEntity;

/**
* Context to store the state.
Expand All @@ -11,18 +10,8 @@ public enum CucumberContext {

CONTEXT;

private static final String RESPONSE = "RESPONSE";

private final ThreadLocal<Map<String, Object>> testContexts = ThreadLocal.withInitial(HashMap::new);

public ResponseEntity<?> getResponse() {
return get(RESPONSE);
}

public ResponseEntity<?> setResponse(ResponseEntity<?> response) {
return set(RESPONSE, response);
}

public <T> T get(String name) {
return (T) testContexts.get()
.get(name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1101,7 +1101,7 @@ public void aGetRequestHasBeenSentForAndForLegalPersonBeneficialOwner(String com
}

@And("a PSC exists for {string} for List summary")
public void aPSCExistsForForListSummary(String companyNumber) throws JsonProcessingException {
public void aPSCExistsForForListSummary(String companyNumber) {

PscData pscData = new PscData();
PscDocument document1 = new PscDocument();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
package uk.gov.companieshouse.pscdataapi.util;

import org.springframework.util.FileCopyUtils;

import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import org.springframework.util.FileCopyUtils;

public class FileReaderUtil {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

import java.time.OffsetDateTime;
import java.util.HashMap;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Service;
import uk.gov.companieshouse.api.InternalApiClient;
Expand All @@ -12,33 +10,37 @@
import uk.gov.companieshouse.api.error.ApiErrorResponseException;
import uk.gov.companieshouse.api.handler.chskafka.request.PrivateChangedResourcePost;
import uk.gov.companieshouse.api.model.ApiResponse;
import uk.gov.companieshouse.api.psc.Statement;
import uk.gov.companieshouse.logging.Logger;
import uk.gov.companieshouse.pscdataapi.exceptions.ServiceUnavailableException;
import uk.gov.companieshouse.pscdataapi.models.PscData;
import uk.gov.companieshouse.pscdataapi.util.PscTransformationHelper;

@Service
public class ChsKafkaApiService {
@Autowired
InternalApiClient internalApiClient;

private static final String PSC_URI = "/company/%s/persons-with-significant-control/"
+ "%s/%s";
private static final String CHANGED_EVENT_TYPE = "changed";
private static final String DELETE_EVENT_TYPE = "deleted";

private final InternalApiClient internalApiClient;
private final Logger logger;

@Value("${chs.api.kafka.url}")
private String chsKafkaApiUrl;
@Value("${chs.api.kafka.resource-changed.uri}")
private String resourceChangedUri;
private static final String PSC_URI = "/company/%s/persons-with-significant-control/"
+ "%s/%s";
private static final String CHANGED_EVENT_TYPE = "changed";

private static final String DELETE_EVENT_TYPE = "deleted";
@Autowired
private Logger logger;
public ChsKafkaApiService(InternalApiClient internalApiClient, Logger logger) {
this.internalApiClient = internalApiClient;
this.logger = logger;
}

/**
* Creates a ChangedResource object to send a request to the chs kafka api.
*
* @param contextId chs kafka id
* @param companyNumber company number of psc
* @param contextId chs kafka id
* @param companyNumber company number of psc
* @param notificationId mongo id
* @return passes request to api response handling
*/
Expand All @@ -57,8 +59,8 @@ public ApiResponse<Void> invokeChsKafkaApi(String contextId, String companyNumbe
/**
* Creates a ChangedResource object to send a delete request to the chs kafka api.
*
* @param contextId chs kafka id
* @param companyNumber company number of psc
* @param contextId chs kafka id
* @param companyNumber company number of psc
* @param notificationId mongo id
* @return passes request to api response handling
*/
Expand All @@ -70,9 +72,9 @@ public ApiResponse<Void> invokeChsKafkaApiWithDeleteEvent(String contextId,
internalApiClient.setBasePath(chsKafkaApiUrl);
PrivateChangedResourcePost changedResourcePost =
internalApiClient.privateChangedResourceHandler()
.postChangedResource(resourceChangedUri,
mapChangedResource(contextId, companyNumber,
notificationId, kind, true, pscData));
.postChangedResource(resourceChangedUri,
mapChangedResource(contextId, companyNumber,
notificationId, kind, true, pscData));
return handleApiCall(changedResourcePost);
}

Expand All @@ -97,7 +99,7 @@ private ChangedResource mapChangedResource(String contextId, String companyNumbe
}

private static String mapKind(String kind) {
HashMap<String,String> kindMap = new HashMap<>();
HashMap<String, String> kindMap = new HashMap<>();
kindMap.put("individual-person-with-significant-control", "individual");
kindMap.put("legal-person-person-with-significant-control", "legal-person");
kindMap.put("corporate-entity-person-with-significant-control", "corporate-entity");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,14 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationFeature;
import com.fasterxml.jackson.databind.module.SimpleModule;

import java.time.LocalDate;
import java.time.OffsetDateTime;
import java.util.List;
import java.util.function.Supplier;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.data.mongodb.core.convert.MongoCustomConversions;
import uk.gov.companieshouse.api.InternalApiClient;
import uk.gov.companieshouse.api.api.CompanyMetricsApiService;
import uk.gov.companieshouse.api.converter.EnumWriteConverter;
import uk.gov.companieshouse.api.psc.SensitiveData;
import uk.gov.companieshouse.pscdataapi.converter.CompanyPscReadConverter;
import uk.gov.companieshouse.pscdataapi.converter.CompanyPscSensitiveReadConverter;
import uk.gov.companieshouse.pscdataapi.converter.CompanyPscSensitiveWriteConverter;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package uk.gov.companieshouse.pscdataapi.config;

import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Lazy;
import org.springframework.data.mongodb.core.convert.DefaultMongoTypeMapper;
Expand All @@ -10,9 +9,11 @@
@Configuration
public class MongoDbConfig implements InitializingBean {

@Autowired
@Lazy
private MappingMongoConverter mappingMongoConverter;
private final MappingMongoConverter mappingMongoConverter;

public MongoDbConfig(@Lazy MappingMongoConverter mappingMongoConverter) {
this.mappingMongoConverter = mappingMongoConverter;
}

// Remove _class field from data
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import com.mongodb.MongoClientSettings;
import com.mongodb.client.MongoClient;
import com.mongodb.client.MongoClients;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
Expand All @@ -24,8 +23,11 @@ public class MongoPscConfig extends AbstractMongoClientConfiguration {
@Value("${spring.data.mongodb.uri}")
private String databaseUri;

@Autowired
MongoCustomConversions mongoCustomConversions;
private final MongoCustomConversions mongoCustomConversions;

public MongoPscConfig(MongoCustomConversions mongoCustomConversions) {
this.mongoCustomConversions = mongoCustomConversions;
}

@Bean
MongoTransactionManager transactionManager(MongoDatabaseFactory dbFactory) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import java.util.Arrays;
import java.util.List;

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.web.servlet.config.annotation.InterceptorRegistry;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package uk.gov.companieshouse.pscdataapi.controller;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.dao.DataAccessException;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
Expand Down Expand Up @@ -37,10 +36,14 @@
produces = "application/json")
public class CompanyPscController {

@Autowired
CompanyPscService pscService;

private static final Logger LOGGER = LoggerFactory.getLogger("psc-data-api");
private static final String GETTING_PSC_DATA_WITH_COMPANY_NUMBER = "Getting PSC data with company number %s";

private final CompanyPscService pscService;

public CompanyPscController(CompanyPscService pscService) {
this.pscService = pscService;
}

/**
* PUT endpoint for PSC record
Expand Down Expand Up @@ -120,7 +123,7 @@ public ResponseEntity<Individual> getIndividualPscData(
DataMapHolder.get()
.companyNumber(companyNumber)
.itemId(notificationId);
LOGGER.info(String.format("Getting PSC data with company number %s", companyNumber),
LOGGER.info(String.format(GETTING_PSC_DATA_WITH_COMPANY_NUMBER, companyNumber),
DataMapHolder.getLogMap());
try {
Individual individual = pscService
Expand Down Expand Up @@ -150,7 +153,7 @@ public ResponseEntity<IndividualBeneficialOwner> getIndividualBeneficialOwnerPsc
DataMapHolder.get()
.companyNumber(companyNumber)
.itemId(notificationId);
LOGGER.info(String.format("Getting PSC data with company number %s", companyNumber),
LOGGER.info(String.format(GETTING_PSC_DATA_WITH_COMPANY_NUMBER, companyNumber),
DataMapHolder.getLogMap());
try {
IndividualBeneficialOwner individualBeneficialOwner =
Expand Down Expand Up @@ -179,7 +182,7 @@ public ResponseEntity<CorporateEntity> getCorporateEntityPscData(
DataMapHolder.get()
.companyNumber(companyNumber)
.itemId(notificationId);
LOGGER.info(String.format("Getting PSC data with company number %s", companyNumber),
LOGGER.info(String.format(GETTING_PSC_DATA_WITH_COMPANY_NUMBER, companyNumber),
DataMapHolder.getLogMap());
try {
CorporateEntity corporateEntity =
Expand Down Expand Up @@ -207,7 +210,7 @@ public ResponseEntity<CorporateEntityBeneficialOwner> getCorporateEntityBenefici
DataMapHolder.get()
.companyNumber(companyNumber)
.itemId(notificationId);
LOGGER.info(String.format("Getting PSC data with company number %s", companyNumber),
LOGGER.info(String.format(GETTING_PSC_DATA_WITH_COMPANY_NUMBER, companyNumber),
DataMapHolder.getLogMap());
try {
CorporateEntityBeneficialOwner corporateEntityBeneficialOwner =
Expand Down Expand Up @@ -235,7 +238,7 @@ public ResponseEntity<LegalPerson> getLegalPersonPscData(
DataMapHolder.get()
.companyNumber(companyNumber)
.itemId(notificationId);
LOGGER.info(String.format("Getting PSC data with company number %s", companyNumber),
LOGGER.info(String.format(GETTING_PSC_DATA_WITH_COMPANY_NUMBER, companyNumber),
DataMapHolder.getLogMap());
try {
LegalPerson legalPerson =
Expand Down Expand Up @@ -263,7 +266,7 @@ public ResponseEntity<LegalPersonBeneficialOwner> getLegalPersonBeneficialOwnerP
DataMapHolder.get()
.companyNumber(companyNumber)
.itemId(notificationId);
LOGGER.info(String.format("Getting PSC data with company number %s", companyNumber),
LOGGER.info(String.format(GETTING_PSC_DATA_WITH_COMPANY_NUMBER, companyNumber),
DataMapHolder.getLogMap());
try {
LegalPersonBeneficialOwner legalPersonBeneficialOwner =
Expand Down Expand Up @@ -291,7 +294,7 @@ public ResponseEntity<SuperSecure> getSuperSecurePscData(
DataMapHolder.get()
.companyNumber(companyNumber)
.itemId(notificationId);
LOGGER.info(String.format("Getting PSC data with company number %s", companyNumber),
LOGGER.info(String.format(GETTING_PSC_DATA_WITH_COMPANY_NUMBER, companyNumber),
DataMapHolder.getLogMap());
try {
SuperSecure superSecure =
Expand Down Expand Up @@ -319,7 +322,7 @@ public ResponseEntity<SuperSecureBeneficialOwner> getSuperSecureBeneficialOwnerP
DataMapHolder.get()
.companyNumber(companyNumber)
.itemId(notificationId);
LOGGER.info(String.format("Getting PSC data with company number %s", companyNumber),
LOGGER.info(String.format(GETTING_PSC_DATA_WITH_COMPANY_NUMBER, companyNumber),
DataMapHolder.getLogMap());
try {
SuperSecureBeneficialOwner superSecureBeneficialOwner =
Expand Down Expand Up @@ -348,7 +351,7 @@ public ResponseEntity<PscList> searchPscListSummary(
@RequestParam(value = "start_index",
required = false, defaultValue = "0") final Integer startIndex,
@RequestParam(
value = "register_view", required = false) boolean registerView) {
value = "register_view", required = false) Boolean registerView) {
DataMapHolder.get()
.companyNumber(companyNumber);
itemsPerPage = Math.min(itemsPerPage, 100);
Expand Down
Loading

0 comments on commit 0605cb8

Please sign in to comment.