vaadin / portlet

Portlet support for Vaadin Flow
https://vaadin.com
Other
2 stars 3 forks source link

IPC reception does not work for an invisible Portlet #171

Open enver-haase opened 4 years ago

enver-haase commented 4 years ago

When a portlet A is maximized (and hence portlet B is not being rendered), then portlet B does not receive the event that it should.

portletViewContext.fireEvent() and context.addEventChangeListener()

are the methods of impact here.

mehdi-vaadin commented 4 years ago

IMHO, it’s not a bug in Vaadin Portlet. When one portlet is maximized there is no other portlet on the page to communicate with. Even Java Portlet Specification says "Portlet events are not guaranteed to be delivered and thus the portlet should always work in a meaningful manner even if some or all events are not being delivered."

We can use onAttach in the view class to update the content of portlet B when it's visible again.

enver-haase commented 4 years ago

I don't think one should read it as "it does not have to be reliable, so we're allowed to not send any events at all, or only some, or even most -- whatever we do, we're good".

" What the JSR committee means here is that a portlet event is part of a request. Events don’t go into a queue where they can be collected if there’s trouble delivering them, and they don’t have any way of handshaking with their receivers to verify that they’ve been received. They simply get sent with the hope that someone out there is listening. " https://freecontent.manning.com/wp-content/uploads/inter-portlet-communication-using-portlet-2-0.pdf

So in case a listener is registered, then it should be able to get its event.

enver-haase commented 4 years ago

We can use onAttach in the view class to update the content of portlet B when it's visible again.

Does this mean you rely on a Portlet to be detached from the UI and then re-attached in order to update its internal view-state (e.g. a number of items in the database)? I think this is inherently wrong to do at that point; for example, when two Portlets are both in NORMAL window mode and Portlet A messages Portlet B via IPC (= event mechanism).

joheriks commented 4 years ago

I think this is more of a new feature, rather than a bug. The Portlet 3.0 JavaScript Hub IPC mechanism (which underlies current PortletViewContext::fireEvent and PortletViewContext::dispatchClientEvent implementations) does not forward events to hidden or minimized portlets at all (at least in the Pluto implementation). Note that we are not using the Portlet 2.0 event handling.

Nevertheless, it may be possible to build a custom event dispatching mechanism that would satisfy the requirement of forwarding events to Vaadin Portlets that have been rendered and subsequently hidden. This would come with at least the following caveats: 1) hidden non-Vaadin-portlets would not receive the events; and 2) a Vaadin Portlet that was hidden from the get-go (e.g. the portal page was loaded with another portlet maximixed) would not receive any events since the views would not yet be instantiated.

enver-haase commented 4 years ago

The Portlet 3.0 JavaScript Hub IPC mechanism (which underlies current the >PortletViewContext::fireEvent and PortletViewContext::dispatchClientEvent implementations)

And here lies the problem. In case there is nothing rendered, we need to deal with a pure Java management mechanism.

I would strongly oppose the second stanza's idea of a custom 'best effort' event dispatching mechanism due to the drawbacks mentioned.

As soon as Portlet is loaded and initialized (init() was called by the Portlet Container) and before it is destroyed (destroy() was called by the Portlet Container) it is a valid target for IPC events, e.g. if it registered for listening to them in its init().

It is possible that Portlet3.0/JavaScript added some complexity but as far as I can see what I outline above is a very minimal and somewhat easy-to-implement requirement, no? There surely is some kind of a "Listing and Launching API" all the Portlets are registered anyway, correct? So that one Portlet and/or the Portlet Container can control another Portlet.

Having said that. I wonder if the bug is in Vaadin Portlet or in Pluto. Or in interfacing the Bridge with Pluto.

enver-haase commented 4 years ago

https://github.com/vaadin/flow-and-components-documentation/blob/master/documentation/portlet-support/portlet-04-inter-portlet-communication.asciidoc

Note that we are not using the Portlet 2.0 event handling.

Is this another bug that I should open? The documentation clearly planned for

Or is this "just a" documentation issue and Portlet implementors can still use lower-level API (not in the Bridge) for those?

pleku commented 4 years ago

Or is this "just a" documentation issue and Portlet implementors can still use lower-level API (not in the Bridge) for those?

Sorry, the documentation is false in the sense that we are not going to add any fallback. We will fix it promptly. If there is any fallback, that would have to be a portlet container feature and not something that we implement by hand. But I think discussion about portlet 2.0 style events goes a bit wrong direction from the actual problem topic...

