Skip to content
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

Refactor copy headers mechanism in REST API #7594

Closed

Conversation

javanna
Copy link
Member

@javanna javanna commented Sep 4, 2014

The functionality of copying headers in the REST layer (from REST requests to transport requests) remains the same. Made it a bit nicer by introducing a ClientFactory component that is a singleton and allows to register useful headers without requiring static methods.

Plugins just have to inject the ClientFactory now, and call its addUsefulHeaders method that is not static anymore.

Relates to #6513

@javanna javanna self-assigned this Sep 4, 2014
@javanna javanna force-pushed the enhancement/rest_client_factory branch from 33c28e4 to e9f961e Compare September 4, 2014 16:51
* Returns a proper {@link Client client} given the provided {@link org.elasticsearch.rest.RestRequest}
*/
public Client client(RestRequest restRequest) {
return usefulHeaders.size() == 0 ? client : new HeadersCopyClient(client, restRequest, usefulHeaders);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use isEmpty()?

@s1monw
Copy link
Contributor

s1monw commented Sep 5, 2014

I left some minor comments

@s1monw s1monw removed the review label Sep 5, 2014
@clintongormley clintongormley changed the title REST API: refactor copy headers mechanism Internal: Refactor copy headers mechanism in REST API Sep 8, 2014
The functionality of copying headers in the REST layer (from REST requests to transport requests) remains the same. Made it a bit nicer by introducing a ClientFactory component that is a singleton and allows to register useful headers without requiring static methods.

Plugins just have to inject the ClientFactory now, and call its `addUsefulHeaders` method that is not static anymore.

Relates to elastic#6513
Closes elastic#7594
@javanna javanna force-pushed the enhancement/rest_client_factory branch from e9f961e to 2d399ef Compare September 9, 2014 16:22
@javanna javanna added the review label Sep 9, 2014
@javanna
Copy link
Member Author

javanna commented Sep 9, 2014

I pushed a new commit that addresses the naming feedback. I also rebased to adapt the code to #7610 that was pushed in the meantime.

@s1monw
Copy link
Contributor

s1monw commented Sep 10, 2014

LGTM

@s1monw s1monw removed the review label Sep 10, 2014
javanna added a commit that referenced this pull request Sep 10, 2014
The functionality of copying headers in the REST layer (from REST requests to transport requests) remains the same. Made it a bit nicer by introducing a RestClientFactory component that is a singleton and allows to register useful headers without requiring static methods.

Plugins just have to inject the RestClientFactory now, and call its `addRelevantHeaders` method that is not static anymore.

Relates to #6513
Closes #7594
@javanna javanna closed this in 5bea31c Sep 10, 2014
javanna added a commit to javanna/elasticsearch that referenced this pull request Sep 11, 2014
…tory

With elastic#7594 we replaced the static `BaseRestHandler#addUsefulHeaders` by introducing the `RestClientFactory` that can be injected and used to register the relevant headers. To simplify things, we can now register relevant headers through the `RestController` and remove the `RestClientFactory` that was just introduced.

Closes elastic#7675
javanna added a commit that referenced this pull request Sep 11, 2014
…tory

With #7594 we replaced the static `BaseRestHandler#addUsefulHeaders` by introducing the `RestClientFactory` that can be injected and used to register the relevant headers. To simplify things, we can now register relevant headers through the `RestController` and remove the `RestClientFactory` that was just introduced.

Closes #7675
javanna added a commit that referenced this pull request Sep 11, 2014
…tory

With #7594 we replaced the static `BaseRestHandler#addUsefulHeaders` by introducing the `RestClientFactory` that can be injected and used to register the relevant headers. To simplify things, we can now register relevant headers through the `RestController` and remove the `RestClientFactory` that was just introduced.

Closes #7675
@clintongormley clintongormley changed the title Internal: Refactor copy headers mechanism in REST API Refactor copy headers mechanism in REST API Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants