Skip to content

Commit

Permalink
[CIRC-1909] Logging - circulation domain (D - L) (#1348)
Browse files Browse the repository at this point in the history 
* CIRC-1909 improve logging

* CIRC-1909 fix code smell issue

* CIRC-1909 incorporating review comments

* CIRC-1909 fix code smells

* CIRC-1909 fix code smells

* CIRC-1909 fix typo
  • Loading branch information
@roman-barannyk
roman-barannyk authored Oct 20, 2023
1 parent f2efd88 commit 233f505
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 26 deletions.
37 changes: 31 additions & 6 deletions src/main/java/org/folio/circulation/domain/InstanceRequestItemsComparer.java
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,32 @@
package org.folio.circulation.domain;

import static org.folio.circulation.support.utils.LogUtil.mapAsString;

import java.lang.invoke.MethodHandles;
import java.time.ZonedDateTime;
import java.util.Comparator;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.UUID;
import java.util.stream.Collectors;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

public class InstanceRequestItemsComparer {

private InstanceRequestItemsComparer() {}

private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass());

public static Map<Item, Integer> sortRequestQueues(Map<Item, Integer> itemsQueueLengthUnsortedMap,
Map<Item, ZonedDateTime> itemDueDateMap,
UUID destinationServicePointId) {
Map<Item, ZonedDateTime> itemDueDateMap, UUID destinationServicePointId) {

log.debug("sortRequestQueues:: parameters itemsQueueLengthUnsortedMap: {}, " +
"itemDueDateMap: {}, destinationServicePointId: {}", () -> mapAsString(
itemsQueueLengthUnsortedMap), () -> mapAsString(itemDueDateMap),
() -> destinationServicePointId);

return itemsQueueLengthUnsortedMap
.entrySet()
.stream()
Expand All @@ -22,8 +35,11 @@ public static Map<Item, Integer> sortRequestQueues(Map<Item, Integer> itemsQueue
(oldValue, newValue) -> oldValue, (LinkedHashMap::new)));
}

private static Comparator<Map.Entry<Item, Integer>> compareQueueLengths(Map<Item, ZonedDateTime> itemDueDateMap,
UUID destinationServicePointId) {
private static Comparator<Map.Entry<Item, Integer>> compareQueueLengths(
Map<Item, ZonedDateTime> itemDueDateMap, UUID destinationServicePointId) {

log.debug("compareQueueLengths:: parameters itemDueDateMap: {}, destinationServicePointId: {}",
() -> mapAsString(itemDueDateMap), () -> destinationServicePointId);
// Sort the map
return (q1Size, q2Size) -> {
int result = compareQueueSize(q1Size.getValue(), q2Size.getValue());
Expand All @@ -37,15 +53,20 @@ private static Comparator<Map.Entry<Item, Integer>> compareQueueLengths(Map<Item
result = compareItemServicePoint(q1Item, q2Item, destinationServicePointId);
}
}
log.info("compareQueueLengths:: result: {}", result);
return result;
};
}

private static int compareQueueSize(int queueSize1, int queueSize2){
log.debug("compareQueueSize:: parameters queueSize1: {}, queueSize2: {}",
queueSize1, queueSize2);
return queueSize1 - queueSize2;
}