If I've understood properly 1) you have a portlet A on the page that is maximized 2) there is another portlet B on the page that is minimized 3) you want to trigger an action from the maximized portlet A to that will normalize/maximize the minimized portlet B and deliver some data to it

Right ? And the problem is that any event usage is not currently delivered to the portlet B when it is minimized ?

pleku commented 4 years ago

Or is this "just a" documentation issue and Portlet implementors can still use lower-level API (not in the Bridge) for those?

Yes IIRC the 2.0 events that you have to declare in the portlet.xml should just work independently.

But again, as the solution for the actual problem, we should be able to have the minimized portlet B (from previous message) in the page so that:

enver-haase commented 4 years ago



My current understanding is that 2.0-Style is probably the answer. IPC via JS is not the only option. According to Denis, the spec mandates an IPC event through the “action” method. I am trying to find the relevant part still though.

Es gibt doch nicht nur diesen Weg! In der Dokumentation steht, das dann ein IPC Event über die Action Methode geworfen werden muss. Leider komme ich mit den normalen mitteln nicht mit Vaadin an die Action Methode. Hierfür muss doch nur ein Weg geschaffen werden. Hat sich den keiner die Portlet Specfication 3.0 durchgelesen? Ich dachte auch, dass Vaadin an der Erstellung der SPEC mitgewirkt hat.

On 16. Jan 2020, at 15:07, Pekka Hyvönen notifications@github.com wrote:



Or is this "just a" documentation issue and Portlet implementors can still use lower-level API (not in the Bridge) for those?

Sorry, the documentation is false in the sense that we are not going to add any fallback. We will fix it promptly. If there is any fallback, that would have to be a portlet feature and not something that we implement by hand. But I think discussion about portlet 2.0 style events goes a bit wrong direction from the actual problem topic...

If I've understood properly

  1. you have a portlet A on the page that is maximized
  2. there is another portlet B on the page that is minimized
  3. you want to trigger an action from the maximized portlet A to that will normalize/maximize the minimized portlet B and deliver some data to it

Right ? And the problem is that any event usage is not currently delivered to the portlet B when it is minimized ?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/vaadin/portlet/issues/171?email_source=notifications&email_token=AB4BPZDTWKJHAVB2PIRHN3LQ6BSZFA5CNFSM4KFGIMI2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJEFSDY#issuecomment-575166735, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4BPZCPQ3LQEMRF6QBKHCLQ6BSZFANCNFSM4KFGIMIQ .

joheriks commented 4 years ago

Comment 1. I want to highlight a conceptual discrepancy between the Vaadin and basic Portlet 2.0 Event IPC that I think is relevant here. In Vaadin IPC, we assume that the recipient of an event is a Vaadin component (that implements PortletView). Since within the same session multiple component instances may be created for a single portlet (first the portlet may be added several times to a single portal page, and second a single portal page may be open in multiple browser windows), event contexts are indexed in the session with two keys: namespace (unique for each portlet view on the page and given by Pluto) and window name (unique for each browser window and given by Vaadin). When an event is sent using the Vaadin IPC and intercepted by a receiving Vaadin Portlet, we direct it to the correct component based on these two values. So an event can only be received by components on the same rendered page (same browser window), and contrapositively, we cannot use Vaadin Portlet IPC to send events to portlets rendered in other windows.

Now with Portlet 2.0 events, since these events can be delivered completely server-side, we do not have access to client-side information like window name. Consequently, while we can intercept the event in VaadinPortlet we can not know which component instance to deliver it to. While we could theoretically deliver it to all view components, this would lead to server and client-side desynchronization unless there is a push or poll mechanism making sure that these components update themselves consequently.

Before deciding what to fix, I believe that we need to first either 1) confirm the above assumption and consequently conclude that client-side event dispatching is required; or 2) invalidate the assumption in favor of delivering events to all portlet instances in the same session together with some suitable mechanism for ensuring that the client-side stays updated.

Comment 2. Regarding the client-side event delivery, Pluto seems to make a difference between minimized and hidden portlets. If the portlet is rendered minimized (i.e., VaadinPortlet:shouldRenderMinimized is overriden to return true), it can receive events when minimized. However when a portlet is hidden because another portlet is maximized, it becomes completely "deaf" already on the client side.

enver-haase commented 4 years ago

I am trying to fully understand your answer. While doing so, please

make sure that the target Portlet does not need to be a Portlet based on any version of Vaadin at all.

denis-anisimov commented 4 years ago

I'm not sure what to do with this ticket:

pleku commented 4 years ago

I think it is fair to say that this is blocked for now, as we need to understand better what is the use case.

make sure that the target Portlet does not need to be a Portlet based on any version of Vaadin at all.

Not sure what this is referring to @enver-haase ? 1) To the Vaadin IPC which uses the standard portlethub to send events, which requires that any portlet getting the event must also be in the page at the same time (this is how portlets work). This should not matter whether it is a Vaadin portlet or NON-Vaadin portlet 2) Portlet 2.0 style IPC ? Why it is needed ? If you're building new portlets, what problem are you trying to solve with the old style events that cause bad UX and can bring dependencies between portlets (can be avoided) ? 3) Something else ?

Please elaborate on the use case, what you're trying to achieve and why.

denis-anisimov commented 4 years ago

Until it's clarified:

pleku commented 4 years ago

should it have a bug label? should it be in the P1 column ?

Yeah you're right, maybe we'll move this to needs triage to be checked weekly for updates.

enver-haase commented 4 years ago

make sure that the target Portlet does not need to be a Portlet based on any version of Vaadin at all.

Not sure what this is referring to @enver-haase ?

I was merely saying that it must be possible to target any existing Portlet adhering to JSR-168 or JSR-286 or JSR-362 as an event receiver.

The reason is that we promised "Portlet Support Vaadin Flow support of JSR-286 (portlet 2.0 specification) for Pluto 3.0.1 , from the version 14/15 up, e.g. File upload, themes, Inter Portlet Coordination (IPC) etc."

As you asked what this was referring to: This is referring to test cases to be written, not any implementation part of Vaadin Bridge or Pluto, to verify the integration.

1. To the Vaadin IPC which uses the standard portlethub to send events, which requires that any portlet getting the event must also be in the page at the same time (this is how portlets work). This should not matter whether it is a Vaadin portlet or NON-Vaadin portlet

Is this how Portlets work in Pluto? I can see here https://www.ibm.com/developerworks/community/forums/html/threadTopic?id=77777777-0000-0000-0000-000014155072 that people see this behaviour "must be at the same page" as a problem that was fixed in WebSphere: https://www-01.ibm.com/support/docview.wss?uid=swg1PK71507 . If we can say the problem is not with Vaadin-Portlet but with Pluto, then that should be fine. However, a workaround would of course be appreciated.

This whole "must be on the same page" is new to me, but the problem identified had to do with an invisible Portlet, not one that was on another page.

The Portlethub according to https://portals.apache.org/pluto/v301/v3Features.html can be used for IPC, but this does not mean it has to be used or that using it is sufficient in all corner cases. It is merely convenient to use in my understanding. JSR-362 3.7.4 states that "Regardless of how the event phase is initiated, the portlet container can route the events to portlets able to receive them. The algorithm used for event routing is not defined in the Portlet Specification and is implementation specific."

According to one customer, it would help implementing the IPC via the Action Method ( https://portals.apache.org/pluto/portlet-3.0-apidocs/javax/portlet/annotations/ActionMethod.html ) -- and getting access to it (through our Bridge, if my understanding is correct).

JSR-362 22.1.1 we are doing? So the Portlethub was there but could not find the target Portlet?

As we are trying to communicate from Java to Java, I don't see why going "through the JavaScript client-side representation of Portlet A, the Portlethub, the JavaScript client-side representation of Portlet B which is missing" is our only option. JSR-362 22.1 says "The Portlet Specification does not describe how portlets not participating in the portlet hub client-side support are to be handled. However, the Portlet Specification recommends that the portlet hub initiates a page refresh if a portlet hub interaction results in a render state change for a portlet that does not participate in portlet hub client-side support. Otherwise, the non- participating portlet might display stale data." I reckon another sensible action is to send the event(s) through the receiver's ActionMethod (which I think was standard in 2.0 anyway?).

2. Portlet 2.0 style IPC ? Why it is needed ? If you're building new portlets, what problem are you trying to solve with the old style events that cause bad UX and can bring dependencies between portlets (can be avoided) ?

Just assuming there are hundreds if not thousands of existing precompiled JSR-286 Portlets and the customer would like a few of them on page together with a shiny new Vaadin Portlet, then those legacy portlets should be able to receive events. Likewise, the new Portlet should be able to listen to their events.

3. Something else ?

Please elaborate on the use case, what you're trying to achieve and why.

Sorry, we promised a full implementation and use cases are therefore somewhat academic.

Why? Because.

Cheers, Enver

enver-haase commented 4 years ago

"waiting for author"? @pleku do you need more input?