context-propagation icon indicating copy to clipboard operation
context-propagation copied to clipboard

Next major release should have Java 8 as minimum version

Open sjoerdtalsma opened this issue 6 years ago • 4 comments

Is your feature request related to a problem? Please describe.

Java 8 allows for cleaner code because:

  • lambda's
  • Optional
  • Functional interfaces; i.e. lose our SnapshotSupplier in favour of Supplier<Snapshot>.

Describe the solution you'd like

The next major release will be a breaking change, so use that chance for improvement.

  1. Set minimum java version to 1.8
  2. Include the -java8 module into the main version and remove it from the build
  3. Replace SnapshotSupplier by Supplier<Snapshot>.
  4. Make optional results return Optional
  5. Remove all deprecated code
  6. Change the main package so the code can co-exist with v1
  7. Add a compatibility module containing a 'bridge' to v1 ContextManager implementations
  8. Change the ContextManager: replace getActiveContext by Optional<T> getActiveContextValue (breaking change)

Describe alternatives you've considered

'Hard' breaking changes (leaving out the last two items from the list) but that may hurt current users unnecessarily. Two-step getActiveContext --> getActiveContextValue through deprecation plus introducing new method, unfortunately introducing a new method in an interface is only possible compatibly from Java8's default methods.

Additional context

sjoerdtalsma avatar Mar 14 '19 06:03 sjoerdtalsma

New API ideas:

Simplify / redefine concepts:

  • ThreadLocalContext / ThreadContextSnapshot -> snapshot of 1 or more ThreadLocal values
  • ThreadLocalAdapter<T> -> Implements a get / set API for <T> values

sjoerdtalsma avatar Mar 11 '20 11:03 sjoerdtalsma

Possible new module structure: ❓ ❓

  • threadcontext-core
  • threadcontext-metrics
  • threadcontext-micrometer
  • threadcontext-kotlin (todo?)

And supporting modules:

  • support/locale-threadcontext
  • support/servletrequest-threadcontext
  • support/opentracing-threadcontext
  • support/opentracing-threadcontext-adapter
  • support/mdc-threadcontext-adapter
  • support/spring-security-threadcontext-adapter

sjoerdtalsma avatar Mar 13 '20 16:03 sjoerdtalsma

drive by comment. internally probably good to stay imperative as there is measurable overhead doing things functionally especially if arbitrary vs imperative. benchmarks will show the additional allocations and higher runtime.

typically in context or otherwise lowest level code I write internals so they have no chance of allocating for things like iteration etc (via array fields vs lists etc). basically even if you support things that appear nice externally, take the punch internally to help apps remain efficient despite APIs like optional

codefromthecrypt avatar Apr 24 '20 01:04 codefromthecrypt

@adriancole Thanks for your comments, they are really appreciated! I fully agree with you here. Good to have a reminder to keep this in mind!

My main intention is to pull things apart a little further; the biggest negative feedback on this repository is that it is far too extensive for the intended purpose, while all support modules are of course fully optional.

I hope separating an absolute minimal API into a core module with proper documentation may help address these concerns, while 'hiding' all add-ons and custom implementations into a 'support' subdirectory.

sjoerdtalsma avatar Apr 27 '20 11:04 sjoerdtalsma