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.

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

Introduce IndexAccessor SPI to customize the SpEL Indexer #26478

Conversation

jackmiking
Copy link

closes gh-26409.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 30, 2021
@sbrannen

This comment was marked as outdated.

@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) status: waiting-for-feedback We need additional information before we can continue labels Jan 31, 2021
@jackmiking jackmiking force-pushed the SpEL-index-accessor-jackmiking branch 2 times, most recently from 03877f7 to 609002c Compare February 8, 2021 06:00
@jackmiking

This comment was marked as outdated.

@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 Feb 8, 2021
@jackmiking jackmiking force-pushed the SpEL-index-accessor-jackmiking branch from 609002c to 89cc12d Compare March 24, 2021 02:24
@jackmiking jackmiking force-pushed the SpEL-index-accessor-jackmiking branch from 89cc12d to 79ed058 Compare March 24, 2021 06:37
@jackmiking jackmiking force-pushed the SpEL-index-accessor-jackmiking branch from 79ed058 to 1409ba7 Compare March 24, 2021 06:42
@sbrannen sbrannen changed the title add indexAccessor for SpEL. Introduce IndexAccessor SPI to configure the SpEL Indexer Dec 7, 2022
@sbrannen sbrannen marked this pull request as draft December 7, 2022 22:36
@jackmiking jackmiking marked this pull request as ready for review December 13, 2022 09:22
@sbrannen sbrannen added the type: enhancement A general enhancement label Dec 13, 2022
@sbrannen sbrannen self-assigned this Feb 6, 2024
@sbrannen sbrannen removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Feb 6, 2024
@sbrannen sbrannen modified the milestone: 6.2.x Feb 6, 2024
@sbrannen sbrannen added status: superseded An issue that has been superseded by another and removed status: superseded An issue that has been superseded by another labels Feb 6, 2024
@sbrannen sbrannen added this to the 6.2.x milestone Feb 6, 2024
@sbrannen sbrannen removed this from the 6.2.x milestone Feb 6, 2024
@jackmiking
Copy link
Author

it's glad to hear your response. Your affirmation cheers me up.
I am anticipating the day it is merged into the release version.

sbrannen pushed a commit to sbrannen/spring-framework that referenced this pull request Apr 10, 2024
sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Apr 10, 2024
sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Apr 10, 2024
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 pull request Apr 10, 2024
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 pull request Apr 10, 2024
sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Apr 10, 2024
sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Apr 10, 2024
sbrannen added a commit that referenced this pull request Apr 10, 2024
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 pull request Apr 10, 2024
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 pull request Apr 10, 2024
sbrannen added a commit that referenced this pull request Apr 10, 2024
@sbrannen sbrannen closed this in ae3dc0d Apr 10, 2024
@jackmiking

This comment was marked as off-topic.

@sbrannen
Copy link
Member

sbrannen commented Apr 10, 2024

This has been merged into main in ae3dc0d along with an additional 6 commits that revise the PR.

For an example, see the JacksonArrayNodeIndexAccessor in IndexingTests:

/**
* {@link IndexAccessor} that knows how to read and write indexes in a
* Jackson {@link ArrayNode}.
*/
private static class JacksonArrayNodeIndexAccessor implements IndexAccessor {
private final ObjectMapper objectMapper;
JacksonArrayNodeIndexAccessor(ObjectMapper objectMapper) {
this.objectMapper = objectMapper;
}
@Override
public Class<?>[] getSpecificTargetClasses() {
return new Class[] { ArrayNode.class };
}
@Override
public boolean canRead(EvaluationContext context, Object target, Object index) {
return (target instanceof ArrayNode && index instanceof Integer);
}
@Override
public TypedValue read(EvaluationContext context, Object target, Object index) {
ArrayNode arrayNode = (ArrayNode) target;
Integer intIndex = (Integer) index;
return new TypedValue(arrayNode.get(intIndex));
}
@Override
public boolean canWrite(EvaluationContext context, Object target, Object index) {
return canRead(context, target, index);
}
@Override
public void write(EvaluationContext context, Object target, Object index, @Nullable Object newValue) {
ArrayNode arrayNode = (ArrayNode) target;
Integer intIndex = (Integer) index;
arrayNode.set(intIndex, this.objectMapper.convertValue(newValue, JsonNode.class));
}
}
}

@jackmiking, thanks again for putting together the initial prototype!

@artembilan, @pilak, @jackmiking, @jdomigon, @martin-jamszolik, and anyone else interested, we would be grateful if you could try this out in the upcoming 6.2 M1 release (scheduled for tomorrow) and let us know if you run into any issues or come up with any ideas for how to improve (or simplify) the new IndexAccessor SPI.

Cheers,

Sam


p.s. I have opened a separate issue (#32613) to introduce compilation support for an IndexAccessor in Spring Framework 6.2 M2, analogous to the existing CompilablePropertyAccessor SPI and tentatively named CompilableIndexAccessor.

@jackmiking
Copy link
Author

I have reviewed the code and made some cases. It works great.

@sbrannen
Copy link
Member

Hi @jackmiking,

Thanks for trying it out and letting us know that it works! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce SPI to configure the SpEL Indexer
4 participants