Allow override of JMS baseUrl ... #774
Conversation
… with format host:port Added Unit test
| if (propBaseURL.length() > 0) { | ||
| if (propBaseURL.indexOf(':') > -1) { | ||
| final String[] parts = propBaseURL.split(":"); | ||
| return uriInfo.getBaseUriBuilder().clone().host(parts[0]).port(Integer.valueOf(parts[1])).toString(); |
There was a problem hiding this comment.
Isn't parts[0] going to be "http" here?
There was a problem hiding this comment.
Right, but not the hostname, which I would expect to be in parts[1]
There was a problem hiding this comment.
Ok, I see what happened there. My expectation was host is host, and not protocol. But that could definitely depend on interpretation. I'll put in a check.
There was a problem hiding this comment.
Before I do this, is it your expectation that if I am running Fcrepo at "http://blah.com:8080" I might want the JMS messages to reference "https://blah.com:8443"? I couldn't think of why you would, but now I'm thinking it might be easier to pass the whole property to UriBuilder.uri(String s)?
| } else { | ||
| return uriInfo.getBaseUriBuilder().clone().scheme(parts[0]).host(parts[1].substring(2)).toString(); | ||
| } | ||
| } |
There was a problem hiding this comment.
What is the point of breaking up this URI and then reassembling it?
There was a problem hiding this comment.
IMHO, we should return the fcrepo.jms.baseUrl as-is without parsing and rebuilding. The use case for this is users wanting to override the default generated baseURL, and I would expect it to be very frustrating if the value was altered in any way.
There was a problem hiding this comment.
Honestly at this point, I'm not sure there is any point. I can replace it all with:
return uriInfo.getBaseUriBuilder().clone().uri(propBaseURL).toString();
| protected String getBaseUrlProperty() { | ||
| final String propBaseURL = System.getProperty("fcrepo.jms.baseUrl", ""); | ||
| if (propBaseURL.length() > 0 && propBaseURL.startsWith("http")) { | ||
| return uriInfo.getBaseUriBuilder().clone().uri(propBaseURL).toString(); |
There was a problem hiding this comment.
Why the invocation of clone()?
There was a problem hiding this comment.
Possibly a mistaken understanding of the UriBuilder, but my assumption from the Javadocs and the usage in IdentiferConverter was to preserve the original state. Perhaps its not needed?
|
Resolved with: 8779cf0 |
...with system property fcrepo.jms.baseUrl with format host:port
Added Unit test
Addresses FCREPO-1259