Skip to content

Commit b11d2d6

Browse files
JohnnyMendesCJohnnyMendesC
committed
test(DSpace#9611): Fix AccessConditionOptionMatcher validation logic
Updates AccessConditionOptionMatcher to correctly validate date limits using DateMathParser instead of ignoring the arguments. Refactors BulkAccessConditionRestRepositoryIT to use dynamic configuration values instead of hardcoded strings.
1 parent f94fb41 commit b11d2d6

2 files changed

Lines changed: 110 additions & 16 deletions

File tree

‎dspace-server-webapp/src/test/java/org/dspace/app/rest/BulkAccessConditionRestRepositoryIT.java‎

Lines changed: 57 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,21 @@
1515
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
1616
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
1717

18+
import java.util.List;
19+
20+
import org.dspace.app.bulkaccesscontrol.model.BulkAccessConditionConfiguration;
21+
import org.dspace.app.bulkaccesscontrol.service.BulkAccessConditionConfigurationService;
1822
import org.dspace.app.rest.matcher.AccessConditionOptionMatcher;
1923
import org.dspace.app.rest.test.AbstractControllerIntegrationTest;
2024
import org.dspace.builder.CollectionBuilder;
2125
import org.dspace.builder.CommunityBuilder;
2226
import org.dspace.builder.ItemBuilder;
2327
import org.dspace.content.Collection;
2428
import org.dspace.content.Community;
29+
import org.dspace.submit.model.AccessConditionOption;
2530
import org.hamcrest.Matchers;
2631
import org.junit.Test;
32+
import org.springframework.beans.factory.annotation.Autowired;
2733

2834
/**
2935
* Integration test class for the bulkaccessconditionoptions endpoint.
@@ -32,26 +38,40 @@
3238
*/
3339
public class BulkAccessConditionRestRepositoryIT extends AbstractControllerIntegrationTest {
3440

41+
@Autowired
42+
private BulkAccessConditionConfigurationService bulkAccessConditionConfigurationService;
43+
3544
@Test
3645
public void findAllByAdminUserTest() throws Exception {
3746
String authToken = getAuthToken(admin.getEmail(), password);
47+
String embargoLimit = getLimitForOption("embargo", "startDate");
48+
String leaseLimit = getLimitForOption("lease", "endDate");
49+
3850
getClient(authToken)
3951
.perform(get("/api/config/bulkaccessconditionoptions"))
4052
.andExpect(status().isOk())
4153
.andExpect(jsonPath("$.page.totalElements", greaterThanOrEqualTo(1)))
4254
.andExpect(jsonPath("$._embedded.bulkaccessconditionoptions", containsInAnyOrder(allOf(
4355
hasJsonPath("$.id", is("default")),
4456
hasJsonPath("$.itemAccessConditionOptions", Matchers.containsInAnyOrder(
45-
AccessConditionOptionMatcher.matchAccessConditionOption("openaccess", false , false, null, null),
46-
AccessConditionOptionMatcher.matchAccessConditionOption("embargo", true , false, "+36MONTHS", null),
47-
AccessConditionOptionMatcher.matchAccessConditionOption("administrator", false , false, null, null),
48-
AccessConditionOptionMatcher.matchAccessConditionOption("lease", false , true, null, "+6MONTHS"))
57+
AccessConditionOptionMatcher.matchAccessConditionOption(
58+
"openaccess", false , false, null, null),
59+
AccessConditionOptionMatcher.matchAccessConditionOption(
60+
"embargo", true , false, embargoLimit, null),
61+
AccessConditionOptionMatcher.matchAccessConditionOption(
62+
"administrator", false , false, null, null),
63+
AccessConditionOptionMatcher.matchAccessConditionOption(
64+
"lease", false , true, null, leaseLimit))
4965
),
5066
hasJsonPath("$.bitstreamAccessConditionOptions", Matchers.containsInAnyOrder(
51-
AccessConditionOptionMatcher.matchAccessConditionOption("openaccess", false , false, null, null),
52-
AccessConditionOptionMatcher.matchAccessConditionOption("embargo", true , false, "+36MONTHS", null),
53-
AccessConditionOptionMatcher.matchAccessConditionOption("administrator", false , false, null, null),
54-
AccessConditionOptionMatcher.matchAccessConditionOption("lease", false , true, null, "+6MONTHS"))
67+
AccessConditionOptionMatcher.matchAccessConditionOption(
68+
"openaccess", false , false, null, null),
69+
AccessConditionOptionMatcher.matchAccessConditionOption(
70+
"embargo", true , false, embargoLimit, null),
71+
AccessConditionOptionMatcher.matchAccessConditionOption(
72+
"administrator", false , false, null, null),
73+
AccessConditionOptionMatcher.matchAccessConditionOption(
74+
"lease", false , true, null, leaseLimit))
5575
)))));
5676
}
5777

@@ -140,21 +160,24 @@ public void findAllByAnonymousUserTest() throws Exception {
140160
@Test
141161
public void findOneByAdminTest() throws Exception {
142162
String tokenAdmin = getAuthToken(admin.getEmail(), password);
163+
String embargoLimit = getLimitForOption("embargo", "startDate");
164+
String leaseLimit = getLimitForOption("lease", "endDate");
165+
143166
getClient(tokenAdmin)
144167
.perform(get("/api/config/bulkaccessconditionoptions/default"))
145168
.andExpect(status().isOk())
146169
.andExpect(jsonPath("$.id", is("default")))
147170
.andExpect(jsonPath("$.itemAccessConditionOptions", Matchers.containsInAnyOrder(
148171
AccessConditionOptionMatcher.matchAccessConditionOption("openaccess", false , false, null, null),
149-
AccessConditionOptionMatcher.matchAccessConditionOption("embargo", true , false, "+36MONTHS", null),
172+
AccessConditionOptionMatcher.matchAccessConditionOption("embargo", true , false, embargoLimit, null),
150173
AccessConditionOptionMatcher.matchAccessConditionOption("administrator", false , false, null, null),
151-
AccessConditionOptionMatcher.matchAccessConditionOption("lease", false , true, null, "+6MONTHS"))
174+
AccessConditionOptionMatcher.matchAccessConditionOption("lease", false , true, null, leaseLimit))
152175
))
153176
.andExpect(jsonPath("$.bitstreamAccessConditionOptions", Matchers.containsInAnyOrder(
154177
AccessConditionOptionMatcher.matchAccessConditionOption("openaccess", false , false, null, null),
155-
AccessConditionOptionMatcher.matchAccessConditionOption("embargo", true , false, "+36MONTHS", null),
178+
AccessConditionOptionMatcher.matchAccessConditionOption("embargo", true , false, embargoLimit, null),
156179
AccessConditionOptionMatcher.matchAccessConditionOption("administrator", false , false, null, null),
157-
AccessConditionOptionMatcher.matchAccessConditionOption("lease", false , true, null, "+6MONTHS"))
180+
AccessConditionOptionMatcher.matchAccessConditionOption("lease", false , true, null, leaseLimit))
158181
))
159182
.andExpect(jsonPath("$.type", is("bulkaccessconditionoption")));
160183
}
@@ -253,4 +276,26 @@ public void findOneNotFoundTest() throws Exception {
253276
.andExpect(status().isNotFound());
254277
}
255278

279+
/**
280+
* Helper method to extract limits from the current configuration dynamically.
281+
*/
282+
private String getLimitForOption(String optionName, String limitType) {
283+
BulkAccessConditionConfiguration config = bulkAccessConditionConfigurationService.
284+
getBulkAccessConditionConfiguration("default");
285+
286+
if (config != null) {
287+
List<AccessConditionOption> options = config.getItemAccessConditionOptions();
288+
for (AccessConditionOption option : options) {
289+
if (option.getName().equals(optionName)) {
290+
if ("startDate".equals(limitType)) {
291+
return option.getStartDateLimit();
292+
} else if ("endDate".equals(limitType)) {
293+
return option.getEndDateLimit();
294+
}
295+
}
296+
}
297+
}
298+
return null;
299+
}
300+
256301
}

