Skip to content

Commit 676d710

Browse files
bbpennelbbpennel
authored
Specifically check the top level error to see if it is a EofException during download of files. Add a unit test to verify the various exception paths (#1740)
1 parent f933e7c commit 676d710

File tree

2 files changed

+142
-9
lines changed

2 files changed

+142
-9
lines changed

‎web-access-app/src/main/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentController.java

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -102,24 +102,34 @@ private void streamData(String pidString, String datastream, boolean asAttachmen
102102
recordDownloadEvent(pid, datastream, principals, request);
103103
} catch (IOException e) {
104104
handleIOException(pid, datastream, e);
105+
response.setStatus(HttpServletResponse.SC_BAD_REQUEST);
105106
}
106107
}
107108

108109
private void handleIOException(PID pid, String datastream, IOException e) {
110+
if (connectionWasClosed(pid, datastream, e)) {
111+
return;
112+
}
109113
var cause = ExceptionUtils.getRootCause(e);
110-
if (cause != null) {
111-
if (cause.getMessage().contains("Connection reset by peer") || cause instanceof EOFException) {
112-
log.debug("Client reset connection while downloading {}/{}", pid.getId(), datastream);
113-
return;
114-
}
115-
if (cause instanceof TimeoutException) {
116-
log.warn("Request to download {}/{} timed out: {}", pid.getId(), datastream, e.getMessage());
117-
return;
118-
}
114+
if (connectionWasClosed(pid, datastream, cause)) {
115+
return;
119116
}
120117
log.error("Problem retrieving {}/{}", pid.getId(), datastream, e);
121118
}
122119

120+
private boolean connectionWasClosed(PID pid, String datastream, Throwable e) {
121+
if (e instanceof EOFException
122+
|| (e.getMessage() != null && e.getMessage().contains("Connection reset by peer"))) {
123+
log.debug("Client reset connection while downloading {}/{}", pid.getId(), datastream);
124+
return true;
125+
}
126+
if (e instanceof TimeoutException) {
127+
log.warn("Request to download {}/{} timed out: {}", pid.getId(), datastream, e.getMessage());
128+
return true;
129+
}
130+
return false;
131+
}
132+
123133
private void recordDownloadEvent(PID pid, String datastream, AccessGroupSet principals,
124134
HttpServletRequest request) {
125135
if (!(StringUtils.isBlank(datastream) || ORIGINAL_FILE.getId().equals(datastream))) {
@@ -154,4 +164,20 @@ public ResponseEntity<Object> handleArgumentTypeMismatch(RuntimeException ex) {
154164
log.debug("Argument type mismatch", ex);
155165
return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
156166
}
167+
168+
public void setFedoraContentService(FedoraContentService fedoraContentService) {
169+
this.fedoraContentService = fedoraContentService;
170+
}
171+
172+
public void setAnalyticsTracker(AnalyticsTrackerUtil analyticsTracker) {
173+
this.analyticsTracker = analyticsTracker;
174+
}
175+
176+
public void setRepositoryObjectLoader(RepositoryObjectLoader repoObjLoader) {
177+
this.repoObjLoader = repoObjLoader;
178+
}
179+
180+
public void setAccessControlService(AccessControlService accessControlService) {
181+
this.accessControlService = accessControlService;
182+
}
157183
}

‎web-access-app/src/test/java/edu/unc/lib/boxc/web/access/controllers/FedoraContentControllerTest.java

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
package edu.unc.lib.boxc.web.access.controllers;
2+
3+
import edu.unc.lib.boxc.auth.api.services.AccessControlService;
4+
import edu.unc.lib.boxc.model.api.ids.PID;
5+
import edu.unc.lib.boxc.model.api.objects.RepositoryObjectLoader;
6+
import edu.unc.lib.boxc.model.fcrepo.test.TestHelper;
7+
import edu.unc.lib.boxc.web.common.services.FedoraContentService;
8+
import edu.unc.lib.boxc.web.common.utils.AnalyticsTrackerUtil;
9+
import org.eclipse.jetty.io.EofException;
10+
import org.junit.jupiter.api.AfterEach;
11+
import org.junit.jupiter.api.BeforeEach;
12+
import org.junit.jupiter.api.Test;
13+
import org.mockito.Mock;
14+
import org.springframework.beans.factory.annotation.Autowired;
15+
import org.springframework.test.web.servlet.MockMvc;
16+
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
17+
18+
import java.io.IOException;
19+
import java.util.concurrent.TimeoutException;
20+
21+
import static org.mockito.ArgumentMatchers.any;
22+
import static org.mockito.ArgumentMatchers.anyBoolean;
23+
import static org.mockito.Mockito.doThrow;
24+
import static org.mockito.Mockito.when;
25+
import static org.mockito.MockitoAnnotations.openMocks;
26+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
27+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
28+
29+
/**
30+
* @author bbpennel
31+
*/
32+
public class FedoraContentControllerTest {
33+
protected MockMvc mvc;
34+
private AutoCloseable closeable;
35+
private FedoraContentController controller;
36+
@Mock
37+
private FedoraContentService fedoraContentService;
38+
@Mock
39+
private AnalyticsTrackerUtil analyticsTracker;
40+
@Mock
41+
private RepositoryObjectLoader repoObjLoader;
42+
@Mock
43+
private AccessControlService accessControlService;
44+
45+
@BeforeEach
46+
public void setup() throws Exception {
47+
closeable = openMocks(this);
48+
TestHelper.setContentBase("http://localhost:48085/rest");
49+
50+
controller = new FedoraContentController();
51+
controller.setFedoraContentService(fedoraContentService);
52+
controller.setAnalyticsTracker(analyticsTracker);
53+
controller.setRepositoryObjectLoader(repoObjLoader);
54+
controller.setAccessControlService(accessControlService);
55+
56+
mvc = MockMvcBuilders.standaloneSetup(controller).build();
57+
}
58+
59+
@AfterEach
60+
public void closeService() throws Exception {
61+
closeable.close();
62+
}
63+
64+
@Test
65+
public void getContentEofExceptionTest() throws Exception {
66+
PID pid = TestHelper.makePid();
67+
doThrow(new EofException((String) null))
68+
.when(fedoraContentService)
69+
.streamData(any(), any(), anyBoolean(), any());
70+
mvc.perform(get("/content/" + pid.getId()))
71+
.andExpect(status().isBadRequest())
72+
.andReturn();
73+
}
74+
75+
@Test
76+
public void getContentWrappedEofExceptionTest() throws Exception {
77+
PID pid = TestHelper.makePid();
78+
doThrow(new IOException(new EofException((String) null)))
79+
.when(fedoraContentService)
80+
.streamData(any(), any(), anyBoolean(), any());
81+
mvc.perform(get("/content/" + pid.getId()))
82+
.andExpect(status().isBadRequest())
83+
.andReturn();
84+
}
85+
86+
@Test
87+
public void getContentWrappedTimeoutExceptionTest() throws Exception {
88+
PID pid = TestHelper.makePid();
89+
doThrow(new IOException(new TimeoutException()))
90+
.when(fedoraContentService)
91+
.streamData(any(), any(), anyBoolean(), any());
92+
mvc.perform(get("/content/" + pid.getId()))
93+
.andExpect(status().isBadRequest())
94+
.andReturn();
95+
}
96+
97+
@Test
98+
public void getContentIOExceptionTest() throws Exception {
99+
PID pid = TestHelper.makePid();
100+
doThrow(new IOException())
101+
.when(fedoraContentService)
102+
.streamData(any(), any(), anyBoolean(), any());
103+
mvc.perform(get("/content/" + pid.getId()))
104+
.andExpect(status().isBadRequest())
105+
.andReturn();
106+
}
107+
}

0 commit comments

Comments
 (0)