Skip to content

Commit

Permalink
Issue 128: Hrid is needed, possibly adding it to the LoanItem Domain …
Browse files Browse the repository at this point in the history 
…Module.

For the sake of time, this takes the simplest, easiest, approach.
There are more performant ways to do this, but they likely require more complexity.
Expect this approach to be improved upon if not fully replaced in the future.
  • Loading branch information
@kaladay
kaladay committed Mar 25, 2021
1 parent f9ad707 commit cc7b9ce
Show file tree
Hide file tree
Showing 12 changed files with 247 additions and 1,074 deletions.
2 changes: 2 additions & 0 deletions domain/src/main/java/edu/tamu/catalog/domain/model/LoanItem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ public class LoanItem {

private String instanceId;

private String instanceHrid;

private Date loanDate;

private Date loanDueDate;
Expand Down
14 changes: 10 additions & 4 deletions domain/src/test/java/edu/tamu/catalog/domain/model/LoanItemTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public void testCreateAllArgs() {
final Date now = new Date();
final Date due = Date.from(now.toInstant().plusSeconds(100));

final LoanItem loanItem = new LoanItem("loanId", "itemId", "instanceId", now, due, false, "title", "author");
final LoanItem loanItem = new LoanItem("loanId", "itemId", "instanceId", "instanceHrid", now, due, false, "title", "author");

assertNotNull(loanItem);
assertNotNull(loanItem.getLoanDate());
Expand All @@ -29,6 +29,7 @@ public void testCreateAllArgs() {
assertEquals("loanId", loanItem.getLoanId());
assertEquals("itemId", loanItem.getItemId());
assertEquals("instanceId", loanItem.getInstanceId());
assertEquals("instanceHrid", loanItem.getInstanceHrid());
assertEquals(now.toString(), loanItem.getLoanDate().toString());
assertEquals(due.toString(), loanItem.getLoanDueDate().toString());
assertEquals(false, loanItem.isOverdue());
Expand All @@ -45,6 +46,7 @@ public void testCreateNoArgs() {
assertNull(loanItem.getLoanId());
assertNull(loanItem.getItemId());
assertNull(loanItem.getInstanceId());
assertNull(loanItem.getInstanceHrid());
assertNull(loanItem.getLoanDate());
assertNull(loanItem.getLoanDueDate());
assertFalse(loanItem.isOverdue());
Expand All @@ -60,6 +62,7 @@ public void testBuilder() {
.loanId("loanId")
.itemId("itemId")
.instanceId("instanceId")
.instanceHrid("instanceHrid")
.loanDate(now)
.loanDueDate(due)
.overdue(false)
Expand All @@ -74,6 +77,7 @@ public void testBuilder() {
assertEquals("loanId", loanItem.getLoanId());
assertEquals("itemId", loanItem.getItemId());
assertEquals("instanceId", loanItem.getInstanceId());
assertEquals("instanceHrid", loanItem.getInstanceHrid());
assertEquals(now.toString(), loanItem.getLoanDate().toString());
assertEquals(due.toString(), loanItem.getLoanDueDate().toString());
assertEquals(false, loanItem.isOverdue());
Expand All @@ -88,11 +92,12 @@ public void testUpdate() {
final Date later = Date.from(due.toInstant().plusSeconds(100));
final Date dueLater = Date.from(later.toInstant().plusSeconds(100));

final LoanItem loanItem = new LoanItem("loanId", "itemId", "instanceId", now, due, false, "title", "author");
final LoanItem loanItem = new LoanItem("loanId", "itemId", "instanceId", "instanceHrid", now, due, false, "title", "author");

loanItem.setLoanId("updatedLoanId");
loanItem.setItemId("updatedItemId");
loanItem.setInstanceId("updatedInstanceId");
loanItem.setInstanceHrid("updatedInstanceHrid");
loanItem.setLoanDate(later);
loanItem.setLoanDueDate(dueLater);
loanItem.setOverdue(true);
Expand All @@ -106,6 +111,7 @@ public void testUpdate() {
assertEquals("updatedLoanId", loanItem.getLoanId());
assertEquals("updatedItemId", loanItem.getItemId());
assertEquals("updatedInstanceId", loanItem.getInstanceId());
assertEquals("updatedInstanceHrid", loanItem.getInstanceHrid());
assertEquals(later.toString(), loanItem.getLoanDate().toString());
assertEquals(dueLater.toString(), loanItem.getLoanDueDate().toString());
assertEquals(true, loanItem.isOverdue());
Expand All @@ -118,8 +124,8 @@ public void testEquals() {
final Date now = new Date();
final Date due = Date.from(now.toInstant().plusSeconds(100));

final LoanItem loanItem1 = new LoanItem("loanId", "itemId", "instanceId", now, due, false, "title", "author");
final LoanItem loanItem2 = new LoanItem("loanId", "itemId", "instanceId", now, due, false, "title", "author");
final LoanItem loanItem1 = new LoanItem("loanId", "itemId", "instanceId", "instanceHrid", now, due, false, "title", "author");
final LoanItem loanItem2 = new LoanItem("loanId", "itemId", "instanceId", "instanceHrid", now, due, false, "title", "author");
final LoanItem loanItem3 = new LoanItem();

assertTrue(loanItem1.equals(loanItem2));
Expand Down
26 changes: 23 additions & 3 deletions service/src/main/java/edu/tamu/catalog/service/FolioCatalogService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,10 +353,10 @@ private String getServicePointDisplayName(String servicePointId) {
if (Objects.isNull(servicePointId)) {
return null;
}
String sercicePointUrl = String.format("%s/service-points/%s", properties.getBaseOkapiUrl(), servicePointId);
logger.debug("Asking for service-point from: {}", sercicePointUrl);
String servicePointUrl = String.format("%s/service-points/%s", properties.getBaseOkapiUrl(), servicePointId);
logger.debug("Asking for service-point from: {}", servicePointUrl);
String message = String.format("service point with id \"%s\"", servicePointId);
JsonNode response = okapiRequestJsonNode(sercicePointUrl, HttpMethod.GET, message);
JsonNode response = okapiRequestJsonNode(servicePointUrl, HttpMethod.GET, message);
if (response.isContainerNode()) {
JsonNode discoveryDisplayName = response.at("/discoveryDisplayName");
if (discoveryDisplayName.isValueNode()) {
Expand Down Expand Up @@ -604,11 +604,31 @@ private LoanItem buildLoanItem(JsonNode loan) throws Exception {
.overdue(getBoolean(loan, "/overdue", false))
.itemId(getText(loan, "/item/itemId"))
.instanceId(getText(loan, "/item/instanceId"))
.instanceHrid(getInstanceHrid(loan))
.title(getText(loan, "/item/title"))
.author(getText(loan, "/item/author"))
.build();
}

private String getInstanceHrid(JsonNode loan) throws Exception {
// This approach is not the most efficient way and may incur network performance problems for large numbers of instances.
String instanceId = getText(loan, "/item/instanceId");
String url = String.format("%s/instance-storage/instances/%s", properties.getBaseOkapiUrl(), instanceId);
String message = String.format("user with instanceId \"%s\"", instanceId);

logger.debug("Asking for instance from: {}", url);

JsonNode response = okapiRequestJsonNode(url, HttpMethod.GET, message);
if (response.isObject()) {
JsonNode hrid = response.at("/hrid");
if (hrid.isValueNode()) {
return hrid.asText();
}
}

return null;
}

/**
* Get parsed data time from JsonNode at path expression. Return null if value not found.
*
Expand Down
98 changes: 90 additions & 8 deletions service/src/test/java/edu/tamu/catalog/controller/PatronControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ public class PatronControllerTest extends AbstractTestRestController {
private static final String UIN = "1234567890";
private static final String SERVICE_POINTS_ID = "ebab9ccc-4ece-4f35-bc82-01f3325abed8";
private static final String REQUEST_ID = "8bbac557-d66f-4571-bbbf-47a107cc1589";
private static final String INSTANCE_ID1 = "2422160d-23c4-356b-ad1c-44d90fc1320b";
private static final String INSTANCE_ID2 = "829fecd3-67c3-3ca2-b9d4-281227690e0f";
private static final String INSTANCE_ID3 = "b8ea27b8-5280-3023-bbf8-9113849120a1";
private static final String INSTANCE_ID4 = "a0480d3f-181e-3abd-a091-288e1cfc05ab";
private static final String ITEM_ID = "40053ccb-fd0c-304b-9547-b2fc06f34d3e";

private static final String FOLIO_CATALOG = "folio";
Expand All @@ -94,6 +98,10 @@ public class PatronControllerTest extends AbstractTestRestController {
private static Resource patronAccountCancelHoldResponseResource;
private static Resource holdRequestResource;
private static Resource servicePointResource;
private static Resource instance1Resource;
private static Resource instance2Resource;
private static Resource instance3Resource;
private static Resource instance4Resource;

private static Resource finesCatalogResource;
private static Resource loansCatalogResource;
Expand Down Expand Up @@ -148,15 +156,38 @@ public void testLoansMockMVC() throws Exception {
fieldWithPath("[].loanId").description("The loan id."),
fieldWithPath("[].itemId").description("The item id."),
fieldWithPath("[].instanceId").description("The instance id."),
fieldWithPath("[].instanceHrid").description("The instance human-readable id."),
fieldWithPath("[].loanDate").description("The loan date."),
fieldWithPath("[].loanDueDate").description("The loan due date."),
fieldWithPath("[].overdue").description("Is the loan overdue."),
fieldWithPath("[].title").description("The title of the loan item."),
fieldWithPath("[].author").description("The author of the loan item.")
);

performPatronGetWithMockMVC(getLoansUrl(), LOANS_ENDPOINT, pathParameters, requestParameters,
responseFields, loadJsonResource(loansCatalogResource));
expectGetResponse(getLoansUrl(), once(), respondJsonSuccess(patronAccountResource));
expectOkapiLoginResponse(between(0, 4), withStatus(CREATED));
expectGetResponse(getOkapiInstancesUrl(INSTANCE_ID1), once(), respondJsonSuccess(instance1Resource));
expectGetResponse(getOkapiInstancesUrl(INSTANCE_ID2), once(), respondJsonSuccess(instance2Resource));
expectGetResponse(getOkapiInstancesUrl(INSTANCE_ID3), once(), respondJsonSuccess(instance3Resource));
expectGetResponse(getOkapiInstancesUrl(INSTANCE_ID4), once(), respondJsonSuccess(instance4Resource));

mockMvc.perform(
get(PATRON_MVC_PREFIX + LOANS_ENDPOINT, UIN)
.param(CATALOG_FIELD, FOLIO_CATALOG)
.contentType(MediaType.APPLICATION_JSON_UTF8)
.accept(MediaType.APPLICATION_JSON_UTF8, MediaType.TEXT_HTML)
)
.andExpect(status().isOk())
.andExpect(content().contentType(MediaType.APPLICATION_JSON_UTF8))
.andExpect(content().json(loadJsonResource(loansCatalogResource)))
.andDo(
document(
DOC_PREFIX + LOANS_ENDPOINT,
pathParameters,
requestParameters,
responseFields
)
);

restServer.verify();
}
Expand All @@ -176,6 +207,7 @@ public void testLoanItemRenewalMockMVC() throws Exception {
fieldWithPath("loanId").description("The loan id."),
fieldWithPath("itemId").description("The item id."),
fieldWithPath("instanceId").description("The instance id."),
fieldWithPath("instanceHrid").description("The instance human-readable id."),
fieldWithPath("loanDate").description("The loan date."),
fieldWithPath("loanDueDate").description("The loan due date."),
fieldWithPath("overdue").description("Is the loan overdue."),
Expand All @@ -184,6 +216,8 @@ public void testLoanItemRenewalMockMVC() throws Exception {
);

expectPostResponse(getLoanItemRenewalUrl(), once(), respondJsonOk(patronAccountRenewalResource));
expectOkapiLoginResponse(between(0, 1), withStatus(CREATED));
expectGetResponse(getOkapiInstancesUrl(INSTANCE_ID1), once(), respondJsonSuccess(instance1Resource));

mockMvc.perform(
post(PATRON_MVC_PREFIX + RENEW_MVC_PATH, UIN, ITEM_ID)
Expand Down Expand Up @@ -326,6 +360,24 @@ public void testEndpoints(MockHttpServletRequestBuilder builder, String url, Htt
restServer.verify();
}

@ParameterizedTest
@MethodSource("argumentsLoansResponses")
public void testLoansEndpoints(MockHttpServletRequestBuilder builder, String url, HttpMethod method,
ExpectedCount count, DefaultResponseCreator response, ResultMatcher result) throws Exception {

expectResponse(url, method, count, response);
expectOkapiLoginResponse(between(0, 4), withStatus(CREATED));
expectGetResponse(getOkapiInstancesUrl(INSTANCE_ID1), between(0, 1), respondJsonSuccess(instance1Resource));
expectGetResponse(getOkapiInstancesUrl(INSTANCE_ID2), between(0, 1), respondJsonSuccess(instance2Resource));
expectGetResponse(getOkapiInstancesUrl(INSTANCE_ID3), between(0, 1), respondJsonSuccess(instance3Resource));
expectGetResponse(getOkapiInstancesUrl(INSTANCE_ID4), between(0, 1), respondJsonSuccess(instance4Resource));

mockMvc.perform(builder)
.andExpect(result);

restServer.verify();
}

@ParameterizedTest
@MethodSource("argumentsHoldsResponses")
public void testHoldsEndpoint(MockHttpServletRequestBuilder builder, String url,
Expand Down Expand Up @@ -372,12 +424,18 @@ private void performPatronGetWithMockMVC(String url, String endpoint, PathParame

private static Stream<? extends Arguments> argumentsEndpointResponses() throws Exception {
return Stream.of(
streamOfFines(),
streamOfLoans(),
streamOfLoanItems(),
streamOfHoldsCancel()
)
.flatMap(stream -> stream);
streamOfFines(),
streamOfHoldsCancel()
)
.flatMap(stream -> stream);
}

private static Stream<? extends Arguments> argumentsLoansResponses() throws Exception {
return Stream.of(
streamOfLoans(),
streamOfLoanItems()
)
.flatMap(stream -> stream);
}

private static Stream<? extends Arguments> argumentsHoldsResponses() throws Exception {
Expand Down Expand Up @@ -512,6 +570,10 @@ private static String getOkapiRequestsUrl() {
return getOkapiUrl(String.format("circulation/requests/%s", REQUEST_ID));
}

private static String getOkapiInstancesUrl(String instanceId) {
return getOkapiUrl(String.format("instance-storage/instances/%s", instanceId));
}

private static String getCancelHoldRequestUrl() {
return String.format("%spatron/account/%s/holds/%s/cancel?apikey=%s", BASE_PATH, UIN, REQUEST_ID, API_KEY);
}
Expand Down Expand Up @@ -550,6 +612,26 @@ public void setServicePointResource(Resource resource) {
servicePointResource = resource;
}

@Value("classpath:mock/response/instances/in1.json")
public void setInstance1Resource(Resource resource) {
instance1Resource = resource;
}

@Value("classpath:mock/response/instances/in2.json")
public void setInstance2Resource(Resource resource) {
instance2Resource = resource;
}

@Value("classpath:mock/response/instances/in3.json")
public void setInstance3Resource(Resource resource) {
instance3Resource = resource;
}

@Value("classpath:mock/response/instances/in4.json")
public void setInstance4Resource(Resource resource) {
instance4Resource = resource;
}

@Value("classpath:mock/response/catalog/fines.json")
public void setFinesCatalogResource(Resource resource) {
finesCatalogResource = resource;
Expand Down
1 change: 1 addition & 0 deletions service/src/test/resources/mock/response/catalog/loanRenewal.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"loanId": "9b9b592a-1b4c-43b4-a91f-901d4ec9d3d6",
"itemId": "40053ccb-fd0c-304b-9547-b2fc06f34d3e",
"instanceId": "2422160d-23c4-356b-ad1c-44d90fc1320b",
"instanceHrid": "in1",
"loanDate": 1297982768000,
"loanDueDate": 1633744800000,
"overdue": false,
Expand Down

0 comments on commit cc7b9ce

Please sign in to comment.