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

Omnifaces is resolving incorrectly a converter #399

Closed
wsaca opened this Issue Sep 7, 2017 · 5 comments

Comments

Projects
None yet
2 participants
@wsaca

wsaca commented Sep 7, 2017

Hello, I have a converter MyConverter annoted with @FacesConverter(forClass = My.class), it extends SelectItemsConverter, but when I updated from OmniFaces 2.5.1 to 2.6.4 my converter was not working.

PrimeFaces is resolving incorrectly this converter because OmniFaces is returning another one.

I think that there is an error in the ConverterManager implementation. I will check this problem in the next days.

@BalusC

This comment has been minimized.

Show comment
Hide comment
@BalusC

BalusC Sep 7, 2017

Member

Cannot reproduce it on an empty project with Mojarra 2.3.2 + OmniFaces 2.6.4.

public class Person {}
@FacesConverter(forClass=Person.class)
public class PersonConverter extends SelectItemsConverter {
    @Override
    public String getAsString(FacesContext context, UIComponent component, Object value) {
        System.out.println("PersonConverter#getAsString() invoked");
        return super.getAsString(context, component, value);
    }
}
@Named
@RequestScoped
public class Bean {
    private Person person;

    @PostConstruct
    public void init() {
        person = new Person();
    }

    public Person getPerson() {
        return person;
    }
}
<h:outputText value="#{bean.person}" />

It's correctly invoked. A reproducer would be helpful. It would also be helpful to tell what converter exactly it returns instead and how the invalid converter is possibly related to the valid converter.

Member

BalusC commented Sep 7, 2017

Cannot reproduce it on an empty project with Mojarra 2.3.2 + OmniFaces 2.6.4.

public class Person {}
@FacesConverter(forClass=Person.class)
public class PersonConverter extends SelectItemsConverter {
    @Override
    public String getAsString(FacesContext context, UIComponent component, Object value) {
        System.out.println("PersonConverter#getAsString() invoked");
        return super.getAsString(context, component, value);
    }
}
@Named
@RequestScoped
public class Bean {
    private Person person;

    @PostConstruct
    public void init() {
        person = new Person();
    }

    public Person getPerson() {
        return person;
    }
}
<h:outputText value="#{bean.person}" />

It's correctly invoked. A reproducer would be helpful. It would also be helpful to tell what converter exactly it returns instead and how the invalid converter is possibly related to the valid converter.

@wsaca

This comment has been minimized.

Show comment
Hide comment
@wsaca

wsaca Sep 7, 2017

The converter returned is located in a jar file. I'll try to reproduce this issue or find what is the problem for tomorrow. Thanks.

wsaca commented Sep 7, 2017

The converter returned is located in a jar file. I'll try to reproduce this issue or find what is the problem for tomorrow. Thanks.

@wsaca

This comment has been minimized.

Show comment
Hide comment
@wsaca

wsaca Sep 8, 2017

Balus, Can you explain me why if I set "bean-discovery-mode" to "annotated" in my beans.xml OmniFaces is not resolving correctly the Converter? BeansLocal.resolve is returning a different converter because is finding the parent classes, but if I set "bean-discovery-mode" to "all" then is working... I think it shouldn't happen...

wsaca commented Sep 8, 2017

Balus, Can you explain me why if I set "bean-discovery-mode" to "annotated" in my beans.xml OmniFaces is not resolving correctly the Converter? BeansLocal.resolve is returning a different converter because is finding the parent classes, but if I set "bean-discovery-mode" to "all" then is working... I think it shouldn't happen...

@wsaca

This comment has been minimized.

Show comment
Hide comment
@wsaca

wsaca Sep 8, 2017

I attached a basic example that show this error. You could change the version to 2.5.1 to verify that OF was working previously.

test-omnifaces-cdi.zip

wsaca commented Sep 8, 2017

I attached a basic example that show this error. You could change the version to 2.5.1 to verify that OF was working previously.

test-omnifaces-cdi.zip

@BalusC

This comment has been minimized.

Show comment
Hide comment
@BalusC

BalusC Sep 17, 2017

Member

This is the consequence of fix done for a failing Beans#destroy(T instance) in d1f9769#diff-c90994f2e3550a2ad22017614f696cf6. I have removed the fix and improved it.

Do note that with 2.5.1 you didn't actually get a CDI managed converter but a JSF managed one as fallback. In other words, @Inject would never have worked in ModelOneConverter. With 2.6 - 2.6.4 you would get a CDI managed superclass where found, but it's after all indeed the wrong approach. With 2.6.5 the 2.5.1 behavior is restored.

All should work for you now. If in the future you want to be able to use @Inject in ModelOneConverter, then you should use bean-discovery-mode="all". See also http://showcase.omnifaces.org/cdi/FacesConverter

Thanks for report and reproducer!

Member

BalusC commented Sep 17, 2017

This is the consequence of fix done for a failing Beans#destroy(T instance) in d1f9769#diff-c90994f2e3550a2ad22017614f696cf6. I have removed the fix and improved it.

Do note that with 2.5.1 you didn't actually get a CDI managed converter but a JSF managed one as fallback. In other words, @Inject would never have worked in ModelOneConverter. With 2.6 - 2.6.4 you would get a CDI managed superclass where found, but it's after all indeed the wrong approach. With 2.6.5 the 2.5.1 behavior is restored.

All should work for you now. If in the future you want to be able to use @Inject in ModelOneConverter, then you should use bean-discovery-mode="all". See also http://showcase.omnifaces.org/cdi/FacesConverter

Thanks for report and reproducer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment