Hi Everyone,
Patch updated so that the code doesn't swallow exceptions unnecessarily and
I've added a few extra tests to cover the new behaviour of the URI generation
in the presence of optional query strings.
The patch was generated against the 2.4.0 source release tarball,
Joel.
--
Dr Joel Wright
IT Innovation Centre
Gamma House
Enterprise Road
Southampton
SO16 7NS, UK
Tel.:+44 23 8059 8866
mailto:[email protected]
http://www.it-innovation.soton.ac.uk/
________________________________
From: Stian Soiland-Reyes [[email protected]]
Sent: 22 May 2012 10:23
To: List for general discussion and hacking of the Taverna project
Subject: Re: [Taverna-hackers] REST Services and Optional Query String
Parameters
On Mon, May 21, 2012 at 2:20 PM, Joel Wright
<[email protected]<mailto:[email protected]>> wrote:
The patch is undoubtedly not the best method for adding the functionality I was
after (I feel embarrassed even sending a patch that catches and ignores
exceptions), but given the structure of the code it was certainly the
lightest-touch I could come up with.
Thanks for your patch. Unfortunately the code still needs a bit of work.
You should not eat any exception from the reference service. This could hide
lots of different errors, such as it being a binary, and inadvertently make a
slightly different REST call that the user did not expect.
Rather, to see if an input is provided, you should check if the "inputs" map
has the inputName key, and just do a continue if not. (or iterate over the
inputs map, and check if it is an expected activity input).
This would need a good set of unit tests so that we can verify that this works
as intended, and only where intended. Tests should cover edge-cases like
http://example.com/?a=1&b=/fred/{soup}&c=3 and
http://example.com/?{soup}={value}
As the old behaviour was failure on any missing parameter, I don't think this
patch will be breaking any backward comparability.
--
Stian Soiland-Reyes, myGrid team
School of Computer Science
The University of Manchester
diff -crB taverna-workbench-2.4.0/engine/net.sf.taverna.t2.activities/rest-activity/src/main/java/net/sf/taverna/t2/activities/rest/RESTActivity.java taverna-workbench-src-2.4.0-jjw/engine/net.sf.taverna.t2.activities/rest-activity/src/main/java/net/sf/taverna/t2/activities/rest/RESTActivity.java
*** taverna-workbench-2.4.0/engine/net.sf.taverna.t2.activities/rest-activity/src/main/java/net/sf/taverna/t2/activities/rest/RESTActivity.java 2012-03-20 14:35:02.000000000 +0000
--- taverna-workbench-src-2.4.0-jjw/engine/net.sf.taverna.t2.activities/rest-activity/src/main/java/net/sf/taverna/t2/activities/rest/RESTActivity.java 2012-06-18 11:53:08.994666820 +0100
***************
*** 208,214 ****
/**
* This method executes pre-configured instance of REST activity. It
* resolves inputs of the activity and registers its outputs; the real
! * invocation of the HTTP request is perfomed by
* {@link HTTPRequestHandler#initiateHTTPRequest(String, RESTActivityConfigurationBean, String)}
* .
*/
--- 208,214 ----
/**
* This method executes pre-configured instance of REST activity. It
* resolves inputs of the activity and registers its outputs; the real
! * invocation of the HTTP request is performed by
* {@link HTTPRequestHandler#initiateHTTPRequest(String, RESTActivityConfigurationBean, String)}
* .
*/
***************
*** 230,253 ****
// (just use the configuration that was determined in
// configurePorts() - all ports in this set are required)
Map<String, String> urlParameters = new HashMap<String, String>();
! try {
! for (String inputName : configBean.getActivityInputs()
! .keySet()) {
! urlParameters.put(inputName, (String) referenceService
! .renderIdentifier(inputs.get(inputName),
! configBean.getActivityInputs().get(
! inputName), context));
! }
! } catch (Exception e) {
! // problem occurred while resolving the inputs
! callback
! .fail(
! "REST activity was unable to resolve all necessary inputs"
! + "that contain values for populating the URI signature placeholders "
! + "with values.", e);
! // make sure we don't call callback.receiveResult later
! return;
}
String completeURL = URISignatureHandler.generateCompleteURI(
configBean.getUrlSignature(), urlParameters, configBean
--- 230,252 ----
// (just use the configuration that was determined in
// configurePorts() - all ports in this set are required)
Map<String, String> urlParameters = new HashMap<String, String>();
! for (String inputName : configBean.getActivityInputs().keySet()) {
! try {
! if (inputs.containsKey(inputName)) {
! urlParameters.put(inputName, (String) referenceService
! .renderIdentifier(inputs.get(inputName),
! configBean.getActivityInputs().get(inputName), context));
! }
! } catch (Exception e) {
! // problem occurred while resolving the inputs
! callback
! .fail(
! "REST activity was unable to resolve all necessary inputs"
! + "that contain values for populating the URI signature placeholders.", e);
! // make sure we don't call callback.receiveResult later
! return;
! }
}
String completeURL = URISignatureHandler.generateCompleteURI(
configBean.getUrlSignature(), urlParameters, configBean
diff -crB taverna-workbench-2.4.0/engine/net.sf.taverna.t2.activities/rest-activity/src/main/java/net/sf/taverna/t2/activities/rest/URISignatureHandler.java taverna-workbench-src-2.4.0-jjw/engine/net.sf.taverna.t2.activities/rest-activity/src/main/java/net/sf/taverna/t2/activities/rest/URISignatureHandler.java
*** taverna-workbench-2.4.0/engine/net.sf.taverna.t2.activities/rest-activity/src/main/java/net/sf/taverna/t2/activities/rest/URISignatureHandler.java 2011-06-02 12:28:20.000000000 +0100
--- taverna-workbench-src-2.4.0-jjw/engine/net.sf.taverna.t2.activities/rest-activity/src/main/java/net/sf/taverna/t2/activities/rest/URISignatureHandler.java 2012-06-18 11:50:11.626661533 +0100
***************
*** 263,269 ****
* the placeholders found in <code>uriSignature</code>.
*/
public static String generateCompleteURI(String uriSignature,
! Map<String, String> parameters, boolean escapeParameters)
throws URISignatureParsingException,
URIGenerationFromSignatureException {
StringBuilder completeURI = new StringBuilder(uriSignature);
--- 263,269 ----
* the placeholders found in <code>uriSignature</code>.
*/
public static String generateCompleteURI(String uriSignature,
! Map<String, String> parameters, boolean escapeParameters)
throws URISignatureParsingException,
URIGenerationFromSignatureException {
StringBuilder completeURI = new StringBuilder(uriSignature);
***************
*** 298,320 ****
while (placeholdersIterator.hasNext()) {
String placeholder = placeholdersIterator.next();
if (parameters.containsKey(placeholder)) {
- int placeholderStartPos = placeholdersWithPositions
- .get(placeholder) - 1;
- int placeholderEndPos = placeholderStartPos
- + placeholder.length() + 2;
if (escapeParameters) {
completeURI.replace(placeholderStartPos,
placeholderEndPos, urlEncodeQuery(parameters
! .get(placeholder)));
} else {
completeURI.replace(placeholderStartPos,
placeholderEndPos, parameters.get(placeholder));
}
} else {
! throw new URIGenerationFromSignatureException(
! "Parameter map does not contain a key/value for \""
! + placeholder + "\" placeholder");
}
}
}
--- 298,341 ----
while (placeholdersIterator.hasNext()) {
String placeholder = placeholdersIterator.next();
+ int placeholderStartPos = placeholdersWithPositions
+ .get(placeholder) - 1;
+ int placeholderEndPos = placeholderStartPos
+ + placeholder.length() + 2;
if (parameters.containsKey(placeholder)) {
if (escapeParameters) {
completeURI.replace(placeholderStartPos,
placeholderEndPos, urlEncodeQuery(parameters
! .get(placeholder)));
} else {
completeURI.replace(placeholderStartPos,
placeholderEndPos, parameters.get(placeholder));
}
} else {
! int qnPos = completeURI.lastIndexOf("?", placeholderStartPos);
! int ampPos = completeURI.lastIndexOf("&", placeholderStartPos);
! int slashPos = completeURI.lastIndexOf("/", placeholderStartPos);
! int startParamPos = Math.max(qnPos, ampPos);
! if (startParamPos > -1 && startParamPos > slashPos) {
! // We found an optional parameter, so delete all of it
! if (qnPos > ampPos) {
! // It might be the first or only parameter so delete carefully
! if (placeholderEndPos >= (completeURI.length() - 1)) {
! // No parameters
! completeURI.replace(startParamPos, placeholderEndPos, "");
! } else {
! // Remove the & from the following parameter, not the ? that starts
! completeURI.replace(startParamPos + 1, placeholderEndPos + 1, "");
! }
! } else {
! // Just delete the optional parameter in total
! completeURI.replace(startParamPos, placeholderEndPos, "");
! }
! } else {
! throw new URIGenerationFromSignatureException(
! "Parameter map does not contain a key/value for \""
! + placeholder + "\" mandatory placeholder");
! }
}
}
}
diff -crB taverna-workbench-2.4.0/engine/net.sf.taverna.t2.activities/rest-activity/src/test/java/net/sf/taverna/t2/activities/rest/URISignatureHandlerTest.java taverna-workbench-src-2.4.0-jjw/engine/net.sf.taverna.t2.activities/rest-activity/src/test/java/net/sf/taverna/t2/activities/rest/URISignatureHandlerTest.java
*** taverna-workbench-2.4.0/engine/net.sf.taverna.t2.activities/rest-activity/src/test/java/net/sf/taverna/t2/activities/rest/URISignatureHandlerTest.java 2011-06-01 15:52:25.000000000 +0100
--- taverna-workbench-src-2.4.0-jjw/engine/net.sf.taverna.t2.activities/rest-activity/src/test/java/net/sf/taverna/t2/activities/rest/URISignatureHandlerTest.java 2012-06-18 11:51:50.326664474 +0100
***************
*** 33,38 ****
--- 33,42 ----
final String badURI_DuplicatePlaceholders = "http://sandbox.myexperiment.org/user.xml?id={user_id}&verbose={user_id}";;
final String badURI_DuplicatePlaceholdersWithOthers = "http://sysmo-db.org/sops/{unit}/experimental_conditions/{cond_id}?condition_unit={unit}";;
+ //final String validURI_MultipleOptions = "http://sysmo-db.org/sops/{unit}/experimental_conditions/{cond_id}?condition_unit={unit}";;
+ final String validURI_MultipleQueryString =
+ "http://dr-site.esrin.esa.int/{catalogue}/genesi/ASA_IMS_1P/rdf/?count={count?}&startPage={startPage?}&startIndex={startIndex?}&q={searchTerms?}";;
+
// ==========================================================================
// TEST URI SIGNATURE BOOLEAN VALIDATION
// ==========================================================================
***************
*** 61,66 ****
--- 65,75 ----
assertTrue(URISignatureHandler.isValid(validURI_3MixedPlaceholders));
}
+ @Test
+ public void isValid_validURI_MultipleQueryString() {
+ assertTrue(URISignatureHandler.isValid(validURI_MultipleQueryString));
+ }
+
// failure cases
@Test
***************
*** 165,170 ****
--- 174,184 ----
URISignatureHandler.validate(validURI_3MixedPlaceholders);
}
+ @Test
+ public void validate_validURI_validURI_Multiple() {
+ URISignatureHandler.validate(validURI_MultipleQueryString);
+ }
+
// failure cases
@Test(expected = URISignatureHandler.URISignatureParsingException.class)
***************
*** 278,283 ****
--- 292,310 ----
assertEquals("Wrong third placeholder", "unit", placeholders.get(2));
}
+ @Test
+ public void extractPlaceholders_validURI_MultipleQueryString() {
+ List<String> placeholders = URISignatureHandler
+ .extractPlaceholders(validURI_MultipleQueryString);
+ assertNotNull(placeholders);
+ assertEquals(5, placeholders.size());
+ assertEquals("Wrong first placeholder", "catalogue", placeholders.get(0));
+ assertEquals("Wrong second placeholder", "count?", placeholders.get(1));
+ assertEquals("Wrong third placeholder", "startPage?", placeholders.get(2));
+ assertEquals("Wrong fourth placeholder", "startIndex?", placeholders.get(3));
+ assertEquals("Wrong fifth placeholder", "searchTerms?", placeholders.get(4));
+ }
+
// failure cases
/*
***************
*** 421,426 ****
--- 448,503 ----
completeURI);
}
+ @SuppressWarnings("serial")
+ @Test
+ public void generateCompleteURI_successfulURIGeneration_optionalParams() {
+ String uriSignature = "http://dr-site.esrin.esa.int/{catalogue}/genesi/ASA_IMS_1P/rdf/?count={count?}&startPage={startPage?}&startIndex={startIndex?}&q={searchTerms?}";;
+ Map<String, String> allParameters = new HashMap<String, String>() {
+ {
+ put("catalogue", "catalogue");
+ put("count?", "10");
+ put("startPage?", "1");
+ put("startIndex?", "1");
+ put("searchTerms?", "term1");
+ }
+ };
+
+ Map<String, String> parametersMissingOptional = new HashMap<String, String>() {
+ {
+ put("catalogue", "catalogue");
+ put("count?", "10");
+ put("searchTerms?", "term1");
+ }
+ };
+
+ Map<String, String> parametersMissingFirstOptional = new HashMap<String, String>() {
+ {
+ put("catalogue", "catalogue");
+ put("startPage?", "1");
+ put("startIndex?", "1");
+ put("searchTerms?", "term1");
+ }
+ };
+
+ String completeURI1 = URISignatureHandler.generateCompleteURI(
+ uriSignature, allParameters, false);
+ assertEquals(
+ "http://dr-site.esrin.esa.int/catalogue/genesi/ASA_IMS_1P/rdf/?count=10&startPage=1&startIndex=1&q=term1";,
+ completeURI1);
+
+ String completeURI2 = URISignatureHandler.generateCompleteURI(
+ uriSignature, parametersMissingOptional, false);
+ assertEquals(
+ "http://dr-site.esrin.esa.int/catalogue/genesi/ASA_IMS_1P/rdf/?count=10&q=term1";,
+ completeURI2);
+
+ String completeURI3 = URISignatureHandler.generateCompleteURI(
+ uriSignature, parametersMissingFirstOptional, false);
+ assertEquals(
+ "http://dr-site.esrin.esa.int/catalogue/genesi/ASA_IMS_1P/rdf/?startPage=1&startIndex=1&q=term1";,
+ completeURI3);
+ }
+
@Test
public void generateCompleteURI_signatureWithNoPlaceholders_nullParameterMap() {
String completeURI = URISignatureHandler.generateCompleteURI(
***************
*** 451,457 ****
}
@SuppressWarnings("serial")
! @Test(expected = URISignatureHandler.URIGenerationFromSignatureException.class)
public void generateCompleteURI_signatureWithPlaceholders_missingParameterURIGeneration_FailureExpected() {
String uriSignature = "http://sysmo-db.org/sops/{sop_id}/experimental_conditions/{cond_id}?condition_unit={unit}";;
Map<String, String> parameters = new HashMap<String, String>() {
--- 528,534 ----
}
@SuppressWarnings("serial")
! @Test
public void generateCompleteURI_signatureWithPlaceholders_missingParameterURIGeneration_FailureExpected() {
String uriSignature = "http://sysmo-db.org/sops/{sop_id}/experimental_conditions/{cond_id}?condition_unit={unit}";;
Map<String, String> parameters = new HashMap<String, String>() {
***************
*** 465,471 ****
uriSignature, parameters, true);
assertEquals(
! "http://sysmo-db.org/sops/111/experimental_conditions/2222?condition_unit=33";,
completeURI);
}
--- 542,548 ----
uriSignature, parameters, true);
assertEquals(
! "http://sysmo-db.org/sops/111/experimental_conditions/2222";,
completeURI);
}
***************
*** 494,497 ****
--- 571,595 ----
completeURI);
}
+ @SuppressWarnings("serial")
+ @Test(expected = URISignatureHandler.URIGenerationFromSignatureException.class)
+ public void generateCompleteURI_failureURIGeneration_optionalParams() {
+ String uriSignature = "http://dr-site.esrin.esa.int/{catalogue}/genesi/ASA_IMS_1P/rdf/?count={count?}&startPage={startPage?}&startIndex={startIndex?}&q={searchTerms?}";;
+
+ Map<String, String> parametersMissingCompulsory = new HashMap<String, String>() {
+ {
+ put("count?", "10");
+ put("startPage?", "1");
+ put("startIndex?", "1");
+ put("searchTerms?", "term1");
+ }
+ };
+
+ String completeURI = URISignatureHandler.generateCompleteURI(
+ uriSignature, parametersMissingCompulsory, false);
+
+ assertEquals(
+ "http://dr-site.esrin.esa.int/catalogue/genesi/ASA_IMS_1P/rdf/?count={count?}&startPage={startPage?}&startIndex={startIndex?}&q={searchTerms?}";,
+ completeURI);
+ }
}
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
taverna-hackers mailing list
[email protected]
Web site: http://www.taverna.org.uk
Mailing lists: http://www.taverna.org.uk/about/contact-us/
Developers Guide: http://www.taverna.org.uk/developers/
