search

jakartaee / faces

Jakarta Faces
Other
109 stars 55 forks source link

Require firing events for @Initialized, @BeforeDestroyed, @Destroyed for build-in scopes. #1739

Closed arjantijms closed 1 year ago

arjantijms commented 2 years ago

The events are currently weakly encouraged via the CDI specification, with the likely intend to be required by any specifications implementing those. As such Faces should probably require them.

See https://github.com/jakartaee/faces/issues/1731

BalusC commented 1 year ago

@tandraschko Is this already implemented in Mojarra? I can't find associated PR.

tandraschko commented 1 year ago

i thought it is: https://github.com/eclipse-ee4j/mojarra/blob/7b2cc1f49b31e6dffd1c50cf433ca7e6b2c2c7c6/impl/src/main/java/com/sun/faces/application/view/ViewScopedCDIEventFireHelper.java#L25

AFAIR it just wasnt mentioned in the specs

BalusC commented 1 year ago

Yes this is what the issue was about. The spec should require firing the events. This change of the spec is not yet implemented in Mojarra side thus. Is it in MyFaces side?

tandraschko commented 1 year ago

we dont have specs/javadocs for this part but the code side is implemented

BalusC commented 1 year ago

Now also in Mojarra.

volosied commented 9 months ago

Has @BeforeDestroyed been tested? I'm not seeing the method with this annotation invoked in either Mojarra or MyFaces 4.1 M1/RC1 Releases.

TCK only looks at @Initialized and @Destroyed (which work fine) https://github.com/jakartaee/faces/blob/694b38430ddd9ee9ce6e8670c026739c1b3c0721/tck/faces22/cdiInitDestroyEvent/src/main/java/ee/jakarta/tck/faces/test/javaee7/cdiinitdestroyevent/cdiinitdestroyevent/FlowLogger.java

volosied commented 9 months ago

Ah, our Faces impl code doesn't fire the @BeforeDestroyed event.