Skip to content

Commit 58a2ff9

Browse files
milanmajchrakclaude
andcommitted
test: stop bitstream-format leak, de-randomize format names, commit view event
PreviewContentServiceImplIT deleted its custom format through the shared test context after earlier builder deletes had detached it from the thread-bound session, so the delete was silently lost - one leaked row per test broke BitstreamFormatRestRepositoryIT counts when class order put Preview first. totalVisitsReport_Item_Visited could query a stale Solr searcher before its posted view event became visible. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 2d60931 commit 58a2ff9

3 files changed

Lines changed: 56 additions & 31 deletions

File tree

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

Lines changed: 44 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121

2222
import java.util.Arrays;
2323
import java.util.Map;
24-
import java.util.Random;
2524
import java.util.concurrent.atomic.AtomicReference;
2625

2726
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -39,7 +38,9 @@
3938
import org.dspace.eperson.EPerson;
4039
import org.hamcrest.Matchers;
4140
import org.junit.Ignore;
41+
import org.junit.Rule;
4242
import org.junit.Test;
43+
import org.junit.rules.TestName;
4344
import org.springframework.beans.factory.annotation.Autowired;
4445
import org.springframework.test.web.servlet.MvcResult;
4546

@@ -56,7 +57,8 @@ public class BitstreamFormatRestRepositoryIT extends AbstractControllerIntegrati
5657
@Autowired
5758
private BitstreamFormatConverter bitstreamFormatConverter;
5859

59-
private final int DEFAULT_AMOUNT_FORMATS = 95;
60+
@Rule
61+
public TestName testName = new TestName();
6062

6163
@Test
6264
public void findAllPaginationTest() throws Exception {
@@ -136,7 +138,7 @@ public void findOneNonExistentIDInURL() throws Exception {
136138
@Test
137139
public void createAdminAccess() throws Exception {
138140
ObjectMapper mapper = new ObjectMapper();
139-
BitstreamFormatRest bitstreamFormatRest = this.createRandomMockBitstreamRest(false);
141+
BitstreamFormatRest bitstreamFormatRest = this.createMockBitstreamRest();
140142

141143
//Create bitstream format
142144
String token = getAuthToken(admin.getEmail(), password);
@@ -145,10 +147,15 @@ public void createAdminAccess() throws Exception {
145147
AtomicReference<Integer> idRef = new AtomicReference<>();
146148
try {
147149

150+
// Capture the id right after the status check, so the format is cleaned up even when
151+
// a later assertion in this test fails.
148152
MvcResult mvcResult = getClient(token).perform(post("/api/core/bitstreamformats/")
149153
.content(mapper.writeValueAsBytes(bitstreamFormatRest))
150154
.contentType(contentType))
151155
.andExpect(status().isCreated())
156+
.andDo(result -> idRef
157+
.set(read(result.getResponse().getContentAsString(),
158+
"$.id")))
152159
.andExpect(jsonPath("$", HalMatcher.matchNoEmbeds()))
153160
.andReturn();
154161

@@ -168,13 +175,13 @@ public void createAdminAccess() throws Exception {
168175
bitstreamFormatRest.getMimetype(),
169176
bitstreamFormatRest.getDescription(),
170177
bitstreamFormatRest.getShortDescription())
171-
)))
172-
.andDo(result -> idRef
173-
.set(read(result.getResponse().getContentAsString(), "$.id")));
178+
)));
174179

175180
} finally {
176-
// Delete the created community (cleanup after ourselves!)
177-
BitstreamFormatBuilder.deleteBitstreamFormat(idRef.get());
181+
// Delete the created bitstream format (cleanup after ourselves!)
182+
if (idRef.get() != null) {
183+
BitstreamFormatBuilder.deleteBitstreamFormat(idRef.get());
184+
}
178185
}
179186

180187

