New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
added support for prefer headers along with unit and integration tests #31
Conversation
@@ -26,6 +26,8 @@ | |||
|
|||
public static final String FCREPO_TRANSFORM = "FCREPO_TRANSFORM"; | |||
|
|||
public static final String HTTP_PREFER = "CamelHttpPrefer"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there rationale for straying from the ALL_CAPS convention established by the previous three FCREPO headers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sort of. I began using ALL_CAPS because I had seen that elsewhere, but other camel components seem to use the CamelComponentHeader
convention, especially the HTTP headers. There is a common pattern to remove all HTTP headers with a line like
removeHeaders("CamelHttp*")
By following that convention, the CamelHttpPrefer
header fits more easily into existing patterns.
Really, though, the other headers should be changed to:
CamelFcrepoTransform
CamelFcrepoBaseUrl
CamelFcrepoIdentifier
I would recommend changing those headers, too. The main question, though, is whether CamelHttpPrefer
shouldn't instead by CamelFcrepoPrefer
. And the more I think about it, the more I think it should be CamelFcrepoPrefer
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good.
As a general comment, more logging will make this codebase easier to trace and debug. |
@@ -29,6 +33,20 @@ | |||
|
|||
public static final String LDP = "http://www.w3.org/ns/ldp#"; | |||
|
|||
public static final String SERVER_MANAGED = REPOSITORY + "ServerManaged"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of the below properties are used outside of tests.
Do we need to keep the additions to this class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. It's possible that these String
values would be used in running code, but I doubt that. The Map<String, String>
is used in String FcrepoProducer.addPreferNamespace(String)
.
Changes include: * Change visibility of certain protected methods to private in FcrepoProducer * Change format of header values to follow Camel conventions (e.g. CamelFcrepoIdentifier) * Remove unused constants * Updated the documentation related to these changes (and fixed an incorrect link)
Resolved with: 3ad67c8 |
This addresses https://jira.duraspace.org/browse/FCREPO-1273
Specifically, it allows users to control the Prefer headers sent to the fedora endpoint