Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

GTNPC-86 Add support for CDI portlets #6

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Contributor

kenfinnigan commented Mar 28, 2013

No description provided.

Owner

bdaw commented Mar 28, 2013

Julien could you review? I can handle the merge and release.

Member

metacosm commented Mar 28, 2013

I suspect I will need to check it out as well as it might trigger changes in WSRP since there are components that monitor portlets lifecycle.

Contributor

vietj commented Mar 28, 2013

I will review it next monday when I am back.

Contributor

vietj commented Apr 2, 2013

The change as it is cannot be accepted for at at least two reasons:

1/ it does not define properly the create operation in the managed life cycle interface: the transition -> created is done manually in the portlet container implementation.

2/ it does not define a symmetric destroy life cycle operation

Contributor

kenfinnigan commented Apr 2, 2013

1/ The implementation that was developed is exactly what was defined within the GateIn Spec for CDI Support. Is there a reason that it's only a problem now and not when the spec was reviewed?

2/ Yes the spec originally defined a destroy operation, but it turned out that for the purposes of CDI one was not needed. Also, the way I thought a destroy lifecycle callback would be done is that it would invoke the destroy listeners before the container did its own destroy, which would mean CDI would destroy all the beans on the portlet before portlet.destroy() had been called by the container. One way around that is to call portlet.destroy() as part of SHUTDOWN, as it is now, but then that would be confusing I feel.

Contributor

vietj commented Apr 2, 2013

I will work on this to make it work properly.

BTW your PR fails at passing testsuite unless you apply this commit https://github.com/vietj/gatein-pc/commit/19dbe7980de3a644954b6e71279e7e0e4f776f1c (PortletContainerImpl)

Contributor

kenfinnigan commented Apr 2, 2013

thanks Julien.

That's weird because I ran them and they worked fine, maybe I ran them wrong

Contributor

vietj commented Apr 2, 2013

I pushed this work in the master branch of my repo for the moment so you can check it works for you : https://github.com/vietj/gatein-pc . The usage should be mostly the same than previously. Let me know how this works.

Contributor

kenfinnigan commented Apr 3, 2013

I needed to add the following to provide access to the bits for CDI injection, and to override the portletdeployment creation within gatein-portal: kenfinnigan/gatein-pc@2ea56c7

Contributor

vietj commented Apr 3, 2013

Ok, I will cherry pick it and merge all this work in gatein-pc tomorrow.

Contributor

kenfinnigan commented Apr 4, 2013

Thanks Julien

Contributor

vietj commented Apr 4, 2013

Pushed

@vietj vietj closed this Apr 4, 2013

exo-swf pushed a commit to exoplatform/gatein-pc that referenced this pull request Mar 15, 2017

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