use thread local (optionally) for context setting

[pjfanning]

Relates to #89

If this approach of allowing the use of thread specific configuration is ok, I can add extra test coverage and tidy up the code.

My use case is my application is multi-tenanted and different threads might be handling data for different customers with different intercom API keys.

Using thread contexts has its limitations but it requires fewer code changes than the idea of having per-object configuration.

[thewheat]

Thanks for this WIP PR @pjfanning. I'll bring this to discussion with our team to get some input on this and your approach 👍

[thewheat]

@pjfanning got confirmation from the team that this approach is good so do continue with some tests 👍

[pjfanning]

@thewheat does the IntercomTest class look ok? It's not very pretty, especially since I'm restricted from using the Java 8 enhancements for Futures and executor services.

[thewheat]

@pjfanning just back after a long weekend holiday here. I've not dealt with threads in Java before so will need to dig into this a bit with the team. Were there any other questions that you could have by following this current approach? May be easier to get all questions that we can discuss to prevent delays 👍

[pjfanning]

@thewheat I guess this approach has the advantage of limiting the API change but in the end I think that @xpg 's https://github.com/xgp/intercom-java approach is a better bet.
I wonder whether we could keep the existing API and extend it also allow overloaded methods that take an Intercom context instance. This allows existing user's to continue to use the static context by calling the Intercom class static methods but that users who need to manage different Intercom contexts (like different multiple Intercom tokens) could use the new methods that take a context instance in the API.

[thewheat]

I do kind of agree that being able to pass in an extra "authentication" object is nice and I don't think we'd be opposed to a breaking change so long as it is a step in the right direction. But having no breaking changes but still being able to provide new functionally is even better 😄

Just so you know, I'm still waiting on feedback from the team regarding the current approach but will update as soon as possible 👍

[mmartinic]

Hi @pjfanning, sorry for slow reply! I think this is a good approach, it should be quite useful.
I was thinking we could also have a method for cleanup that removes current thread local (if it’s being used). Something like cleanupThreadLocalContext. What do you think?

[pjfanning]

@mmartinic I added a clearThreadLocalContexts method. I prefer the verb clear because it's commonly used in collection APIs. I used Contexts because the method clears all the contexts for all threads.

[mmartinic]

@pjfanning thanks! sorry, I wasn't very clear. I was more thinking it would be useful to have something that clears thread local context for current thread, not all of them.
something like

public static void clearThreadLocalContext() {
  threadContext.remove();
}

this can then be used to clear the context in current thread before setting new values. Idea is to prevent value left over from previous request that was using that same thread.

[pjfanning]

Added extra clearThreadLocalContext method.

[mmartinic]

@pjfanning this is great! thanks for adding it!