‎dspace-server-webapp/src/test/java/org/dspace/app/rest/matcher/AccessConditionOptionMatcher.java‎

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,17 @@
1111
import static com.jayway.jsonpath.matchers.JsonPathMatchers.hasNoJsonPath;
1212
import static org.hamcrest.Matchers.allOf;
1313
import static org.hamcrest.Matchers.is;
14-
import static org.hamcrest.Matchers.notNullValue;
1514

15+
import java.time.LocalDate;
16+
import java.time.LocalDateTime;
17+
import java.time.format.DateTimeFormatter;
18+
import java.time.format.DateTimeParseException;
1619
import java.util.Objects;
1720

1821
import org.apache.commons.lang3.StringUtils;
22+
import org.dspace.util.DateMathParser;
23+
import org.hamcrest.BaseMatcher;
24+
import org.hamcrest.Description;
1925
import org.hamcrest.Matcher;
2026

2127
/**
@@ -36,11 +42,54 @@ public static Matcher<? super Object> matchAccessConditionOption(String name,
3642
: hasNoJsonPath("$.hasStartDate"),
3743
Objects.nonNull(hasEndDate) ? hasJsonPath("$.hasEndDate", is(hasEndDate))
3844
: hasNoJsonPath("$.hasEndDate"),
39-
StringUtils.isNotBlank(maxStartDate) ? hasJsonPath("$.maxStartDate", notNullValue())
45+
StringUtils.isNotBlank(maxStartDate) ? hasJsonPath("$.maxStartDate", new DateMathMatcher(maxStartDate))
4046
: hasNoJsonPath("$.maxStartDate"),
41-
StringUtils.isNotBlank(maxEndDate) ? hasJsonPath("$.maxEndDate", notNullValue())
47+
StringUtils.isNotBlank(maxEndDate) ? hasJsonPath("$.maxEndDate", new DateMathMatcher(maxEndDate))
4248
: hasNoJsonPath("$.maxEndDate")
4349
);
4450
}
4551

46-
}
52+
/**
53+
* Internal matcher to compare an ISO date from JSON with a DateMath expression.
54+
*/
55+
private static class DateMathMatcher extends BaseMatcher<String> {
56+
private final String mathExpression;
57+
private final LocalDate expectedDate;
58+
59+
public DateMathMatcher(String mathExpression) {
60+
this.mathExpression = mathExpression;
61+
try {
62+
DateMathParser dmp = new DateMathParser();
63+
LocalDateTime calculated = dmp.parseMath(mathExpression);
64+
this.expectedDate = calculated.toLocalDate();
65+
} catch (Exception e) {
66+
throw new RuntimeException("Error calculating DateMath in Matcher: " + mathExpression, e);
67+
}
68+
}
69+
70+
@Override
71+
public boolean matches(Object item) {
72+
if (!(item instanceof String)) {
73+
return false;
74+
}
75+
String dateStr = (String) item;
76+
try {
77+
LocalDate actualDate;
78+
79+
if (dateStr.length() > 10) {
80+
actualDate = LocalDate.parse(dateStr, DateTimeFormatter.ISO_DATE_TIME);
81+
} else {
82+
actualDate = LocalDate.parse(dateStr);
83+
}
84+
return expectedDate.equals(actualDate);
85+
} catch (DateTimeParseException e) {
86+
return false;
87+
}
88+
}
89+
90+
@Override
91+
public void describeTo(Description description) {
92+
description.appendText("A date matching expression'" + mathExpression + "' (" + expectedDate + ")");
93+
}
94+
}
95+
}

0 commit comments

Comments
 (0)