Skip to content

Commit

Permalink
Refactor submission rejection process
Browse files Browse the repository at this point in the history
  • Loading branch information
flawmop committed Oct 25, 2024
1 parent 6d4b9b1 commit ae55e1f
Show file tree
Hide file tree
Showing 10 changed files with 57 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class FileProcessingException extends Exception {
/**
* Initialising constructor.
*
* @param message
* @param message Exception message.
*/
public FileProcessingException(final String message) {
super(message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,34 +46,36 @@ public class Simulation {
@GeneratedValue(strategy = IDENTITY)
private Long entityId;

@Column(nullable = false)
@Column(nullable = false, updatable = false)
private long submissionId;

// Optional. How the client/user identifies the simulation
@Column(name = "clientId")
private String clientId;

@Column(nullable = false)
@Column(nullable = false, updatable = false)
private int modelId;

@Column(nullable = false)
@Column(nullable = false, updatable = false)
private BigDecimal pacingFrequency;

@Column(nullable = false)
@Column(nullable = false, updatable = false)
private BigDecimal pacingMaxTime;

@ElementCollection(fetch = FetchType.EAGER)
@CollectionTable(name = "simulation_plasmapoints",
foreignKey = @ForeignKey(name = "simulation_fk"),
joinColumns = @JoinColumn(name = "entityId"))
@Column(nullable = false)
// TODO : Find a way to prevent updating this collection
private List<BigDecimal> plasmaPoints = new ArrayList<>();

@ElementCollection(fetch = FetchType.EAGER)
@CollectionTable(name = "simulation_message",
foreignKey = @ForeignKey(name = "simulation_fk"),
joinColumns = @JoinColumn(name = "entityId"))
@Column(nullable = false)
// TODO : Find a way to prevent updating this collection
private Set<Message> messages = new HashSet<>();

// See JPA auditing
Expand All @@ -84,8 +86,9 @@ public class Simulation {
@LastModifiedBy
String lastModifiedBy;

// Optimistic locking concurrency control
@Version
int version;
private Long lockVersion;

// Default constructor.
Simulation() {}
Expand Down Expand Up @@ -194,7 +197,7 @@ public String toString() {
return "Simulation [entityId=" + entityId + ", submissionId=" + submissionId + ", modelId=" + modelId
+ ", pacingFrequency=" + pacingFrequency + ", pacingMaxTime=" + pacingMaxTime + ", plasmaPoints=" + plasmaPoints
+ ", messages=" + messages + ", lastModifiedDate=" + lastModifiedDate + ", lastModifiedBy=" + lastModifiedBy
+ ", version=" + version + "]";
+ ", lockVersion=" + lockVersion + "]";
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import jakarta.persistence.Id;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.Table;
import jakarta.persistence.Version;

/**
* User submission entity.
Expand Down Expand Up @@ -59,6 +60,10 @@ public class Submission {
@CreatedBy
String createdBy;

// Optimistic locking concurrency control
@Version
private Long lockVersion;

// Default constructor.

/**
Expand All @@ -72,27 +77,14 @@ public Submission() {

//

/**
* Reject the submission and populate with the problem as justification.
* <p>
* Assigns the state to {@link State.REJECTED}
*
* @param problem Problem encountered.
*/
public void rejectWithProblem(final Message problem) {
this.state = State.REJECTED;
this.messages.clear();
this.messages.add(problem);
}

/**
* Reject the submission and populate with the problems as justification.
* <p>
* Assigns the state to {@link State.REJECTED}
*
* @param problems Problems encountered.
*/
public void rejectWithProblems(Map<String, Set<Message>> problems) {
public void rejectWithProblems(final Map<String, Set<Message>> problems) {
this.state = State.REJECTED;
this.messages.clear();
for (Map.Entry<String, Set<Message>> mapEntry : problems.entrySet()) {
Expand Down Expand Up @@ -127,7 +119,7 @@ public State getState() {
@Override
public String toString() {
return "Submission [entityId=" + entityId + ", state=" + state + ", messages=" + messages + ", createdDate="
+ createdDate + ", createdBy=" + createdBy + "]";
+ createdDate + ", createdBy=" + createdBy + ", lockVersion=" + lockVersion + "]";
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public InputProcessorServiceImpl(final SimulationRepository simulationRepository
this.submissionService = submissionService;
}

// Advice applied to this method to capture FileProcessingException
@Override
public void process(final long submissionId, final byte[] file) throws FileProcessingException,
InputVerificationException {
Expand Down Expand Up @@ -116,20 +117,21 @@ public void process(final long submissionId, final byte[] file) throws FileProce
if (simulations.isEmpty())
throw new FileProcessingException("Could not generate any simulations");

Map<String, Set<Message>> problems = new HashMap<>();
int problemCnt = 1;
final Map<String, Set<Message>> problems = new HashMap<>();
int simulationCnt = 1;
for (Simulation simulation: simulations) {
Set<Message> messages = simulation.getMessages(MessageLevel.WARN);
final Set<Message> messages = simulation.getMessages(MessageLevel.WARN);
if (!messages.isEmpty()) {
// Simulations not yet persisted, so no entity id to use.
problems.put(simulation.getClientId()
.orElse(String.valueOf(submissionId).concat(".").concat(String.valueOf(problemCnt))),
.orElse(String.valueOf(submissionId).concat(".").concat(String.valueOf(simulationCnt))),
messages);
problemCnt++;
simulationCnt++;
}
}

if (!problems.isEmpty()) {
submissionService.rejectOnInvalidInput(submissionId, problems);
submissionService.reject(submissionId, problems);
throw new InputVerificationException("Problems encountered translating the simulation input");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,20 @@ public interface SubmissionService {
void delete(long submissionId) throws EntityNotAccessibleException;

/**
* Reject the {@link Submission} due to file processing problems.
*
* @param submissionId Affected submission identifier.
* @param problem Problem encountered.
*/
void rejectOnFileProcessing(long submissionId, Message problem);

/**
* Reject the {@link Submission} due to input validation problems.
*
* Reject the {@link Submission} due to problems.
* <p>
* The structure of {@code problems} is keyed on simulation identifier (preference for
* using optional client-defined identifier) , e.g. :
* <pre>
* {
* &lt;client simulation id | submission id(.&lt;sim 1&gt;)&gt; : [ &lt;Message&gt;, &ltMessage2&gt; ],
* &lt;client simulation id | submission id(.&lt;sim 2&gt;)&gt; : [ &lt;Message&gt; ]
* }
* </pre>
* @param submissionId Affected submission identifier.
* @param problems Collection of problems encountered.
*/
void rejectOnInvalidInput(long submissionId, Map<String, Set<Message>> problems);
void reject(long submissionId, Map<String, Set<Message>> problems);

/**
* Retrieve the {@link Submission} identified by the {@literal submissionId}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,8 @@ public void delete(final long submissionId) throws EntityNotAccessibleException
}

@Override
public void rejectOnFileProcessing(final long submissionId, final Message problem) {
log.debug("~rejectOnFileProcessing() : Invoked for id {}", submissionId);

Submission submission = submissionRepository.getReferenceById(submissionId);
submission.rejectWithProblem(problem);
submissionRepository.save(submission);
}

@Override
public void rejectOnInvalidInput(final long submissionId, final Map<String, Set<Message>> problems) {
log.debug("~rejectOnInvalidInput() : Invoked for id {}", submissionId);
public void reject(final long submissionId, final Map<String, Set<Message>> problems) {
log.debug("~reject() : Invoked for id {}", submissionId);

Submission submission = submissionRepository.getReferenceById(submissionId);
submission.rejectWithProblems(problems);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
package com.insilicosoft.portal.svc.submission.service.advice;

import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

import org.aspectj.lang.JoinPoint;
import org.aspectj.lang.annotation.AfterThrowing;
import org.aspectj.lang.annotation.Aspect;
Expand All @@ -17,7 +22,7 @@
@Component
public class InputProcessorAdvice {

private SubmissionService submissionService;
private final SubmissionService submissionService;

/**
* Initialising constructor.
Expand All @@ -41,8 +46,13 @@ public class InputProcessorAdvice {
public void handleFileProcessingException(JoinPoint joinPoint, FileProcessingException fpe)
throws FileProcessingException {
final long submissionId = Long.valueOf(joinPoint.getArgs()[0].toString());
submissionService.rejectOnFileProcessing(submissionId, new Message(MessageLevel.WARN,
fpe.getMessage()));

final Map<String, Set<Message>> problems = new HashMap<>();
final Set<Message> messages = new HashSet<>();
messages.add(new Message(MessageLevel.WARN, fpe.getMessage()));
problems.put(String.valueOf(submissionId), messages);

submissionService.reject(submissionId, problems);
throw new FileProcessingException(fpe.getMessage());
}

Expand Down
2 changes: 2 additions & 0 deletions src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ com:
thread-name-prefix: ~simsvc-dflt-

logging:
#file:
# name: logs/submission-svc.log
level:
root: DEBUG

Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,23 @@
package com.insilicosoft.portal.svc.submission.actuator;

import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT;


import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.boot.test.web.client.TestRestTemplate;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.security.oauth2.jwt.JwtDecoder;

@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT)
@SpringBootTest(webEnvironment = RANDOM_PORT)
public class ActuatorE2E {

private String actuatorUsername;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ void successOnSubmissionLifecycle() {
var multipartBodyBuilder = new MultipartBodyBuilder();
multipartBodyBuilder.part(SubmissionIdentifiers.PARAM_NAME_SIMULATION_FILE,
new FileSystemResource(goodPath));

webTestClient.post()
.uri(postUrl)
.headers(headers -> {
Expand Down

0 comments on commit ae55e1f

Please sign in to comment.