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

Support Iterable in SpEL Indexer #26323

Conversation

pilak
Copy link

@pilak pilak commented Dec 28, 2020

  • make object implementing java.lang.Iterable accessible with SpEL
    indexing
  • only read access is possible, as we cannot write an iterable object
  • make compiler compatible

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 28, 2020
@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement labels Dec 28, 2020
@sbrannen sbrannen changed the title Add the ability for AST Indexer to interpret java.lang.Iterable object Add the ability for SpEL AST Indexer to interpret java.lang.Iterable object Dec 28, 2020
@pilak pilak marked this pull request as draft December 28, 2020 18:45
* make object implementing java.lang.Iterable accessible with SpEL
indexing
* only read access is possible, as we cannot write an iterable object
* make compiler compatible
@pilak pilak force-pushed the ast_indexer_interpret_Iterable_capability branch from 4903437 to bdd8c10 Compare December 31, 2020 23:06
@pilak pilak marked this pull request as ready for review December 31, 2020 23:19
@sbrannen
Copy link
Member

sbrannen commented Jan 2, 2021

Is it reasonable to support access to Iterable objects via an index?

For example, the Javadoc for Collection explicitly states:

Returns an iterator over the elements in this collection. There are no guarantees concerning the order in which the elements are returned (unless this collection is an instance of some class that provides a guarantee).

Thus, for many Collection implementations and other Iterable implementations, the order returned by the iterator() method may potentially change between invocations, which would lead to unreliable evaluations for SpEL expressions making use of this proposed feature.

@aclement & @jhoeller, what your thoughts on the proposal?

@pilak
Copy link
Author

pilak commented Jan 12, 2021

@sbrannen thank you for the interesting notice

In the Javadoc of java.util.List#iterator, we can read this:

Returns an iterator over the elements in this list in proper sequence.

So I guess that in most cases the Iterator will preserve the ordering, it would be worth to allow the access with Indexer.

Furthermore, if the user traverses the object in an indexing manner (using [n]), isn't that its own responsibility to know if the underlying Iterator is ordered or not?

Thanks for your advises

@sbrannen sbrannen added status: superseded An issue that has been superseded by another and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 9, 2021
@sbrannen
Copy link
Member

sbrannen commented Feb 9, 2021

Superseded by #26409

@sbrannen sbrannen closed this Feb 9, 2021
@sbrannen sbrannen changed the title Add the ability for SpEL AST Indexer to interpret java.lang.Iterable object Support Iterable in SpEL Indexer Feb 6, 2024
@sbrannen
Copy link
Member

sbrannen commented Feb 7, 2024

I realize this issue was closed 3 years ago, but I discovered there are already ways to "index" into an Iterable in SpEL.

In 4.2, @jhoeller added collection selection/projection support for Iterable (see #17822).

In light of that I introduced dedicated tests to show this in action. Note that the Counter implements Iterable but is not a Collection.

void indexIntoIterableSelection() {
EvaluationContext context = new StandardEvaluationContext();
context.setVariable("counter", new Counter(5));
ExpressionParser parser = new SpelExpressionParser();
// Select first (selection expression is hard-coded to true)
Expression expression = parser.parseExpression("#counter.^[true]");
assertThat(expression.getValue(context, Integer.class)).isEqualTo(1);
// Select last (selection expression is hard-coded to true)
expression = parser.parseExpression("#counter.$[true]");
assertThat(expression.getValue(context, Integer.class)).isEqualTo(5);
// Select index (selection expression is hard-coded to true)
// The following will not work, since we cannot index into an Iterable.
// expression = parser.parseExpression("#counter[1]");
// The following works, since project selection with a selection expression
// that always evaluates to true results in a List containing all elements
// of the Iterable, and we can index into that List.
expression = parser.parseExpression("#counter.?[true][1]");
assertThat(expression.getValue(context, Integer.class)).isEqualTo(2);
expression = parser.parseExpression("#counter.?[true][2]");
assertThat(expression.getValue(context, Integer.class)).isEqualTo(3);
}

In summary, given an Iterable available as #iterable, if we supply true as the selection expression (which basically means select all elements from the Iterable in its iteration order and store them in a new List), we can:

  • select first element: #iterable.^[true]
  • select last element: #iterable.$[true]
  • select 2nd element: #iterable.?[true][1]
  • select 3rd element: #iterable.?[true][2]

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) status: superseded An issue that has been superseded by another type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants