Skip to content

Commit

Permalink
TMI2-512-code-clean-up (#82)
Browse files Browse the repository at this point in the history
* Code quality improvements
  • Loading branch information
IlyasBaqqari-CabinetOffice authored Jan 5, 2024
1 parent 2c0f1bf commit 6342411
Show file tree
Hide file tree
Showing 59 changed files with 204 additions and 237 deletions.
4 changes: 2 additions & 2 deletions localdev/postgres/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ services:
image: postgres
restart: always
ports:
- 5432:5432
- "5432:5432"
volumes:
- postgres:/var/lib/postgresql/data
environment:
Expand All @@ -17,7 +17,7 @@ services:
image: adminer
restart: always
ports:
- 8082:8080
- "8082:8080"

volumes:
postgres:
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
@ComponentScan(basePackages = "gov.cabinetoffice.gap")
public class ApplyBackendApplication {

public static void main(String[] args) {
SpringApplication.run(ApplyBackendApplication.class, args);
}

public static void main(String[] args) {
SpringApplication.run(ApplyBackendApplication.class, args);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public SpringDocConfigProperties springDocConfigProperties() {
}

@Bean
ObjectMapperProvider objectMapperProvider(SpringDocConfigProperties springDocConfigProperties){
ObjectMapperProvider objectMapperProvider(SpringDocConfigProperties springDocConfigProperties) {
return new ObjectMapperProvider(springDocConfigProperties);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public class MandatoryQuestionConstants {
public static final String APPLICANT_FUNDING_LOCATION_TITLE = "Where will this funding be spent?";
public static final String APPLICANT_FUNDING_LOCATION_HINT_TEXT = "Select the location where the grant funding will be spent. You can choose more than one, if it is being spent in more than one location.\n\nSelect all that apply:";
public static final String APPLICANT_FUNDING_LOCATION_ADMIN_SUMMARY = "where the funding will be spent";
public static final String[] APPLICANT_FUNDING_LOCATION_OPTIONS = new String[] {
public static final String[] APPLICANT_FUNDING_LOCATION_OPTIONS = new String[]{
"North East England",
"North West England",
"South East England",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public enum GrantApplicantOrganisationType {

private String name;

private GrantApplicantOrganisationType(String name) {
GrantApplicantOrganisationType(String name) {
this.name = name;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public enum GrantMandatoryQuestionOrgType {

private String name;

private GrantMandatoryQuestionOrgType(String name) {
GrantMandatoryQuestionOrgType(String name) {
this.name = name;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ public enum SubmissionQuestionResponseType {
MultipleSelection,
Date,
SingleFileUpload,
Numeric;
Numeric
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package gov.cabinetoffice.gap.applybackend.exception;

public class GrantApplicationNotPublishedException extends RuntimeException{
public class GrantApplicationNotPublishedException extends RuntimeException {

public GrantApplicationNotPublishedException(String message) {
super(message);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package gov.cabinetoffice.gap.applybackend.exception;

public class JwtExpiredTokenException extends RuntimeException{
public class JwtExpiredTokenException extends RuntimeException {

public JwtExpiredTokenException(String message) {
super(message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
public interface GrantMandatoryQuestionMapper {


@Mapping(source= "grantScheme.id", target = "schemeId")
@Mapping(source = "grantScheme.id", target = "schemeId")
@Mapping(source = "orgType", target = "orgType", qualifiedByName = "mapEntityOrgTypeToDtoOrgType")
@Mapping(source = "fundingAmount", target = "fundingAmount", qualifiedByName = "mapEntityFundingAmountToDtoFundingAmount")
@Mapping(source = "fundingLocation", target = "fundingLocation", qualifiedByName = "mapEntityFundingLocationToDtoFundingLocation")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class GrantScheme {
@Builder.Default
@Column(name = "version", nullable = false)
private Integer version = 1;

@Builder.Default
@Column(name = "created_date", nullable = false)
private Instant createdDate = Instant.now();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,24 @@
})
public class RegisterApplicant {
@NotBlank(message = "Enter a first name")
@ContainsOnlyAlphaChars(message="First name must contain only letters")
@ContainsOnlyAlphaChars(message = "First name must contain only letters")
private String firstName;

@NotBlank(message = "Enter a last name")
@ContainsOnlyAlphaChars(message="Last name must contain only letters")
@ContainsOnlyAlphaChars(message = "Last name must contain only letters")
private String lastName;

@NotBlank(message = "Enter an email address")
@Email(message = "Enter an email address in the correct format, like [email protected]")
@Size(max=254, message = "Email address must be 254 characters or less")
@Size(max = 254, message = "Email address must be 254 characters or less")
private String email;

@NotBlank(message = "Enter an email address")
@Email(message = "Enter an email address in the correct format, like [email protected]")
@Size(max=254, message = "Email address must be 254 characters or less")
@Size(max = 254, message = "Email address must be 254 characters or less")
private String emailConfirmed;

@PhoneNumberIsValid(message = "Enter a UK telephone number, like 07123456789", field="telephone")
@PhoneNumberIsValid(message = "Enter a UK telephone number, like 07123456789", field = "telephone")
private String telephone;

@NotBlank(message = "You must confirm that you have read and agreed to the privacy policy")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
@AllArgsConstructor
@Builder
@Entity
@Table(name= "spotlight_batch")
@Table(name = "spotlight_batch")
public class SpotlightBatch {

@Id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
@AllArgsConstructor
@Builder
@Entity
@Table(name= "spotlight_submission")
@Table(name = "spotlight_submission")
public class SpotlightSubmission {

@Id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
@AllArgsConstructor
@Builder
@Entity
@Table(name= "grant_submission")
@Table(name = "grant_submission")
public class Submission extends BaseEntity {
@Id
@GeneratedValue
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,5 @@
@Repository
public interface GrantApplicantRepository extends JpaRepository<GrantApplicant, Long> {
Optional<GrantApplicant> findByUserId(String userid);

boolean existsByUserId(String userId);



}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package gov.cabinetoffice.gap.applybackend.repository;

import gov.cabinetoffice.gap.applybackend.dto.api.GetGrantSchemeDto;
import gov.cabinetoffice.gap.applybackend.model.GrantScheme;
import org.springframework.data.jpa.repository.EntityGraph;
import org.springframework.data.jpa.repository.JpaRepository;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,5 @@
public interface SubmissionRepository extends JpaRepository<Submission, UUID> {
List<Submission> findByApplicantId(long applicantId);
Optional<Submission> findByApplicantIdAndApplicationId(long applicantId, Integer applicationId);

Optional<Submission> findByIdAndApplicantUserId(UUID id, String userId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import gov.cabinetoffice.gap.applybackend.dto.api.JwtPayload;
import gov.cabinetoffice.gap.applybackend.service.JwtService;
import lombok.RequiredArgsConstructor;
import org.jetbrains.annotations.NotNull;
import org.springframework.http.HttpHeaders;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
Expand All @@ -29,9 +30,9 @@ public class JwtTokenFilter extends OncePerRequestFilter {
private final JwtService jwtService;

@Override
protected void doFilterInternal(HttpServletRequest request,
HttpServletResponse response,
FilterChain chain)
protected void doFilterInternal(@NotNull HttpServletRequest request,
@NotNull HttpServletResponse response,
@NotNull FilterChain chain)
throws ServletException, IOException {

// Check if auth header exists. If not, return without setting authentication in the security context
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
import gov.cabinetoffice.gap.applybackend.model.GrantApplicantOrganisationProfile;
import gov.cabinetoffice.gap.applybackend.repository.GrantApplicantOrganisationProfileRepository;
import gov.cabinetoffice.gap.applybackend.repository.GrantApplicantRepository;
import gov.cabinetoffice.gap.applybackend.service.GrantApplicantOrganisationProfileService;
import gov.cabinetoffice.gap.applybackend.service.GrantApplicantService;
import gov.cabinetoffice.gap.applybackend.service.JwtService;
import lombok.RequiredArgsConstructor;
import org.jetbrains.annotations.NotNull;
import org.springframework.http.HttpHeaders;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
Expand All @@ -23,7 +23,6 @@
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.util.Collections;
import java.util.Optional;

import static org.springframework.util.ObjectUtils.isEmpty;

Expand All @@ -40,9 +39,9 @@ public class JwtTokenFilterV2 extends OncePerRequestFilter {
private final GrantApplicantOrganisationProfileRepository grantApplicantOrganisationProfileRepository;

@Override
protected void doFilterInternal(HttpServletRequest request,
HttpServletResponse response,
FilterChain chain)
protected void doFilterInternal(@NotNull HttpServletRequest request,
@NotNull HttpServletResponse response,
@NotNull FilterChain chain)
throws ServletException, IOException {

// Check if auth header exists. If not, return without setting authentication in the security context
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,21 @@ public SecurityFilterChain filterChainPublic(HttpSecurity http) throws Exception
.anyRequest()
.authenticated();

if(oneLoginEnabled) {
http
.addFilterBefore(new JwtTokenFilterV2(jwtService, grantApplicantRepository, grantApplicantService, grantApplicantOrganisationProfileRepository), UsernamePasswordAuthenticationFilter.class);
if (oneLoginEnabled) {
http.addFilterBefore(
new JwtTokenFilterV2(
jwtService,
grantApplicantRepository,
grantApplicantService,
grantApplicantOrganisationProfileRepository
),
UsernamePasswordAuthenticationFilter.class
);
} else {
http
.addFilterBefore(new JwtTokenFilter(jwtService), UsernamePasswordAuthenticationFilter.class);
http.addFilterBefore(
new JwtTokenFilter(jwtService),
UsernamePasswordAuthenticationFilter.class
);
}

// disable a bunch of Spring Security default stuff we don't need
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import gov.cabinetoffice.gap.applybackend.repository.GrantAdvertRepository;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Service;

import java.net.URL;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public GrantApplicant getApplicantFromPrincipal() {
return this.getApplicantById(jwtPayload.getSub());
}

public GrantApplicant saveApplicant(GrantApplicant applicant){
public GrantApplicant saveApplicant(GrantApplicant applicant) {
return grantApplicantRepository.save(applicant);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,18 @@ public GrantAttachment createAttachment(GrantAttachment attachment) {
public GrantAttachment getAttachment(UUID attachmentId) {
return grantAttachmentRepository
.findById(attachmentId)
.orElseThrow(() -> new NotFoundException(String.format("No Grant Attachment with ID %s was found", attachmentId.toString())));
.orElseThrow(() -> new NotFoundException(String.format("No Grant Attachment with ID %s was found", attachmentId)));
}

public GrantAttachment getAttachmentBySubmissionAndQuestion(Submission submission, String questionId) {
return grantAttachmentRepository
.findBySubmissionAndQuestionId(submission, questionId)
.orElseThrow(() -> new NotFoundException(String.format("No Grant Attachment for Submission %s and Question %s wass found", submission.getId().toString(), questionId)));
.orElseThrow(() -> new NotFoundException(String.format("No Grant Attachment for Submission %s and Question %s was found", submission.getId().toString(), questionId)));

}

public GrantAttachment save(GrantAttachment attachment) {
return grantAttachmentRepository.save(attachment);

}

public void delete(GrantAttachment attachment) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@

import gov.cabinetoffice.gap.applybackend.config.properties.EnvironmentProperties;
import gov.cabinetoffice.gap.applybackend.constants.MandatoryQuestionConstants;
import gov.cabinetoffice.gap.applybackend.enums.GrantMandatoryQuestionOrgType;
import gov.cabinetoffice.gap.applybackend.enums.GrantMandatoryQuestionStatus;
import gov.cabinetoffice.gap.applybackend.enums.SubmissionQuestionResponseType;
import gov.cabinetoffice.gap.applybackend.enums.SubmissionSectionStatus;
import gov.cabinetoffice.gap.applybackend.enums.*;
import gov.cabinetoffice.gap.applybackend.exception.ForbiddenException;
import gov.cabinetoffice.gap.applybackend.exception.NotFoundException;
import gov.cabinetoffice.gap.applybackend.mapper.GrantApplicantOrganisationProfileMapper;
Expand Down Expand Up @@ -48,7 +45,7 @@ public GrantMandatoryQuestions getGrantMandatoryQuestionBySubmissionIdAndApplica
.orElseThrow(() -> new NotFoundException(String.format("No Mandatory Question with submission id %s was found", submissionId))));

if (!grantMandatoryQuestion.get().getCreatedBy().getUserId().equals(applicantSub)) {
throw new ForbiddenException(String.format("Mandatory Question with id % and submission ID %s was not created by %s", grantMandatoryQuestion.get().getId(), submissionId, applicantSub));
throw new ForbiddenException(String.format("Mandatory Question with id %s and submission ID %s was not created by %s", grantMandatoryQuestion.get().getId(), submissionId, applicantSub));
}

return grantMandatoryQuestion.get();
Expand All @@ -60,7 +57,7 @@ public GrantMandatoryQuestions getMandatoryQuestionBySchemeId(Integer schemeId,
.orElseThrow(() -> new NotFoundException(String.format("No Mandatory Question with scheme id %s was found", schemeId))));

if (!grantMandatoryQuestion.get().getCreatedBy().getUserId().equals(applicantSub)) {
throw new ForbiddenException(String.format("Mandatory Question with id % and scheme ID %s was not created by %s",
throw new ForbiddenException(String.format("Mandatory Question with id %s and scheme ID %s was not created by %s",
grantMandatoryQuestion.get().getId(), schemeId, applicantSub));
}

Expand All @@ -77,7 +74,7 @@ public GrantMandatoryQuestions createMandatoryQuestion(GrantScheme scheme, Grant

final GrantMandatoryQuestions grantMandatoryQuestions = organisationProfileMapper.mapOrgProfileToGrantMandatoryQuestion(organisationProfile);

//Fix to exclude any existing Charity Comission Number or Companies House Number which have invalid lengths,
// Fix to exclude any existing Charity Commission Number or Companies House Number which have invalid lengths,
//This will force the applicant to go through the MQ journey and update their details with a valid length number
if (grantMandatoryQuestions.getCharityCommissionNumber() != null && grantMandatoryQuestions.getCharityCommissionNumber().length() > MandatoryQuestionConstants.CHARITY_COMMISSION_NUMBER_MAX_LENGTH) {
grantMandatoryQuestions.setCharityCommissionNumber(null);
Expand Down Expand Up @@ -218,24 +215,16 @@ public SubmissionSection buildFundingDetailsSubmissionSection(final GrantMandato
}

public SubmissionQuestion mandatoryQuestionToSubmissionQuestion(final String questionId, final GrantMandatoryQuestions mandatoryQuestions) {
switch (questionId) {
case "APPLICANT_ORG_NAME":
return buildOrganisationNameQuestion(mandatoryQuestions);
case "APPLICANT_TYPE":
return buildApplicationTypeQuestion(mandatoryQuestions);
case "APPLICANT_ORG_CHARITY_NUMBER":
return buildCharityCommissionNumberQuestion(mandatoryQuestions);
case "APPLICANT_ORG_COMPANIES_HOUSE":
return buildCompaniesHouseNumberQuestion(mandatoryQuestions);
case "APPLICANT_AMOUNT":
return buildFundingAmountQuestion(mandatoryQuestions);
case "BENEFITIARY_LOCATION":
return buildFundingLocationQuestion(mandatoryQuestions);
case "APPLICANT_ORG_ADDRESS":
return buildOrganisationAddressQuestion(mandatoryQuestions);
default:
throw new IllegalArgumentException("There is no method to process this question");
}
return switch (questionId) {
case "APPLICANT_ORG_NAME" -> buildOrganisationNameQuestion(mandatoryQuestions);
case "APPLICANT_TYPE" -> buildApplicationTypeQuestion(mandatoryQuestions);
case "APPLICANT_ORG_CHARITY_NUMBER" -> buildCharityCommissionNumberQuestion(mandatoryQuestions);
case "APPLICANT_ORG_COMPANIES_HOUSE" -> buildCompaniesHouseNumberQuestion(mandatoryQuestions);
case "APPLICANT_AMOUNT" -> buildFundingAmountQuestion(mandatoryQuestions);
case "BENEFITIARY_LOCATION" -> buildFundingLocationQuestion(mandatoryQuestions);
case "APPLICANT_ORG_ADDRESS" -> buildOrganisationAddressQuestion(mandatoryQuestions);
default -> throw new IllegalArgumentException("There is no method to process this question");
};
}

public boolean existsBySchemeIdAndApplicantId(Integer schemeId, Long applicantId) {
Expand Down Expand Up @@ -343,7 +332,7 @@ private SubmissionQuestion buildFundingAmountQuestion(final GrantMandatoryQuesti

private SubmissionQuestion buildFundingLocationQuestion(final GrantMandatoryQuestions mandatoryQuestions) {
final String[] locations = Arrays.stream(mandatoryQuestions.getFundingLocation())
.map(location -> location.getName())
.map(GrantMandatoryQuestionFundingLocation::getName)
.toArray(String[]::new);

final SubmissionQuestionValidation validation = SubmissionQuestionValidation.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ public class SecretAuthService {
* Intended to authenticate requests coming from lambdas, which shouldn't pass through
* the JWT auth process. TODO I think this could be converted into an annotation for
* lambda controller methods
*
* @param authHeader value taken from Authorization header
*/
public void authenticateSecret(String authHeader) {
Expand Down
Loading

0 comments on commit 6342411

Please sign in to comment.