@@ -183,8 +190,9 @@ public void createAdminAccess() throws Exception {
183190
@Test
184191
public void createNonValidSupportLevel() throws Exception {
185192
ObjectMapper mapper = new ObjectMapper();
186-
BitstreamFormatRest bitstreamFormatRest = this.createRandomMockBitstreamRest(false);
193+
BitstreamFormatRest bitstreamFormatRest = this.createMockBitstreamRest();
187194
bitstreamFormatRest.setSupportLevel("NONVALID SUPPORT LVL");
195+
int totalFormatsBefore = currentTotalFormats();
188196
//Attempt to create bitstream with a non-valid support lvl
189197
String token = getAuthToken(admin.getEmail(), password);
190198
getClient(token).perform(post("/api/core/bitstreamformats/")
@@ -194,13 +202,14 @@ public void createNonValidSupportLevel() throws Exception {
194202
// Check that no new bitstreamformat was created
195203
getClient().perform(get("/api/core/bitstreamformats/"))
196204
.andExpect(status().isOk())
197-
.andExpect(jsonPath("$.page.totalElements", is(DEFAULT_AMOUNT_FORMATS)));
205+
.andExpect(jsonPath("$.page.totalElements", is(totalFormatsBefore)));
198206
}
199207

200208
@Test
201209
public void createNoAccess() throws Exception {
202210
ObjectMapper mapper = new ObjectMapper();
203-
BitstreamFormatRest bitstreamFormatRest = this.createRandomMockBitstreamRest(false);
211+
BitstreamFormatRest bitstreamFormatRest = this.createMockBitstreamRest();
212+
int totalFormatsBefore = currentTotalFormats();
204213

205214
//Try to create bitstreamFormat without auth token
206215
getClient(null).perform(post("/api/core/bitstreamformats/")
@@ -210,13 +219,14 @@ public void createNoAccess() throws Exception {
210219
// Check that no new bitstreamformat was created
211220
getClient().perform(get("/api/core/bitstreamformats/"))
212221
.andExpect(status().isOk())
213-
.andExpect(jsonPath("$.page.totalElements", is(DEFAULT_AMOUNT_FORMATS)));
222+
.andExpect(jsonPath("$.page.totalElements", is(totalFormatsBefore)));
214223
}
215224

216225
@Test
217226
public void createNonAdminAccess() throws Exception {
218227
ObjectMapper mapper = new ObjectMapper();
219-
BitstreamFormatRest bitstreamFormatRest = this.createRandomMockBitstreamRest(false);
228+
BitstreamFormatRest bitstreamFormatRest = this.createMockBitstreamRest();
229+
int totalFormatsBefore = currentTotalFormats();
220230
context.turnOffAuthorisationSystem();
221231
EPerson user = EPersonBuilder.createEPerson(context)
222232
.withNameInMetadata("first", "last")
@@ -234,13 +244,14 @@ public void createNonAdminAccess() throws Exception {
234244
// Check that no new bitstreamformat was created
235245
getClient().perform(get("/api/core/bitstreamformats/"))
236246
.andExpect(status().isOk())
237-
.andExpect(jsonPath("$.page.totalElements", is(DEFAULT_AMOUNT_FORMATS)));
247+
.andExpect(jsonPath("$.page.totalElements", is(totalFormatsBefore)));
238248
}
239249

240250
@Test
241251
public void createAlreadyExisting() throws Exception {
242252
ObjectMapper mapper = new ObjectMapper();
243-
BitstreamFormatRest bitstreamFormatRest = this.createRandomMockBitstreamRest(true);
253+
BitstreamFormatRest bitstreamFormatRest = this.createMockBitstreamRest();
254+
int totalFormatsBefore = currentTotalFormats();
244255

245256
// Capture the Id of the created BitstreamFormat (see andDo() below)
246257
AtomicReference<Integer> idRef = new AtomicReference<>();
@@ -264,12 +275,14 @@ public void createAlreadyExisting() throws Exception {
264275
// Check that the new bitstreamformat was created only once
265276
getClient().perform(get("/api/core/bitstreamformats/"))
266277
.andExpect(status().isOk())
267-
.andExpect(jsonPath("$.page.totalElements", is(DEFAULT_AMOUNT_FORMATS + 1)));
278+
.andExpect(jsonPath("$.page.totalElements", is(totalFormatsBefore + 1)));
268279

269280

270281
} finally {
271-
// Delete the created community (cleanup after ourselves!)
272-
BitstreamFormatBuilder.deleteBitstreamFormat(idRef.get());
282+
// Delete the created bitstream format (cleanup after ourselves!)
283+
if (idRef.get() != null) {
284+
BitstreamFormatBuilder.deleteBitstreamFormat(idRef.get());
285+
}
273286
}
274287
}
275288

@@ -645,19 +658,25 @@ public void deleteNonAdminAccess() throws Exception {
645658
)));
646659
}
647660

648-
private BitstreamFormatRest createRandomMockBitstreamRest(boolean withRand) {
661+
/**
662+
* Short descriptions are unique in the format registry, so derive them from the test method
663+
* name: deterministic and collision-free, unlike the random numbers used before.
664+
*/
665+
private BitstreamFormatRest createMockBitstreamRest() {
649666
BitstreamFormatRest bitstreamFormatRest = new BitstreamFormatRest();
650-
String random = null;
651-
if (withRand) {
652-
Random rand = new Random();
653-
random = String.valueOf(rand.nextInt(100) + 1);
654-
}
655-
bitstreamFormatRest.setShortDescription("Test short" + random);
667+
bitstreamFormatRest.setShortDescription("Test short " + testName.getMethodName());
656668
bitstreamFormatRest.setDescription("Full description of Test short");
657669
bitstreamFormatRest.setMimetype("text/plain");
658670
bitstreamFormatRest.setSupportLevel("KNOWN");
659671
bitstreamFormatRest.setInternal(false);
660672
bitstreamFormatRest.setExtensions(Arrays.asList("txt", "asc"));
661673
return bitstreamFormatRest;
662674
}
675+
676+
private int currentTotalFormats() throws Exception {
677+
String response = getClient().perform(get("/api/core/bitstreamformats/"))
678+
.andExpect(status().isOk())
679+
.andReturn().getResponse().getContentAsString();
680+
return read(response, "$.page.totalElements");
681+
}
663682
}

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.dspace.app.rest.test.AbstractControllerIntegrationTest;
2222
import org.dspace.authorize.AuthorizeException;
2323
import org.dspace.builder.BitstreamBuilder;
24+
import org.dspace.builder.BitstreamFormatBuilder;
2425
import org.dspace.builder.BundleBuilder;
2526
import org.dspace.builder.CollectionBuilder;
2627
import org.dspace.builder.CommunityBuilder;
@@ -33,7 +34,6 @@
3334
import org.dspace.content.Community;
3435
import org.dspace.content.Item;
3536
import org.dspace.content.PreviewContent;
36-
import org.dspace.content.service.BitstreamFormatService;
3737
import org.dspace.content.service.PreviewContentService;
3838
import org.dspace.util.FileInfo;
3939
import org.junit.After;
@@ -46,8 +46,6 @@ public class PreviewContentServiceImplIT extends AbstractControllerIntegrationTe
4646

4747
@Autowired
4848
PreviewContentService previewContentService;
49-
@Autowired
50-
BitstreamFormatService bitstreamFormatService;
5149

5250
PreviewContent previewContent0;
5351
PreviewContent previewContent1;
@@ -256,11 +254,15 @@ public void destroy() throws Exception {
256254
BitstreamBuilder.deleteBitstream(tarXzFileWithIncorrectMimeType.getID());
257255

258256
// removing custom mime type format created for tarXGzipFile and tgzFileWithGzipMimeType files
259-
if (customMimeTypeFormat != null) {
260-
bitstreamFormatService.delete(context, customMimeTypeFormat);
261-
}
257+
Integer customMimeTypeFormatId = customMimeTypeFormat == null ? null : customMimeTypeFormat.getID();
262258

263259
super.destroy();
260+
261+
// Must run in its own Context after super.destroy(): a delete through the shared test
262+
// context is never committed here, which leaked one format row per test.
263+
if (customMimeTypeFormatId != null) {
264+
BitstreamFormatBuilder.deleteBitstreamFormat(customMimeTypeFormatId);
265+
}
264266
}
265267

266268
@Test

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,10 @@ public void totalVisitsReport_Item_Visited() throws Exception {
403403
.contentType(contentType))
404404
.andExpect(status().isCreated());
405405

406+
// Commit the view event waiting for a new searcher so it is visible to the report query
407+
// below (same race and fix as in topCountriesReport_Community_Visited).
408+
StatisticsServiceFactory.getInstance().getSolrLoggerService().commit();
409+
406410
// And request that collection's TotalVisits stat report
407411
getClient(adminToken).perform(
408412
get("/api/statistics/usagereports/" + itemVisited.getID() + "_" + TOTAL_VISITS_REPORT_ID))

0 commit comments

Comments
 (0)