Inject customizable extraction functions to RegisteredLookupDimension #724
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
QubitPi
force-pushed
the
issue-723-customizable-extractFns
branch
from
d0f86ef
to
6bfdece
Compare
QubitPi
force-pushed
the
issue-723-customizable-extractFns
branch
2 times, most recently
from
30e2281
to
3649ac1
Compare
QubitPi
commented
Jun 21, 2018
| keyValueStore, | ||
| searchProvider | ||
| ); | ||
| this.registeredLookupExtractionFns = Collections.unmodifiableList(registeredLookupExtractionFns); |
There was a problem hiding this comment.
Only this line is effectively changed for this constructor, the rest is Javadoc updates
| @NotNull LinkedHashSet<DimensionField> fields, | ||
| @NotNull KeyValueStore keyValueStore, | ||
| @NotNull SearchProvider searchProvider, | ||
| @NotNull List<ExtractionFunction> registeredLookupExtractionFns |
There was a problem hiding this comment.
Only this line is effectively changed for this constructor, the rest is Javadoc updates
michael-mclawhorn
requested changes
Jun 27, 2018
| @@ -17,9 +18,9 @@ default Class getType() { | |||
| } | |||
|
|
|||
| /** | |||
| * Returns a list of lookups used to configure the Lookup dimension. | |||
| * Returns a list of extraction function for the lookup dimension values. | |||
There was a problem hiding this comment.
(sp) extraction functions
michael-mclawhorn
approved these changes
Jun 27, 2018
efronbs
approved these changes
Jun 27, 2018
QubitPi
force-pushed
the
issue-723-customizable-extractFns
branch
from
3d8f29f
to
c194850
Compare
|
Thank you for the review and approvals, @michael-mclawhorn @efronbs ! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses issue #723
Problem
We have a business requirement to return different default value for different registered lookup. To be more specific, here is an example of serialized registered lookup extraction function
{ "type" : "registeredLookup", "lookup" : "<lookup name>", "retainMissingValue" : false, "replaceMissingValueWith" : "<default lookup value>", "injective" : false, "optimize" : true }API supports lookups with user-defined lookup name as in
"lookup" : "<lookup name>". It is, however, not possible for downstream projects to specify the rest of attributes. We need a different default lookup value as in"replaceMissingValueWith" : "<default lookup value>"for each lookup extraction function.Solution
The cause of this issue is in
DefaultRegisteredLookupDimensionConfig, we only pass in a list of lookup names. If, instead, we pass in a list ofRegisteredLookupExtractionFunction, the downstream projects could freely define all of the attributes for a registered lookup extraction function and then configure them in.This PR essentially changes
List<String> lookupstoList<ExtractionFunction> registeredLookupExtractionFnsinDefaultRegisteredLookupDimensionConfig.@michael-mclawhorn This might have impact on #719