Skip to content

Commit b7afeca

Browse files
loitlyloitly
authored
Merge pull request #907 from Caltech-IPAC/FIREFLY-414_async_tap_redirect
FIREFLY-414: Fail to retrieve results from LSST async TAP queries
2 parents 8ec7226 + 40eaa9f commit b7afeca

File tree

4 files changed

+89
-37
lines changed

4 files changed

+89
-37
lines changed

‎src/firefly/java/edu/caltech/ipac/firefly/server/network/HttpServiceInput.java‎

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ public class HttpServiceInput implements Cloneable{
2828
private Map<String, File> files;
2929
private String userId;
3030
private String passwd;
31+
private boolean followRedirect = true;
3132

3233
public HttpServiceInput() {}
3334

@@ -38,7 +39,6 @@ public HttpServiceInput(String requestUrl) {
3839
public String getRequestUrl() {
3940
return requestUrl;
4041
}
41-
4242
public HttpServiceInput setRequestUrl(String requestUrl) {
4343
this.requestUrl = requestUrl;
4444
return this;
@@ -47,66 +47,65 @@ public HttpServiceInput setRequestUrl(String requestUrl) {
4747
public String getUserId() {
4848
return userId;
4949
}
50-
51-
public String getPasswd() {
52-
return passwd;
53-
}
54-
55-
public Map<String, String> getParams() {
56-
return params;
57-
}
58-
59-
public Map<String, String> getHeaders() {
60-
return headers;
61-
}
62-
63-
public Map<String, String> getCookies() {
64-
return cookies;
65-
}
66-
67-
public Map<String, File> getFiles() {
68-
return files;
69-
}
70-
71-
7250
public HttpServiceInput setUserId(String userId) {
7351
this.userId = userId;
7452
return this;
7553
}
7654

55+
public String getPasswd() {
56+
return passwd;
57+
}
7758
public HttpServiceInput setPasswd(String passwd) {
7859
this.passwd = passwd;
7960
return this;
8061
}
8162

63+
public Map<String, String> getParams() {
64+
return params;
65+
}
8266
public HttpServiceInput setParam(String key, String value) {
8367
if (StringUtils.isEmpty(key)) return this;
8468
if (params == null) params = new HashMap<>();
8569
params.put(key, value);
8670
return this;
8771
}
8872

73+
public Map<String, String> getHeaders() {
74+
return headers;
75+
}
8976
public HttpServiceInput setHeader(String key, String value) {
9077
if (StringUtils.isEmpty(key)) return this;
9178
if (headers == null) headers = new HashMap<>();
9279
headers.put(key, value);
9380
return this;
9481
}
9582

83+
public Map<String, String> getCookies() {
84+
return cookies;
85+
}
9686
public HttpServiceInput setCookie(String key, String value) {
9787
if (StringUtils.isEmpty(key)) return this;
9888
if (cookies == null) cookies = new HashMap<>();
9989
cookies.put(key, value);
10090
return this;
10191
}
10292

93+
public Map<String, File> getFiles() {
94+
return files;
95+
}
10396
public HttpServiceInput setFile(String key, File value) {
10497
if (StringUtils.isEmpty(key)) return this;
10598
if (files == null) files = new HashMap<>();
10699
files.put(key, value);
107100
return this;
108101
}
109102

103+
public boolean isFollowRedirect() { return followRedirect; }
104+
public HttpServiceInput setFollowRedirect(boolean followRedirect) {
105+
this.followRedirect = followRedirect;
106+
return this;
107+
}
108+
110109
public String getDesc() {
111110
StringBuilder sb = new StringBuilder();
112111

‎src/firefly/java/edu/caltech/ipac/firefly/server/network/HttpServices.java‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import org.apache.commons.httpclient.HttpClient;
1212
import org.apache.commons.httpclient.HttpMethod;
1313
import org.apache.commons.httpclient.NameValuePair;
14-
import org.apache.commons.httpclient.URIException;
1514
import org.apache.commons.httpclient.UsernamePasswordCredentials;
1615
import org.apache.commons.httpclient.auth.AuthScope;
1716
import org.apache.commons.httpclient.methods.GetMethod;
@@ -191,7 +190,7 @@ public static HttpMethod executeMethod(HttpMethod method, HttpServiceInput input
191190
method.setRequestHeader("User-Agent", USER_AGENT);
192191
method.setRequestHeader(HttpHeaders.ACCEPT_ENCODING, "gzip");
193192
if (method instanceof GetMethod) {
194-
method.setFollowRedirects(true); // post are not allowed to follow redirect
193+
method.setFollowRedirects(input.isFollowRedirect()); // post are not allowed to follow redirect
195194
}
196195
HttpClient httpClient = newHttpClient();
197196

@@ -204,7 +203,7 @@ public static HttpMethod executeMethod(HttpMethod method, HttpServiceInput input
204203
handleParams(method, input.getParams(), input.getFiles());
205204

206205
int status = httpClient.executeMethod(method);
207-
if (status < 200 || status >= 300) {
206+
if ( ( !isOk(method) || (isRedirected(method) && input.isFollowRedirect())) ) {
208207
// logs bad requests
209208
LOG.error("HTTP request failed with status:" + status + "\n" + getDetailDesc(method, input));
210209
}
@@ -319,6 +318,28 @@ public void handleResponse(HttpMethod method) {
319318
}
320319
}
321320

321+
public static boolean isOk(HttpMethod method) {
322+
int status = method.getStatusCode();
323+
return status >= 200 && status < 300;
324+
}
325+
326+
public static boolean isRedirected(HttpMethod method) {
327+
int status = method.getStatusCode();
328+
return status >= 300 && status < 400;
329+
}
330+
331+
public static String getReqHeader(HttpMethod method, String key, String def) {
332+
Header header = method.getRequestHeader(key);
333+
if (header == null || header.getValue() == null) return def;
334+
return header.getValue().trim();
335+
}
336+
337+
public static String getResHeader(HttpMethod method, String key, String def) {
338+
Header header = method.getResponseHeader(key);
339+
if (header == null || header.getValue() == null) return def;
340+
return header.getValue().trim();
341+
}
342+
322343
//====================================================================
323344
// Private functions
324345
//====================================================================

‎src/firefly/java/edu/caltech/ipac/firefly/server/query/AsyncTapQuery.java‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@
1515
import edu.caltech.ipac.util.AppProperties;
1616
import edu.caltech.ipac.util.FileUtil;
1717
import edu.caltech.ipac.util.StringUtils;
18-
import org.apache.commons.httpclient.Header;
1918

2019
import java.io.*;
2120

21+
2222
@SearchProcessorImpl(id = AsyncTapQuery.ID, params = {
2323
@ParamDoc(name = "serviceUrl", desc = "base TAP url endpoint excluding '/async'"),
2424
@ParamDoc(name = "QUERY", desc = "query string"),
@@ -54,10 +54,7 @@ public AsyncJob submitRequest(ServerRequest request) throws DataAccessException
5454

5555
Ref<String> location = new Ref<>();
5656
HttpServices.postData(inputs, (method -> {
57-
Header loc = method.getResponseHeader("Location");
58-
if (loc != null) {
59-
location.setSource(loc.getValue().trim());
60-
}
57+
location.setSource(HttpServices.getResHeader(method, "Location", null));
6158
}));
6259

6360
if (location.getSource() == null) {
@@ -95,7 +92,27 @@ public DataGroup getDataGroup() throws DataAccessException {
9592
//download file first: failing to parse gaia results with topcat SAX parser from url
9693
String filename = getFilename(baseJobUrl);
9794
File outFile = File.createTempFile(filename, ".vot", ServerContext.getTempWorkDir());
98-
HttpServices.getData(HttpServiceInput.createWithCredential(baseJobUrl + "/results/result"), outFile);
95+
HttpServiceInput input = HttpServiceInput.createWithCredential(baseJobUrl + "/results/result")
96+
.setFollowRedirect(false);
97+
HttpServices.getData(input, (method -> {
98+
try {
99+
if(HttpServices.isOk(method)) {
100+
HttpServices.defaultHandler(outFile).handleResponse(method);
101+
} else if (HttpServices.isRedirected(method)) {
102+
String location = HttpServices.getResHeader(method, "Location", null);
103+
if (location != null) {
104+
HttpServices.getData(HttpServiceInput.createWithCredential(location), outFile);
105+
} else {
106+
throw new RuntimeException("Request redirected without a location header");
107+
}
108+
} else {
109+
throw new RuntimeException("Request failed with status:" + method.getStatusText());
110+
}
111+
} catch (Exception e) {
112+
throw new RuntimeException(e.getMessage());
113+
}
114+
}));
115+
99116
DataGroup[] results = VoTableReader.voToDataGroups(outFile.getAbsolutePath());
100117
if (results.length > 0) {
101118
DataGroup dg = results[0];
@@ -143,8 +160,8 @@ public String getErrorMsg() {
143160
HttpServices.Status status = HttpServices.getData(HttpServiceInput.createWithCredential(errorUrl),
144161
(method -> {
145162
boolean isText = false;
146-
Header contentType = method.getResponseHeader("Content-Type");
147-
if (contentType != null && contentType.getValue().startsWith("text/plain")) {
163+
String contentType = HttpServices.getResHeader(method, "Content-Type", "");
164+
if (contentType.startsWith("text/plain")) {
148165
isText = true;
149166
}
150167
try {

‎src/firefly/test/edu/caltech/ipac/firefly/server/network/HttpServicesTest.java‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@
1818
import java.util.HashMap;
1919

2020
import static junit.framework.TestCase.assertNotNull;
21-
import static org.junit.Assert.assertEquals;
22-
import static org.junit.Assert.assertFalse;
23-
import static org.junit.Assert.fail;
21+
import static org.junit.Assert.*;
2422

2523
public class HttpServicesTest {
2624

@@ -121,6 +119,23 @@ public void testRedirectData(){
121119
assertEquals("url should be redirected to /get now", GET_URL, getProp(results.toString(), "url"));
122120
}
123121

122+
@Test
123+
public void testFollowRedirect(){
124+
HttpServiceInput nInput = input.setRequestUrl(REDIRECT_URL)
125+
.setParam("url", "http://www.acme.org")
126+
.setParam("status_code", "301");
127+
128+
HttpServices.getData(nInput, (method -> {
129+
assertFalse(HttpServices.isRedirected(method));
130+
}));
131+
132+
HttpServices.getData(nInput.setFollowRedirect(false), (method -> {
133+
assertTrue(HttpServices.isRedirected(method));
134+
assertEquals("redirect to www.acme.org", method.getResponseHeader("location").getValue(), "http://www.acme.org");
135+
}));
136+
}
137+
138+
124139
//====================================================================
125140
// private
126141
//====================================================================

0 commit comments

Comments
 (0)