Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Introduce SPI to configure the SpEL Indexer #26409

Closed
jackmiking opened this issue Jan 20, 2021 · 22 comments
Closed

Introduce SPI to configure the SpEL Indexer #26409

jackmiking opened this issue Jan 20, 2021 · 22 comments
Assignees
@sbrannen
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: superseded An issue that has been superseded by another type: enhancement A general enhancement

Comments

@jackmiking
Copy link

jackmiking commented Jan 20, 2021 •
edited by sbrannen
Loading

The SpEL indexer doesn't provide support for Jackson's ArrayNode, and it seems to be non-extensible; whereas, a Jackson ObjectNode can be operated on within a SpEL expression via PropertyAccessor support.
@snicoll snicoll transferred this issue from spring-projects/spring-boot Jan 20, 2021
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 20, 2021
@sbrannen sbrannen changed the title spel can't work with jackson's arrayNode SpEL does not support Jackson ArrayNode Jan 20, 2021
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Jan 20, 2021
@sbrannen
Copy link
Member

Can you please describe your use case and explain why you want to parse Jackson types in a SpEL expression?

@sbrannen sbrannen added the status: waiting-for-feedback We need additional information before we can continue label Jan 20, 2021
@jackmiking
Copy link
Author

jackmiking commented Jan 21, 2021 •
edited by sbrannen
Loading

well, we are building up a code-less project which should store request context in JSON since it's data structure is completely dynamic. And this request context will be used by some handler by using config data.

For example, if the request context looks like this: {"a": {"b": [{"c": "dd"},{"c": "ee"}]}}.
there might be a handler which want to make a choice for the next handler base on the value of "a.b[0].c".

it is easy to implement only the fetch logic here. However we might also want to execute some functions and even a calculation. It is hard to finish it from scratch, and here SpEL comes into play, and that is why we need an extended mode for the Indexer class.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 21, 2021
@sbrannen
Copy link
Member

How are you evaluating your SpEL expressions?

Do you have custom setup for the SpelExpressionParser, EvaluationContext, etc.?

@sbrannen sbrannen added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jan 22, 2021
@jackmiking
Copy link
Author

yes, the context can config property accessor,however, that is no the case for arrayNode. an array item is accessed by the Indexer class which doesn't allow for extending

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 27, 2021
@sbrannen
Copy link
Member

yes, the context can config property accessor,however, that is no the case for arrayNode. an array item is accessed by the Indexer class which doesn't allow for extending

That is precisely why we asked if you have custom setup for the SpelExpressionParser, EvaluationContext, etc.

I'm assuming you answered "yes" to that question. If that is not what you were answering "yes" to, please expound.

In light of that, would it meet your needs if we made it possible to configure the Indexer so that you could provide your own support for treating a Jackson ArrayNode as an indexed collection -- for example, via a strategy interface that you could implement and register?

@sbrannen sbrannen added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jan 27, 2021
@jackmiking
Copy link
Author

yes, it is totally enough.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 27, 2021
@sbrannen
Copy link
Member

@aclement, do you have time to introduce a new strategy for customizing the indexer?

If not, simply providing some pseudo-code style tips regarding how you best foresee accomplishing this might be enough for a core committer to take over.

@jackmiking
Copy link
Author

Maybe I can help. I would try to provide a solution for this issue at weekend.

@aclement, do you have time to introduce a new strategy for customizing the indexer?

If not, simply providing some pseudo-code style tips regarding how you best foresee accomplishing this might be enough for a core committer to take over.

@sbrannen
Copy link
Member

sbrannen commented Jan 28, 2021 •
edited
Loading

I just learned that Spring Integration has custom SpEL support for Jackson.

See their JsonPropertyAccessor (and its nested ArrayNodeAsList type) and SpelPropertyAccessorRegistrar.

Perhaps their is room for synergies between Spring Integration (SI) and Spring Framework regarding SI's current support, this issue, and #26323.

@garyrussell, @artembilan, @aclement, and @jhoeller : thoughts?

@artembilan
Copy link
Member

Right. We don't have any objections to move JsonPropertyAccessor from Spring Integration to Spring Framework.

The SpelPropertyAccessorRegistrar has direct interaction with the AbstractEvaluationContextFactoryBean which produces prototype instances of the EvaluationContext whenever we evaluation expressions in Spring Integration. Not sure if that is relevant for the current issue. Perhaps JsonPropertyAccessor for the target EvaluationContext is fully enough to satisfy this issue requirements.

To have a fix for #26323 would be a good addition, too.

@jackmiking jackmiking mentioned this issue Jan 30, 2021
Closed
@jackmiking
Copy link
Author

warping ArrayNode in a list seems good. however, I still commit a pr for this problem as an alternative.

@pilak
Copy link

pilak commented Feb 5, 2021

this solution looks like better than #26323

thus it would be possible for any type of indexer to make its own accessing implementation
moreover it will leave the possibility for the JsonNode to be writable in SpEL one day IMO :)

