Skip to content

Commit

Permalink
Merge pull request #156 from companieshouse/IDVA6-1953-logging-overhaul
Browse files Browse the repository at this point in the history
IDVA6-1953: Added new logging and updated existing logging
  • Loading branch information
krishna-patel-ch authored Dec 6, 2024
2 parents ebb516f + 7677c8d commit e1e88cc
Show file tree
Hide file tree
Showing 10 changed files with 203 additions and 135 deletions.
12 changes: 6 additions & 6 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
<maven-compiler-plugin.version>3.13.0</maven-compiler-plugin.version>
<maven-surefire-plugin.version>3.5.1</maven-surefire-plugin.version>
<structured-logging.version>3.0.20</structured-logging.version>
<log4j-version>2.24.2</log4j-version>
<rest-service-common-library-version>2.0.2</rest-service-common-library-version>
<private-api-sdk-java.version>4.0.224</private-api-sdk-java.version>
<api-sdk-manager-java-library.version>3.0.6</api-sdk-manager-java-library.version>
Expand Down Expand Up @@ -76,6 +77,11 @@
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-slf4j-impl</artifactId>
<version>${log4j-version}</version>
</dependency>
<dependency>
<groupId>uk.gov.companieshouse</groupId>
<artifactId>rest-service-common-library</artifactId>
Expand Down Expand Up @@ -109,12 +115,6 @@
<artifactId>commons-compress</artifactId>
<version>${commons-compress.version}</version>
</dependency>
<dependency>
<groupId>org.testcontainers</groupId>
<artifactId>junit-jupiter</artifactId>
<version>${junit-jupiter.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>uk.gov.companieshouse</groupId>
<artifactId>private-api-sdk-java</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,25 @@ public AssociationsListForCompanyController(AssociationsService associationsServ

@Override
public ResponseEntity<AssociationsList> getAssociationsForCompany( final String companyNumber, final String xRequestId, final Boolean includeRemoved, final Integer pageIndex, final Integer itemsPerPage) {

LOG.debugContext(xRequestId, String.format( "Attempting to fetch users that are associated with company %s. includeRemoved=%b, itemsPerPage=%d, and pageIndex=%d.", companyNumber, includeRemoved, itemsPerPage, pageIndex ),null );
LOG.infoContext( xRequestId, String.format( "Received request with company_number=%s, includeRemoved=%b, itemsPerPage=%d, pageIndex=%d.", companyNumber, includeRemoved, itemsPerPage, pageIndex ),null );

if ( pageIndex < 0 ){
LOG.error("pageIndex was less then 0" );
LOG.errorContext( xRequestId, new Exception( "pageIndex was less than 0" ), null );
throw new BadRequestRuntimeException( "Please check the request and try again" );
}

if ( itemsPerPage <= 0 ){
LOG.error( "itemsPerPage was less then 0" );
LOG.errorContext( xRequestId, new Exception( "itemsPerPage was less than or equal to 0" ), null);
throw new BadRequestRuntimeException( "Please check the request and try again" );
}

final var companyProfile = companyService.fetchCompanyProfile( companyNumber );

LOG.debugContext( xRequestId, "Attempting to fetch associated users", null );
final var associationsList = associationsService.fetchAssociatedUsers( companyNumber, companyProfile, includeRemoved, itemsPerPage, pageIndex );
final var associationsListIsEmpty = associationsList.getItems().isEmpty();

LOG.debugContext(xRequestId, associationsListIsEmpty ? "Could not find any associations" : "Successfully fetched associations",null) ;
LOG.infoContext( xRequestId, associationsListIsEmpty ? "Could not find any associations" : String.format( "Successfully fetched %d associations", associationsList.getItems().size() ),null );

return new ResponseEntity<>( associationsList, HttpStatus.OK );
}
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ public String toMessage(){
return message;
}

public String toMessageSendingFailureLoggingMessage(){
var message = String.format( "Failed to send %s notification to %s at %s from %s, regarding company %s.", messageType, sentTo, sentTime.toString(), sentFrom, companyNumber );
message += Objects.isNull( invitationExpiryTimestamp ) ? "" : String.format( " Invitation would have expired at %s.", invitationExpiryTimestamp );
return message;
}

@Override
public String toString() {
return "EmailNotification{" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.stream.Collectors;

import static uk.gov.companieshouse.GenerateEtagUtil.generateEtag;
import static uk.gov.companieshouse.accounts.association.utils.RequestContextUtil.getXRequestId;
import static uk.gov.companieshouse.accounts.association.utils.StaticPropertyUtil.DAYS_SINCE_INVITE_TILL_EXPIRES;

@Service
Expand Down Expand Up @@ -128,10 +129,12 @@ public AssociationDao createAssociation(final String companyNumber,
final ApprovalRouteEnum approvalRoute,
final String invitedByUserId) {
if (Objects.isNull(companyNumber) || companyNumber.isEmpty()) {
LOG.errorContext( getXRequestId(), new Exception( "companyNumber is null" ), null );
throw new NullPointerException("companyNumber must not be null");
}

if (Objects.isNull(userId) && Objects.isNull(userEmail)) {
LOG.errorContext( getXRequestId(), new Exception( "userId and userEmail is null" ), null );
throw new NullPointerException("UserId or UserEmail should be provided");
}

Expand All @@ -153,6 +156,7 @@ public AssociationDao createAssociation(final String companyNumber,

private static void addInvitation(String invitedByUserId, AssociationDao association) {
if ( Objects.isNull( invitedByUserId ) ){
LOG.errorContext( getXRequestId(), new Exception( "invitedByUserId is null" ), null );
throw new NullPointerException( "invitedByUserId cannot be null." );
}

Expand All @@ -174,14 +178,14 @@ public AssociationDao sendNewInvitation(final String invitedByUserId, final Asso
@Transactional
public void updateAssociation(final String associationId, final Update update) {
if (Objects.isNull(associationId)) {
LOG.error("Attempted to update association with null association id");
LOG.errorContext( getXRequestId(), new Exception( "Attempted to update association with null association id" ), null);
throw new NullPointerException("associationId must not be null");
}
update.set("etag", generateEtag());
final var numRecordsUpdated = associationsRepository.updateAssociation(associationId, update);

if (numRecordsUpdated == 0) {
LOG.error(String.format("Failed to update association with id: %s", associationId));
LOG.errorContext( getXRequestId(), new Exception( String.format( "Failed to update association with id: %s", associationId ) ), null );
throw new InternalServerErrorRuntimeException("Failed to update association");
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package uk.gov.companieshouse.accounts.association.service;

import static uk.gov.companieshouse.accounts.association.utils.RequestContextUtil.getXRequestId;

import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Supplier;
Expand Down Expand Up @@ -32,23 +34,24 @@ public CompanyService(CompanyProfileEndpoint companyProfileEndpoint) {

public Supplier<CompanyDetails> createFetchCompanyProfileRequest( final String companyNumber ) {
final var request = companyProfileEndpoint.createFetchCompanyProfileRequest( companyNumber );
final var xRequestId = getXRequestId();
return () -> {
try {
LOG.debug( String.format( "Attempting to fetch company profile for company %s" , companyNumber ) );
LOG.debugContext( xRequestId, String.format( "Sending request to company-profile-api: GET /company/{company_number}/company-detail. Attempting to retrieve company profile for company: %s" , companyNumber ), null );
return request.execute().getData();
} catch ( ApiErrorResponseException exception ) {
if ( exception.getStatusCode() == 404 ) {
LOG.error( String.format( "Could not find company profile for company %s", companyNumber ) );
LOG.errorContext( xRequestId, new Exception( String.format( "Could not find company profile for company %s", companyNumber ) ), null );
throw new NotFoundRuntimeException( "accounts-association-api", "Failed to find company" );
} else {
LOG.error( String.format( "Failed to retrieve company profile for company %s", companyNumber ) );
LOG.errorContext( xRequestId, new Exception( String.format( "Failed to retrieve company profile for company %s" , companyNumber ) ), null );
throw new InternalServerErrorRuntimeException( "Failed to retrieve company profile" );
}
} catch ( URIValidationException exception ) {
LOG.error( String.format( "Failed to fetch company profile for company %s, because uri was incorrectly formatted", companyNumber ) );
LOG.errorContext( xRequestId, new Exception( String.format( "Failed to fetch company profile for company %s, because uri was incorrectly formatted", companyNumber ) ), null );
throw new InternalServerErrorRuntimeException( "Invalid uri for company-profile-api service" );
} catch ( Exception exception ) {
LOG.error( String.format("Failed to retrieve company profile for company %s", companyNumber) );
LOG.errorContext( xRequestId, new Exception( String.format("Failed to retrieve company profile for company %s", companyNumber) ), null );
throw new InternalServerErrorRuntimeException( "Failed to retrieve company profile" );
}
};
Expand Down
Loading

0 comments on commit e1e88cc

Please sign in to comment.