Skip to content

Commit

Permalink
Optimize transaction creation, Fix unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@azizbekxm
azizbekxm committed Dec 6, 2024
1 parent 4e6e8d6 commit 8d2ccc4
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 43 deletions.
69 changes: 42 additions & 27 deletions src/main/java/org/folio/services/financedata/FinanceDataService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.folio.okapi.common.GenericCompositeFuture;
import org.folio.rest.core.RestClient;
import org.folio.rest.core.models.RequestContext;
import org.folio.rest.core.models.RequestEntry;
Expand Down Expand Up @@ -78,56 +77,60 @@ public Future<Void> putFinanceData(FyFinanceDataCollection financeDataCollection
return succeededFuture();
}

validateFinanceDataCollection(financeDataCollection);
validateFinanceDataCollection(financeDataCollection, getFiscalYearId(financeDataCollection));

return processAllocationTransaction(financeDataCollection, requestContext)
.compose(v -> updateFinanceData(financeDataCollection, requestContext))
.onSuccess(asyncResult -> processLogs(financeDataCollection, requestContext, COMPLETED))
.onFailure(asyncResult -> processLogs(financeDataCollection, requestContext, ERROR));
}

private void validateFinanceDataCollection(FyFinanceDataCollection financeDataCollection) {
private void validateFinanceDataCollection(FyFinanceDataCollection financeDataCollection, String fiscalYearId) {
for (int i = 0; i < financeDataCollection.getFyFinanceData().size(); i++) {
var financeData = financeDataCollection.getFyFinanceData().get(i);
validateFinanceDataFields(financeData, i);
validateFinanceDataFields(financeData, i, fiscalYearId);

var allocationChange = BigDecimal.valueOf(financeData.getBudgetAllocationChange());
var initialAllocation = BigDecimal.valueOf(financeData.getBudgetInitialAllocation());

if (allocationChange.doubleValue() < 0 && allocationChange.abs().doubleValue() > initialAllocation.doubleValue()) {
var params = List.of(new Parameter().withKey(String.format("financeData[%s].budgetAllocationChange", i))
.withValue(String.valueOf(financeData.getBudgetAllocationChange())));
var error = new Error().withMessage("Allocation change cannot be greater than initial allocation").withParameters(params);
var error = createError("Allocation change cannot be greater than initial allocation",
String.format("financeData[%s].budgetAllocationChange", i), String.valueOf(financeData.getBudgetAllocationChange()));
throw new HttpException(422, new Errors().withErrors(List.of(error)));
}
}
}

private Future<Void> processAllocationTransaction(FyFinanceDataCollection fyFinanceDataCollection,
RequestContext requestContext) {
var transactionsFuture = fyFinanceDataCollection.getFyFinanceData().stream()
.map(financeData -> createAllocationTransaction(financeData, requestContext))
.toList();
return fiscalYearService.getFiscalYearById(getFiscalYearId(fyFinanceDataCollection), requestContext)
.map(fiscalYear -> createAllocationTransactions(fyFinanceDataCollection, fiscalYear.getCurrency()))
.compose(transactions -> createBatchTransaction(transactions, requestContext));
}

return GenericCompositeFuture.join(transactionsFuture)
.compose(compositeFuture -> {
List<Transaction> transactions = compositeFuture.list();
return createBatchTransaction(transactions, requestContext);
});
private String getFiscalYearId(FyFinanceDataCollection fyFinanceDataCollection) {
return fyFinanceDataCollection.getFyFinanceData().get(0).getFiscalYearId();
}

private Future<Transaction> createAllocationTransaction(FyFinanceData financeData, RequestContext requestContext) {
private List<Transaction> createAllocationTransactions(FyFinanceDataCollection financeDataCollection, String currency) {
return financeDataCollection.getFyFinanceData().stream()
.map(financeData -> createAllocationTransaction(financeData, currency))
.toList();
}

private Transaction createAllocationTransaction(FyFinanceData financeData, String currency) {
var allocation = calculateAllocation(financeData);
var transaction = new Transaction()
log.info("Creating allocation transaction for fund '{}' and budget '{}' with allocation '{}'",
financeData.getFundId(), financeData.getBudgetId(), allocation);

return new Transaction()
.withTransactionType(Transaction.TransactionType.ALLOCATION)
.withId(UUID.randomUUID().toString())
.withAmount(allocation)
.withFiscalYearId(financeData.getFiscalYearId())
.withToFundId(financeData.getFundId())
.withSource(Transaction.Source.USER);
log.info("Creating allocation transaction for fund '{}' and budget '{}' with allocation '{}'",
financeData.getFundId(), financeData.getBudgetId(), allocation);

return fiscalYearService.getFiscalYearById(financeData.getFiscalYearId(), requestContext)
.map(fiscalYear -> transaction.withCurrency(fiscalYear.getCurrency()));
.withSource(Transaction.Source.USER)
.withCurrency(currency);
}

private Double calculateAllocation(FyFinanceData financeData) {
Expand All @@ -151,17 +154,24 @@ private void processLogs(FyFinanceDataCollection financeDataCollection,
RequestContext requestContext, FundUpdateLog.Status status) {
var jobDetails = new JobDetails().withAdditionalProperty("fyFinanceData", financeDataCollection.getFyFinanceData());
var fundUpdateLog = new FundUpdateLog().withId(UUID.randomUUID().toString())
.withJobName("Update finance data")
.withJobName("Update finance data") // TODO: Update job name generation
.withStatus(status)
.withRecordsCount(financeDataCollection.getTotalRecords())
.withJobDetails(jobDetails)
.withJobNumber(1);
fundUpdateLogService.createFundUpdateLog(fundUpdateLog, requestContext);
}

private void validateFinanceDataFields(FyFinanceData financeData, int i) {
private void validateFinanceDataFields(FyFinanceData financeData, int i, String fiscalYearId) {
var errors = new Errors().withErrors(new ArrayList<>());

if (!financeData.getFiscalYearId().equals(fiscalYearId)) {
errors.getErrors().add(createError(
String.format("Fiscal year ID must be the same as other fiscal year ID(s) '[%s]' in the request", fiscalYearId),
String.format("financeData[%s].fiscalYearId", i), financeData.getFiscalYearId())
);
}

validateField(errors, String.format("financeData[%s].fundCode", i), financeData.getFundCode(), "Fund code is required");
validateField(errors, String.format("financeData[%s].fundName", i), financeData.getFundName(), "Fund name is required");
validateField(errors, String.format("financeData[%s].fundDescription", i), financeData.getFundDescription(), "Fund description is required");
Expand All @@ -183,9 +193,14 @@ private void validateFinanceDataFields(FyFinanceData financeData, int i) {

private void validateField(Errors errors, String fieldName, Object fieldValue, String errorMessage) {
if (fieldValue == null) {
var params = List.of(new Parameter().withKey(fieldName).withValue("null"));
errors.getErrors().add(new Error().withMessage(errorMessage).withParameters(params));
errors.getErrors().add(createError(errorMessage, fieldName, "null"));
}
}

private Error createError(String message, String key, String value) {
log.warn("Validation error: {}", message);
var param = new Parameter().withKey(key).withValue(value);
return new Error().withMessage(message).withParameters(List.of(param));
}
}

3 changes: 2 additions & 1 deletion src/test/java/org/folio/rest/impl/FinanceDataApiTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.folio.rest.jaxrs.model.FyFinanceDataCollection;
import org.folio.services.financedata.FinanceDataService;
import org.folio.services.protection.AcqUnitsService;
import org.folio.util.CopilotGenerated;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
Expand All @@ -56,7 +57,7 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;


@CopilotGenerated(partiallyGenerated = true)
public class FinanceDataApiTest {

@Autowired
Expand Down
24 changes: 9 additions & 15 deletions src/test/java/org/folio/services/financedata/FinanceDataServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.util.UUID;

import io.vertx.core.Context;
import io.vertx.core.Future;
import io.vertx.core.Vertx;
import io.vertx.junit5.VertxExtension;
import io.vertx.junit5.VertxTestContext;
Expand Down Expand Up @@ -186,24 +185,19 @@ void negative_testPutFinanceData_FailureInProcessAllocationTransaction(VertxTest
}

@Test
void testCreateAllocationTransactionUsingReflection(VertxTestContext vertxTestContext) throws Exception {
FyFinanceData data = createValidFyFinanceData();
FiscalYear fiscalYear = new FiscalYear().withCurrency("USD");

when(fiscalYearService.getFiscalYearById(any(), any())).thenReturn(succeededFuture(fiscalYear));
void testCreateAllocationTransactionUsingReflection() throws Exception {
var data = createValidFyFinanceData();
var fiscalYear = new FiscalYear().withCurrency("USD");

// Use reflection to access the private method
var method = FinanceDataService.class.getDeclaredMethod("createAllocationTransaction", FyFinanceData.class, RequestContext.class);
var method = FinanceDataService.class.getDeclaredMethod("createAllocationTransaction", FyFinanceData.class, String.class);
method.setAccessible(true);

Future<Transaction> future = (Future<Transaction>) method.invoke(financeDataService, data, requestContextMock);
future.onComplete(vertxTestContext.succeeding(transaction -> {
assertEquals(Transaction.TransactionType.ALLOCATION, transaction.getTransactionType());
assertEquals(data.getFundId(), transaction.getToFundId());
assertEquals(fiscalYear.getCurrency(), transaction.getCurrency());
assertEquals(150.0, transaction.getAmount()); // Assuming initial allocation is 100 and change is 50
vertxTestContext.completeNow();
}));
Transaction transaction = (Transaction) method.invoke(financeDataService, data, fiscalYear.getCurrency());
assertEquals(Transaction.TransactionType.ALLOCATION, transaction.getTransactionType());
assertEquals(data.getFundId(), transaction.getToFundId());
assertEquals(fiscalYear.getCurrency(), transaction.getCurrency());
assertEquals(150.0, transaction.getAmount()); // Assuming initial allocation is 100 and change is 50
}

@Test
Expand Down
25 changes: 25 additions & 0 deletions src/test/java/org/folio/util/CopilotGenerated.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package org.folio.util;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

import org.springframework.core.annotation.AliasFor;

/**
* Annotation to indicate that test(s) in the class are generated by GitHub Copilot.
* <br><br>
* Set <code>value</code> or <code>partiallyGenerated</code> attribute to <code>true</code>
* if the generated test(s) were significantly modified/altered by the developer.
*/
@Target(ElementType.TYPE)
@Retention(RetentionPolicy.SOURCE)
public @interface CopilotGenerated {

@AliasFor("partiallyGenerated")
boolean value() default false;

@AliasFor("value")
boolean partiallyGenerated() default false;
}

0 comments on commit 8d2ccc4

Please sign in to comment.