@jhoeller jhoeller modified the milestones: 6.0.x, 6.1.x Jan 11, 2023
@jhoeller jhoeller modified the milestones: 6.1.x, 6.x Backlog Nov 9, 2023
@sbrannen sbrannen modified the milestones: 6.x Backlog, 6.2.x Feb 6, 2024
@sbrannen sbrannen added the status: superseded An issue that has been superseded by another label Feb 6, 2024
@sbrannen sbrannen removed this from the 6.2.x milestone Feb 6, 2024
@sbrannen
Copy link
Member

sbrannen commented Feb 6, 2024

@sbrannen sbrannen closed this as not planned Won't fix, can't repro, duplicate, stale Feb 6, 2024
@jackmiking

This comment was marked as off-topic.

Sign in to view
sbrannen pushed a commit to sbrannen/spring-framework that referenced this issue Apr 10, 2024
@sbrannen
Introduce IndexAccessor SPI to customize the SpEL Indexer
50a0000 
See spring-projectsgh-26409
See spring-projectsgh-26478
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Apr 10, 2024
@sbrannen
Initial review and polish of IndexAccessor support in SpEL
725bf4a 
See spring-projectsgh-26409
See spring-projectsgh-26478
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Apr 10, 2024
@sbrannen
Revise null-safety contracts in IndexAccessor SPI
92ecaf1 
When indexing into an object, the target object can never be null.

See spring-projectsgh-26409
See spring-projectsgh-26478
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Apr 10, 2024
@sbrannen
Change IndexAccessor#read()'s return type to TypedValue
1c937a6 
Prior to this commit, the read() method in the IndexAccessor SPI
declared a return type of ValueRef which introduced a package cycle.

This commit addresses this by aligning with the PropertyAccess SPI and
returning TypedValue from IndexAccessor's read() method. This commit
also reworks the internals of Indexer based on a new, local
IndexAccessorValueRef implementation.

See spring-projectsgh-26409
See spring-projectsgh-26478
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Apr 10, 2024
@sbrannen
Introduce proper error handling for IndexAccessor support in SpEL
da5349f 
See spring-projectsgh-26409
See spring-projectsgh-26478
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Apr 10, 2024
@sbrannen
Revise Javadoc for IndexAccessor
77119c1 
See spring-projectsgh-26409
See spring-projectsgh-26478
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Apr 10, 2024
@sbrannen
Honor IndexAccessor#canWrite() and cache accessors
8f860b9 
See spring-projectsgh-26409
See spring-projectsgh-26478
sbrannen added a commit that referenced this issue Apr 10, 2024
@sbrannen
Initial review and polish of IndexAccessor support in SpEL
d912770 
See gh-26409
See gh-26478
sbrannen added a commit that referenced this issue Apr 10, 2024
@sbrannen
Revise null-safety contracts in IndexAccessor SPI
b7c3833 
When indexing into an object, the target object can never be null.

See gh-26409
See gh-26478
sbrannen added a commit that referenced this issue Apr 10, 2024
@sbrannen
Change IndexAccessor#read()'s return type to TypedValue
5c6b82a 
Prior to this commit, the read() method in the IndexAccessor SPI
declared a return type of ValueRef which introduced a package cycle.

This commit addresses this by aligning with the PropertyAccess SPI and
returning TypedValue from IndexAccessor's read() method. This commit
also reworks the internals of Indexer based on a new, local
IndexAccessorValueRef implementation.

See gh-26409
See gh-26478
sbrannen added a commit that referenced this issue Apr 10, 2024
@sbrannen
Introduce proper error handling for IndexAccessor support in SpEL
22bfe7d 
See gh-26409
See gh-26478
sbrannen added a commit that referenced this issue Apr 10, 2024
@sbrannen
Revise Javadoc for IndexAccessor
1551189 
See gh-26409
See gh-26478
sbrannen added a commit that referenced this issue Apr 10, 2024
@sbrannen
Honor IndexAccessor#canWrite() and cache accessors
1c9cff6 
See gh-26409
See gh-26478
sbrannen added a commit that referenced this issue Apr 10, 2024
@sbrannen
Merge SpEL IndexAccessor feature into main
ae3dc0d 
This set of commits introduces a new IndexAccessor SPI for the Spring
Expression Language (SpEL) which allows third parties to customize the
SpEL Indexer.

A custom IndexAccessor implementation can be registered in a
StandardEvaluationContext.

For an example, see the JacksonArrayNodeIndexAccessor in IndexingTests
in the spring-expression module.

See gh-26409
Closes gh-26478
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sbrannen sbrannen

Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: superseded An issue that has been superseded by another type: enhancement A general enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Introduce IndexAccessor SPI to customize the SpEL Indexer jackmiking/spring-framework
8 participants
@sbrannen @artembilan @martin-jamszolik @jhoeller @pilak @jackmiking @jdomigon @spring-projects-issues