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

Deduplicate useful headers that get copied from REST to transport layer #7590

Closed

Conversation

Projects
None yet
4 participants
@javanna
Copy link
Member

javanna commented Sep 4, 2014

The useful headers are now stored into a Set instead of an array so we can easily deduplicate them. A set is also returned instead of an array by the usefulHeaders static getter.

@javanna javanna self-assigned this Sep 4, 2014

@javanna javanna force-pushed the javanna:enhancement/deduplicate_useful_headers branch Sep 4, 2014

/**
* Base handler for REST requests
*/
public abstract class BaseRestHandler extends AbstractComponent implements RestHandler {

// non volatile since the assumption is that useful headers are registered on startup
private static String[] usefulHeaders = new String[0];
private static Set<String> usefulHeaders = Sets.newCopyOnWriteArraySet();

This comment has been minimized.

Copy link
@uboness

uboness Sep 4, 2014

Contributor

it can be null by default and lazily initialized when headers are actually added

This comment has been minimized.

Copy link
@javanna

javanna Sep 4, 2014

Author Member

not sure I'd like having null here and the null check. It also means we have to add back the synchronized to the addUsefulHeaders to make sure the lazily init works and check again for null there.

This comment has been minimized.

Copy link
@uboness

uboness Sep 4, 2014

Contributor

yeah... fair point... ok... leave as is

@@ -63,7 +61,7 @@ protected BaseRestHandler(Settings settings, Client client) {

@Override
public final void handleRequest(RestRequest request, RestChannel channel) throws Exception {
handleRequest(request, channel, usefulHeaders.length == 0 ? client : new HeadersCopyClient(client, request, usefulHeaders));
handleRequest(request, channel, usefulHeaders.size() == 0 ? client : new HeadersCopyClient(client, request, usefulHeaders));

This comment has been minimized.

Copy link
@uboness

uboness Sep 4, 2014

Contributor

based on the comment above, a null check instead of size check

}
});
}

executorService.shutdown();
assertThat(executorService.awaitTermination(1, TimeUnit.SECONDS), equalTo(true));
String[] usefulHeaders = BaseRestHandler.usefulHeaders();
String[] usefulHeaders = BaseRestHandler.usefulHeaders().toArray(new String[BaseRestHandler.usefulHeaders().size()]);

This comment has been minimized.

Copy link
@uboness

uboness Sep 4, 2014

Contributor

wouldn't it be cleaner to just work with sets here? (instead of converting to arrays)

This comment has been minimized.

Copy link
@javanna

javanna Sep 4, 2014

Author Member

I need to sort the headers to compare them with the expected ones (sorted too). I either convert it to an array or to a list so I can use either Arrays.sort or Collections.sort. I think it's ok then to keep the array?

Internal: deduplicate useful headers that get copied from REST to tra…
…nsport requests

The useful headers are now stored into a `Set` instead of an array so we can easily deduplicate them. A set is also returned instead of an array by the `usefulHeaders` static getter.

Relates to #6513

Closes #7590

@javanna javanna force-pushed the javanna:enhancement/deduplicate_useful_headers branch to 904ab97 Sep 4, 2014

@spinscale

This comment has been minimized.

Copy link
Member

spinscale commented Sep 4, 2014

LGTM

1 similar comment
@uboness

This comment has been minimized.

Copy link
Contributor

uboness commented Sep 4, 2014

LGTM

javanna added a commit that referenced this pull request Sep 4, 2014

Internal: deduplicate useful headers that get copied from REST to tra…
…nsport requests

The useful headers are now stored into a `Set` instead of an array so we can easily deduplicate them. A set is also returned instead of an array by the `usefulHeaders` static getter.

Relates to #6513

Closes #7590

@javanna javanna closed this in 6633221 Sep 4, 2014

@javanna javanna removed the review label Sep 4, 2014

javanna added a commit that referenced this pull request Sep 8, 2014

Internal: deduplicate useful headers that get copied from REST to tra…
…nsport requests

The useful headers are now stored into a `Set` instead of an array so we can easily deduplicate them. A set is also returned instead of an array by the `usefulHeaders` static getter.

Relates to #6513

Closes #7590

@clintongormley clintongormley changed the title Internal: deduplicate useful headers that get copied from REST to transp... Internal: Deduplicate useful headers that get copied from REST to transport layer Sep 8, 2014

@clintongormley clintongormley changed the title Internal: Deduplicate useful headers that get copied from REST to transport layer Deduplicate useful headers that get copied from REST to transport layer Jun 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.