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

Change plugin REST resource injection to use Class instances #2492

Merged
merged 2 commits into from Jul 26, 2016

Conversation

Projects
None yet
3 participants
@bernd
Member

bernd commented Jul 16, 2016

Using the actual resource instances caused injection of all the dependencies, even when we don't need the actual instance at all. The registration simply needs to have the class. This also prevents early injection of objects which have a lifecycle but don't manage it. (this continues to be a problem, but cannot be solved on this level)

@bernd bernd added this to the 2.1.0 milestone Jul 16, 2016

kroepke and others added some commits Jul 15, 2016

change plugin rest resource injection to use Class instances
using the actual resource instances caused injection of all the dependencies, even when we don't need the actual instance at all
the registration simply needs to have the class. this also prevents early injection of objects which have a lifecycle but don't manage it
(this continues to be a problem, but cannot be solved on this level)
Make sure there is a binding for the plugin rest resource classes
Fixes a binding error when running without any plugins.

@bernd bernd force-pushed the fix-plugin-resource-injection branch from 03052f8 to 738b059 Jul 26, 2016

@kroepke

This comment has been minimized.

Member

kroepke commented Jul 26, 2016

lgtm, good catch with missing plugins!

@bernd

This comment has been minimized.

Member

bernd commented Jul 26, 2016

lgtm, good catch with missing plugins!

That was actually catched by the integration tests. 😄

@joschi

This comment has been minimized.

Contributor

joschi commented Jul 26, 2016

LGTM. 👍

@joschi joschi self-assigned this Jul 26, 2016

@joschi joschi merged commit 5a63d3c into master Jul 26, 2016

4 checks passed

ci-server-integration Jenkins build graylog2-server-integration-pr 1162 has succeeded
Details
ci-web-linter Jenkins build graylog-pr-linter-check 645 has succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@joschi joschi deleted the fix-plugin-resource-injection branch Jul 26, 2016

bernd added a commit that referenced this pull request Jul 27, 2016

Fix REST API browser after changes to the PluginRestResource injection
Create a wrapper class for the map of PluginRestResource classes to make
it injectable into Jersey resources.

Fallout from #2492.

joschi added a commit that referenced this pull request Jul 28, 2016

Fix REST API browser after changes to the PluginRestResource injection (
#2550)

Create a wrapper class for the map of PluginRestResource classes to make
it injectable into Jersey resources.

Fallout from #2492.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment