UI.getPushConfiguration.getTransport() *always* returns null

General
1

Hello everyone,

I noticed something peculiar when I called UI.getPushConfiguration().getTransport():
it
always
 returns null. I was curious about this, and I tried this even in a brand new Vaadin 7.3.9 project.

Here is the code from this new project (7.3.9, Tomcat 8.0.17, also tried with Tomcat 7, all with Java 8):

[code]
@SuppressWarnings(“serial”)
@Push(transport = Transport.WEBSOCKET, value = PushMode.AUTOMATIC)
public class EeeweUI extends UI {

@WebServlet(value = "/*", asyncSupported = true)
@VaadinServletConfiguration(productionMode = false, ui = EeeweUI.class)
public static class Servlet extends VaadinServlet { }

@Override
protected void init(VaadinRequest request) 
{
    System.out.println("Transport is: " + getPushConfiguration().getTransport());
    getPushConfiguration().setTransport(Transport.WEBSOCKET);
    System.out.println("Transport is: " + getPushConfiguration().getTransport());
}

}
[/code]Please notice the @Push annotation at the top specifying the Transport, as well as the code inside the init method programmtically specifying the Transport.

Both times, the output is null.

Here is the output on the console from running this:

Transport is: null
Transport is: null

Can anyone confirm this strange behavior?

Is this something that should be filed as a bug report?

Thank you,

Chris Boyd

2

I can confirm that the call returns null for me too, in an application where push with websocket works fine.

3

I have been stepping into this and found something very interesting, attached and shown in the screen shot:

The PushConfiguration.getTransport() method is hard-coded to return null in a catch block, and it is.

    /*
     * (non-Javadoc)
     * 
     * @see com.vaadin.ui.PushConfiguration#getTransport()
     */
    @Override
    public Transport getTransport() {
        try {
            return Transport
                    .valueOf(getParameter(PushConfigurationState.TRANSPORT_PARAM));
        } catch (IllegalArgumentException e) {
            return null;
        }
    }

What’s more interesting is the exception that’s being thrown:

java.lang.IllegalArgumentException: No enum constant com.vaadin.shared.ui.ui.Transport.websocket

and this exception is swallowed, and null is returned.

The interesting thing is what is happening here, and in the Transport class.

getParameter(PushConfigurationState.TRANSPORT_PARAM) is returning "
websocket
", not
WEBSOCKET
, which would be what is required for the Transport.valueOf code to work.

“websocket” comes from Transport enum as defined here:

public enum Transport {
    /**
     * Websockets
     */
    WEBSOCKET("websocket"),
    /**
     * HTTP streaming
     */
    STREAMING("streaming"),
    /**
     * HTTP long polling
     */
    LONG_POLLING("long-polling");

    private String identifier;

    private Transport(String identifier) {
        this.identifier = identifier;
    }

    public String getIdentifier() {
        return identifier;
    }

}

It’s the identifier, the one returned by getIdentifier().

This is why getParameter(PushConfigurationState.TRANSPORT_PARAM) returns “websocket” rather than “WEBSOCKET”.

If the Enum names in Transport matched the identifier, then there would be no issue.

I tested this by building a Vaadin Shared jar based off of 7.3.7, and changed the identifiers in Transport to match their enum names.

After rebuilding and relaunching, getPushConfiguration().getTransport() started returning WEBSOCKET appropriately.

Should I file a bug report with Vaadin for this?
17827.png

4

I reported this as a bug with Vaadin:
http://dev.vaadin.com/ticket/16499

I also created a Pull Request on GitHub against 7.4, to address the issue:
https://github.com/vaadin/vaadin/pull/23

5

Oops, thank you for the investigation!

The string identifiers are what the client-side Atmosphere recognizes and thus cannot be strictly equal to the enumeration symbols. Either getTransport should do the reverse mapping by hand or we should store the transport in a properly typed field instead of the “generic” map of push parameters.

6

Hi Johannes,

please look at the Git Pull request, where I addressed this by adding a static method to Transport to properly return the Transport enum based on it’s string identifier.

Here is the relevant method, and then PushConfigurationImpl was changed to call this method.

public static Transport getByIdentifier(String identifier) {
for (Transport t : values()) {
if (t.getIdentifier().equals(identifier)) {
return t;
}
}
return null;
}

I was informed by Artur that the correct way to submit changes is through the Gerrit system rather than Github, so I have spent some time today familiarizing myself with this and getting set up with Gerrit.

I am sync’ed up and almost ready to push the change, but I’m still trying to find the best place for the Unit Test for this.

7

I submitted the change and a Unit Test to the Gerrit system here:
https://dev.vaadin.com/review/#/c/7080/

UPDATE

The code has been merged, and the associated ticket has been closed:
http://dev.vaadin.com/ticket/16499

8

Thank you for the contribution!