private static int compareDueDate(Item item1, Item item2, Map<Item, ZonedDateTime> itemDueDateMap) {
log.debug("compareDueDate:: parameters item1: {}, item2: {}, itemDueDateMap: {}",
() -> item1, () -> item2, () -> mapAsString(itemDueDateMap));
ZonedDateTime q1ItemDueDate = itemDueDateMap.get(item1);
ZonedDateTime q2ItemDueDate = itemDueDateMap.get(item2);

Expand All @@ -61,9 +82,13 @@ private static int compareDueDate(Item item1, Item item2, Map<Item, ZonedDateTim
}

private static int compareItemServicePoint(Item item1, Item item2,
UUID destinationServicePointId) {
if (destinationServicePointId == null)
UUID destinationServicePointId) {

log.debug("compareItemServicePoint:: parameters item1: {}, item2: {}, " +
"destinationServicePointId: {}", item1, item2, destinationServicePointId);
if (destinationServicePointId == null) {
return 0;
}

Location item1Location = item1.getLocation();
Location item2Location = item2.getLocation();
Expand Down
64 changes: 50 additions & 14 deletions src/main/java/org/folio/circulation/domain/Loan.java
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package org.folio.circulation.domain;

import static java.lang.Boolean.TRUE;
import static java.lang.Boolean.FALSE;
import static java.lang.Boolean.TRUE;
import static java.time.ZoneOffset.UTC;
import static java.util.Collections.emptyList;
import static java.util.Objects.nonNull;
Expand All @@ -21,6 +21,8 @@
import static org.folio.circulation.domain.representations.LoanProperties.ACTION_COMMENT;
import static org.folio.circulation.domain.representations.LoanProperties.AGED_TO_LOST_DATE;
import static org.folio.circulation.domain.representations.LoanProperties.AGED_TO_LOST_DELAYED_BILLING;
import static org.folio.circulation.domain.representations.LoanProperties.BILL_DATE;
import static org.folio.circulation.domain.representations.LoanProperties.BILL_NUMBER;
import static org.folio.circulation.domain.representations.LoanProperties.CHECKIN_SERVICE_POINT_ID;
import static org.folio.circulation.domain.representations.LoanProperties.CHECKOUT_SERVICE_POINT_ID;
import static org.folio.circulation.domain.representations.LoanProperties.CLAIMED_RETURNED_DATE;
Expand All @@ -29,20 +31,18 @@
import static org.folio.circulation.domain.representations.LoanProperties.DUE_DATE;
import static org.folio.circulation.domain.representations.LoanProperties.ITEM_LOCATION_ID_AT_CHECKOUT;
import static org.folio.circulation.domain.representations.LoanProperties.ITEM_STATUS;
import static org.folio.circulation.domain.representations.LoanProperties.LAST_FEE_BILLED;
import static org.folio.circulation.domain.representations.LoanProperties.LOAN_POLICY_ID;
import static org.folio.circulation.domain.representations.LoanProperties.LOST_ITEM_HAS_BEEN_BILLED;
import static org.folio.circulation.domain.representations.LoanProperties.LOST_ITEM_POLICY_ID;
import static org.folio.circulation.domain.representations.LoanProperties.METADATA;
import static org.folio.circulation.domain.representations.LoanProperties.OVERDUE_FINE_POLICY_ID;
import static org.folio.circulation.domain.representations.LoanProperties.REMINDERS;
import static org.folio.circulation.domain.representations.LoanProperties.RETURN_DATE;
import static org.folio.circulation.domain.representations.LoanProperties.STATUS;
import static org.folio.circulation.domain.representations.LoanProperties.SYSTEM_RETURN_DATE;
import static org.folio.circulation.domain.representations.LoanProperties.UPDATED_BY_USER_ID;
import static org.folio.circulation.domain.representations.LoanProperties.USER_ID;
import static org.folio.circulation.domain.representations.LoanProperties.REMINDERS;
import static org.folio.circulation.domain.representations.LoanProperties.LAST_FEE_BILLED;
import static org.folio.circulation.domain.representations.LoanProperties.BILL_DATE;
import static org.folio.circulation.domain.representations.LoanProperties.BILL_NUMBER;
import static org.folio.circulation.support.ValidationErrorFailure.failedValidation;
import static org.folio.circulation.support.json.JsonPropertyFetcher.getBooleanProperty;
import static org.folio.circulation.support.json.JsonPropertyFetcher.getDateTimeProperty;
Expand All @@ -62,13 +62,16 @@
import static org.folio.circulation.support.utils.DateTimeUtil.isSameMillis;
import static org.folio.circulation.support.utils.DateTimeUtil.mostRecentDate;

import java.lang.invoke.MethodHandles;
import java.time.ZonedDateTime;
import java.util.Collection;
import java.util.UUID;
import java.util.stream.Stream;

import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.folio.circulation.domain.policy.LoanPolicy;
import org.folio.circulation.domain.policy.OverdueFinePolicy;
import org.folio.circulation.domain.policy.OverdueFinePolicyRemindersPolicy;
Expand All @@ -85,6 +88,7 @@
@AllArgsConstructor(access = PRIVATE)
@ToString(onlyExplicitlyIncluded = true)
public class Loan implements ItemRelatedRecord, UserRelatedRecord {
private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass());
@ToString.Include
private final JsonObject representation;
@Getter
Expand Down Expand Up @@ -142,6 +146,7 @@ public boolean allFeesAndFinesClosed() {
}

public Loan changeDueDate(ZonedDateTime newDueDate) {
log.debug("changeDueDate:: parameters newDueDate: {}", () -> newDueDate);
write(representation, DUE_DATE, newDueDate.withZoneSameInstant(UTC));

return this;
Expand Down Expand Up @@ -170,18 +175,22 @@ public Loan setDueDateChangedByNearExpireUser() {
}

private void changeReturnDate(ZonedDateTime returnDate) {
log.debug("changeReturnDate:: parameters returnDate: {}", () -> returnDate);
write(representation, RETURN_DATE, returnDate);
}

private void changeSystemReturnDate(ZonedDateTime systemReturnDate) {
log.debug("changeSystemReturnDate:: parameters systemReturnDate: {}", () -> systemReturnDate);
write(representation, SYSTEM_RETURN_DATE, systemReturnDate);
}

public void changeAction(LoanAction action) {
log.debug("changeAction:: parameters action: {}", action);
changeAction(action.getValue());
}

public void changeAction(String action) {
log.debug("changeAction:: parameters action: {}", action);
write(representation, LoanProperties.ACTION, action);
}

Expand All @@ -190,10 +199,12 @@ public String getAction() {
}

private void changeCheckInServicePointId(UUID servicePointId) {
log.debug("changeCheckInServicePointId:: parameters servicePointId: {}", servicePointId);
write(representation, "checkinServicePointId", servicePointId);
}

public Loan changeItemStatusForItemAndLoan(ItemStatus itemStatus) {
log.debug("changeItemStatusForItemAndLoan:: parameters itemStatus: {}", itemStatus);
Item itemToChange = getItem();

executeIfNotNull(itemToChange, f -> f.changeStatus(itemStatus));
Expand All @@ -204,19 +215,23 @@ public Loan changeItemStatusForItemAndLoan(ItemStatus itemStatus) {
}

private void changeStatus(LoanStatus status) {
log.debug("changeStatus:: parameters status: {}", status);
representation.put(STATUS, new JsonObject().put("name", status.getValue()));
}

public Loan changeItemEffectiveLocationIdAtCheckOut(String locationId) {
log.debug("changeItemEffectiveLocationIdAtCheckOut:: parameters locationId: {}", locationId);
representation.put(ITEM_LOCATION_ID_AT_CHECKOUT, locationId);
return this;
}

public void changeActionComment(String comment) {
log.debug("changeActionComment:: parameters comment: {}", comment);
representation.put(ACTION_COMMENT, comment);
}

public void removeActionComment() {
log.debug("removeActionComment:: ");
representation.remove(ACTION_COMMENT);
}

Expand All @@ -226,31 +241,36 @@ public String getActionComment() {

public Result<Void> isValidStatus() {
if (!representation.containsKey(STATUS)) {
return failedDueToServerError("Loan does not have a status");
String errorMessage = "Loan does not have a status";
log.warn("isValidStatus:: {}", errorMessage);
return failedDueToServerError(errorMessage);
}

// Provided status name is not present in the enum
if (getStatus() == null) {
return failedValidation("Loan status must be \"Open\" or \"Closed\"",
STATUS, getStatusName());
String errorMessage = "Loan status must be \"Open\" or \"Closed\"";
log.warn("isValidStatus:: {}", errorMessage);
return failedValidation(errorMessage, STATUS, getStatusName());
}

return succeeded(null);
}

public Result<Void> openLoanHasUserId() {
if (isOpen() && getUserId() == null) {
return failedValidation("Open loan must have a user ID",
USER_ID, getUserId());
String errorMessage = "Open loan must have a user ID";
log.warn("openLoanHasUserId::{}", errorMessage);
return failedValidation(errorMessage, USER_ID, getUserId());
} else {
return succeeded(null);
}
}

public Result<Void> closedLoanHasCheckInServicePointId() {
if (isClosed() && getCheckInServicePointId() == null) {
return failedValidation("A Closed loan must have a Checkin Service Point",
CHECKIN_SERVICE_POINT_ID, getCheckInServicePointId());
String errorMessage = "A Closed loan must have a Checkin Service Point";
log.warn("closedLoanHasCheckInServicePointId:: {}", errorMessage);
return failedValidation(errorMessage, CHECKIN_SERVICE_POINT_ID, getCheckInServicePointId());
} else {
return succeeded(null);
}
Expand Down Expand Up @@ -454,6 +474,8 @@ public String getPatronGroupIdAtCheckout() {
}

public Loan renew(ZonedDateTime dueDate, String basedUponLoanPolicyId) {
log.debug("renew:: parameters dueDate: {}, basedUponLoanPolicyId: {}",
() -> dueDate, () -> basedUponLoanPolicyId);
changeAction(RENEWED);
removeActionComment();
setLoanPolicyId(basedUponLoanPolicyId);
Expand All @@ -463,10 +485,11 @@ public Loan renew(ZonedDateTime dueDate, String basedUponLoanPolicyId) {
return this;
}

public Loan overrideRenewal(ZonedDateTime dueDate,
String basedUponLoanPolicyId,
public Loan overrideRenewal(ZonedDateTime dueDate, String basedUponLoanPolicyId,
String actionComment) {

log.debug("overrideRenewal:: parameters dueDate: {}, basedUponLoanPolicyId: {}, " +
"actionComment: {}", () -> dueDate, () -> basedUponLoanPolicyId, () -> actionComment);
changeAction(RENEWED_THROUGH_OVERRIDE);
setLoanPolicyId(basedUponLoanPolicyId);
changeDueDate(dueDate);
Expand All @@ -479,6 +502,9 @@ public Loan overrideRenewal(ZonedDateTime dueDate,
private Loan checkIn(LoanAction action, ZonedDateTime returnDateTime,
ZonedDateTime systemReturnDateTime, UUID servicePointId) {

log.debug("checkIn:: parameters action: {}, returnDateTime: {}, " +
"systemReturnDateTime: {}, servicePointId: {}", () -> action, () -> returnDateTime,
() -> systemReturnDateTime, () -> servicePointId);
closeLoan(action);
changeReturnDate(returnDateTime);
changeSystemReturnDate(systemReturnDateTime);
Expand All @@ -500,6 +526,7 @@ Loan resolveClaimedReturned(LoanAction resolveAction,


public Loan declareItemLost(String comment, ZonedDateTime dateTime) {
log.debug("declareItemLost:: parameters comment: {}, dateTime: {}", () -> comment, () -> dateTime);
changeAction(DECLARED_LOST);
changeActionComment(comment);
changeItemStatusForItemAndLoan(ItemStatus.DECLARED_LOST);
Expand Down Expand Up @@ -583,10 +610,12 @@ public ZonedDateTime getReturnDate() {
}

public void changeItemStatus(String itemStatus) {
log.debug("changeItemStatus:: parameters itemStatus: {}", itemStatus);
representation.put(LoanProperties.ITEM_STATUS, itemStatus);
}

public void changeDeclaredLostDateTime(ZonedDateTime dateTime) {
log.debug("changeDeclaredLostDateTime:: parameters dateTime: {}", () -> dateTime);
write(representation, DECLARED_LOST_DATE, dateTime);
}

Expand Down Expand Up @@ -646,6 +675,8 @@ public OverdueFinePolicyRemindersPolicy.ReminderSequenceEntry getNextReminder()
}

public Loan claimItemReturned(String comment, ZonedDateTime claimedReturnedDate) {
log.debug("claimItemReturned:: parameters comment: {}, claimedReturnedDate: {}",
() -> comment, () -> claimedReturnedDate);
changeAction(CLAIMED_RETURNED);
if (StringUtils.isNotBlank(comment)) {
changeActionComment(comment);
Expand All @@ -662,6 +693,7 @@ private void changeClaimedReturnedDate(ZonedDateTime claimedReturnedDate) {
}

public Loan closeLoan(LoanAction action) {
log.debug("closeLoan:: parameters action: {}", action);
changeStatus(LoanStatus.CLOSED);

changeAction(action);
Expand All @@ -671,6 +703,7 @@ public Loan closeLoan(LoanAction action) {
}

public Loan closeLoan(LoanAction action, String comment) {
log.debug("closeLoan:: parameters action: {}, comment: {}", action, comment);
changeStatus(LoanStatus.CLOSED);

changeAction(action);
Expand All @@ -680,6 +713,7 @@ public Loan closeLoan(LoanAction action, String comment) {
}

public Loan markItemMissing(String comment) {
log.debug("markItemMissing:: parameters comment: {}", comment);
changeItemStatusForItemAndLoan(ItemStatus.MISSING);

return closeLoan(MISSING, comment);
Expand All @@ -698,6 +732,7 @@ public FeeAmount getRemainingFeeFineAmount() {
}

public void closeLoanAsLostAndPaid() {
log.debug("closeLoanAsLostAndPaid:: ");
closeLoan(CLOSED_LOAN);
changeItemStatusForItemAndLoan(ItemStatus.LOST_AND_PAID);
}
Expand All @@ -709,6 +744,7 @@ public Loan copy() {
}

public Loan ageOverdueItemToLost(ZonedDateTime ageToLostDate) {
log.debug("ageOverdueItemToLost:: parameters ageToLostDate: {}", () -> ageToLostDate);
changeAction(ITEM_AGED_TO_LOST);
removeActionComment();
changeItemStatusForItemAndLoan(ItemStatus.AGED_TO_LOST);
Expand Down
Loading

0 comments on commit 233f505

Please sign in to comment.