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

Custom ELResolver's setValue with a null base cannot be called #37

Closed
glassfishrobot opened this issue Jan 14, 2014 · 5 comments
Closed
Assignees
Labels
Component: API Affects the API and (by implication) the spec and all implementations invalid This doesn't seem right Priority: Major

Comments

@glassfishrobot
Copy link

I think there's no way that a custom ELResolver's setValue with null 'base' argument is called.

That's because in StandardELContext the call of a LocalBeanNameResolver precedes the customResolvers:

@Override
    public ELResolver getELResolver() {
        if (elResolver == null) {
            CompositeELResolver resolver = new CompositeELResolver();
            resolver.add(new BeanNameELResolver(new LocalBeanNameResolver()));
            customResolvers = new CompositeELResolver();
            resolver.add(customResolvers);
            if (streamELResolver != null) {
resolver.add(streamELResolver);
            }
            resolver.add(new StaticFieldELResolver());
            resolver.add(new MapELResolver());
            resolver.add(new ResourceBundleELResolver());
            resolver.add(new ListELResolver());
            resolver.add(new ArrayELResolver());
            resolver.add(new BeanELResolver());
            elResolver = resolver;
        }
        return elResolver;
    }

( The correct order would be

customResolvers = new CompositeELResolver();
            resolver.add(customResolvers);
            resolver.add(new BeanNameELResolver(new LocalBeanNameResolver()));

)

Futhermore, there's no way to replace the StandardELContext,
because the ELManager puts a wrapper to the ELContext argument.

public ELContext setELContext(ELContext context) {
        ELContext prev = elContext;
        elContext = new StandardELContext(context);
        return prev;
    }

( I think the correct code would be the following, because the attribute elContext may have only ELContext type not StandardELContext:

public ELContext setELContext(ELContext context) {
        ELContext prev = elContext;
        elContext = context;
        return prev;
    }

)

Moreover, it is not possible to replace the ELManager in the ELProcessor (it is private field) not even using inheritance (a subclass), because the code of the ELProcessor references directly to the private field and not uses the getELProcessor() getter method.


Test:

@Test public void test_ELResolver_setValue(){
    final Map<String,Object> map=new HashMap<String, Object>();
    ELProcessor el = new ELProcessor();
    el.getELManager().addELResolver(new BeanELResolver(){
      public void setValue(ELContext context, Object base, Object property, Object val) {
        if(base==null){
          context.setPropertyResolved(true);
          map.put(property.toString(), val);
        }
      }
      public boolean isReadOnly(ELContext context, Object base, Object property) {
        return false;
      }
      /*public Object getValue(ELContext context, Object base, Object property) {
        if(base==null){
          context.setPropertyResolved(true);
          return map.get(property.toString());
        }
        return null;
      }*/      
    });            
    Object r1=el.eval("a=1");
    el.setValue("b", 2);
    // error: map is still empty!
    Assert.assertEquals(map.get("a"),1L); // error!
    Assert.assertEquals(map.get("b"),1); // error!
  }

Workaround:

To create subclasses using small modifications mentioned above in the StandardELContext and ELManager, and create a subclass but a whole replacement of the ELProcessor that uses the ELManager subclass.

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
Reported by kchung

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
This issue was imported from java.net JIRA UEL-37

@glassfishrobot
Copy link
Author

@markt-asf
Copy link
Contributor

This needs further investigation / thought.

The code was changed after the EL 3.0 release (but the Javadoc was not) while under the JCP process. The code was then transferred to Eclipse. The change was, therefore present in the Jakarta EE 8 release but not in the Java EE 8 release. (Actually, there was no EL update for Java EE 8 so the 3.0 version from Java EE 7 was used.)

We either need to revert this change or update the Javadoc and spec doc to reflect the change.

@markt-asf markt-asf added the Component: API Affects the API and (by implication) the spec and all implementations label Jan 24, 2020
markt-asf added a commit that referenced this issue Feb 26, 2020
Reverts this commit:
javaee/uel-ri@e0390e9
The change is being reverted because:
- the problem statement in the bug report that led to the change (#37)
  is incorrect
- this was an undocumented functional change made between Java EE 8 and
  Jakarta EE 8

Signed-off-by: Mark Thomas <markt@apache.org>
@markt-asf markt-asf added invalid This doesn't seem right and removed bug labels Feb 26, 2020
@markt-asf
Copy link
Contributor

My reading of the code is that the problem statement is not
correct. You can't override the BeanELResolver but it does not resolve
all properties with a null base therefore any custom resolver will be
called if the BeanELResolver is unable to resolve the property.

The change made after the EL 3.0 has been reverted for Jakarta EL 4.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: API Affects the API and (by implication) the spec and all implementations invalid This doesn't seem right Priority: Major
Projects
None yet
Development

No branches or pull requests

3 participants