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

Flatten IndicesModule and add tests #12921

Merged
merged 1 commit into from Aug 17, 2015

Conversation

Projects
None yet
5 participants
@rjernst
Member

rjernst commented Aug 17, 2015

The IndicesModule was made up of two submodules, one which handled registering queries, and the other for registering hunspell dictionaries. This change moves those into IndicesModule. It also adds a new extension point type, InstanceMap. This is simply a Map<K,V>, where K and V are actual objects, not classes like most other extension points. I also added a test method to help testing instance map extensions. This was particularly painful because of how guice binds the key and value as separate bindings, and then reconstitutes them into a Map at injection time. In order to gain access to the object which links the key and value, I had to tweak our guice copy to not use an anonymous inner class for the Provider.

Note that I also renamed the existing extension point types, since they were very redundant. For example, ExtensionPoint.MapExtensionPoint is now ExtensionPoint.ClassMap.

See #12783.

Internal: Flatten IndicesModule and add tests
The IndicesModule was made up of two submodules, one which
handled registering queries, and the other for registering
hunspell dictionaries. This change moves those into
IndicesModule. It also adds a new extension point type,
InstanceMap. This is simply a Map<K,V>, where K and V are
actual objects, not classes like most other extension points.
I also added a test method to help testing instance map extensions.
This was particularly painful because of how guice binds the key
and value as separate bindings, and then reconstitutes them
into a Map at injection time. In order to gain access to the
object which links the key and value, I had to tweak our
guice copy to not use an anonymous inner class for the Provider.

Note that I also renamed the existing extension point types, since
they were very redundant. For example, ExtensionPoint.MapExtensionPoint
is now ExtensionPoint.ClassMap.

See #12783.
@s1monw

This comment has been minimized.

Show comment
Hide comment
@s1monw

s1monw Aug 17, 2015

Contributor

LGTM

Contributor

s1monw commented Aug 17, 2015

LGTM

rjernst added a commit that referenced this pull request Aug 17, 2015

Merge pull request #12921 from rjernst/module_culling2
Flatten IndicesModule and add tests

@rjernst rjernst merged commit 52f3eb5 into elastic:master Aug 17, 2015

1 check passed

CLA Commit author is a member of Elasticsearch
Details

@rjernst rjernst deleted the rjernst:module_culling2 branch Aug 17, 2015

@rjernst

This comment has been minimized.

Show comment
Hide comment
@rjernst

rjernst Aug 17, 2015

Member

I will backport to the 2.0 branch once the beta is out.

Member

rjernst commented Aug 17, 2015

I will backport to the 2.0 branch once the beta is out.

rjernst added a commit that referenced this pull request Aug 17, 2015

javanna added a commit to javanna/elasticsearch that referenced this pull request Aug 18, 2015

Internal: IndicesQueriesRegitry back to being created only once
With #12921 we refactored IndicesModule but we forgot to make sure we create IndicesQueriesRegistry once. IndicesQueriesModule used to do `bind(IndicesQueriesRegistry.class).asEagerSingleton();` otherwise we get multiple instances of the registry. This needs to be ported do the IndicesModule.
@javanna

This comment has been minimized.

Show comment
Hide comment
@javanna

javanna Aug 18, 2015

Member

@rjernst when you backport don't forget about #12956 ;)

Member

javanna commented Aug 18, 2015

@rjernst when you backport don't forget about #12956 ;)

rjernst added a commit that referenced this pull request Aug 19, 2015

Internal: Flatten IndicesModule and add tests
The IndicesModule was made up of two submodules, one which
handled registering queries, and the other for registering
hunspell dictionaries. This change moves those into
IndicesModule. It also adds a new extension point type,
InstanceMap. This is simply a Map<K,V>, where K and V are
actual objects, not classes like most other extension points.
I also added a test method to help testing instance map extensions.
This was particularly painful because of how guice binds the key
and value as separate bindings, and then reconstitutes them
into a Map at injection time. In order to gain access to the
object which links the key and value, I had to tweak our
guice copy to not use an anonymous inner class for the Provider.

Note that I also renamed the existing extension point types, since
they were very redundant. For example, ExtensionPoint.MapExtensionPoint
is now ExtensionPoint.ClassMap.

See #12783.

Backport of #12921

javanna added a commit that referenced this pull request Aug 19, 2015

Internal: IndicesQueriesRegitry back to being created only once
With #12921 we refactored IndicesModule but we forgot to make sure we create IndicesQueriesRegistry once. IndicesQueriesModule used to do `bind(IndicesQueriesRegistry.class).asEagerSingleton();` otherwise we get multiple instances of the registry. This needs to be ported do the IndicesModule.

backport of #12956

@rjernst rjernst added v2.0.0-beta1 and removed v2.0.0 labels Aug 19, 2015

@jpountz jpountz removed the v2.1.0 label Aug 21, 2015

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