vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
618 stars 167 forks source link

RouteConfiguration.forSessionScope().getUrl() does not return scheme and domain name #13999

Open dmitrilc opened 2 years ago

dmitrilc commented 2 years ago

Description of the bug

The method RouteConfiguration.forSessionScope().getUrl() only returns the route and not a valid URL.

Expected behavior

Since the method RouteConfiguration.forSessionScope().getUrl() is named getUrl(), I expected it to return at least the scheme + host

https://datatracker.ietf.org/doc/html/rfc1738#section-3.3

Minimal reproducible example

@PageTitle("Static")
@Route(value = "static", layout = MainLayout.class)
@RouteAlias(value = "", layout = MainLayout.class)
@PermitAll
public class StaticView extends HorizontalLayout {

    private TextField name;
    private Button sayHello;

    public StaticView() {
        name = new TextField("Your name");
        sayHello = new Button("Say hello");
        sayHello.addClickListener(e -> {
            Notification.show("Hello " + name.getValue());
        });

        setMargin(true);
        setVerticalComponentAlignment(Alignment.END, name, sayHello);

        add(name, sayHello);

        String route = RouteConfiguration
                .forSessionScope()
                .getUrl(StaticView.class);

        System.out.println(route); //prints "static"
    }

}

Versions

tarekoraby commented 2 years ago

The method RouteConfiguration.forSessionScope().getUrl() only returns the route parameter and not a valid URL.

Just a small nitpick, it returns the route (the one we define using @Route), not the route parameter.

tarekoraby commented 2 years ago

And for the record, the scheme + host URL is already retrievable using UI.getCurrent().getPage().fetchCurrentURL(). I think it would be better to rename RouteConfiguration.getUrl() to RouteConfiguration.getRoute().

Artur- commented 2 years ago

In which case do you need the full URL?

The proxy case always complicates things where the user might be accessing the app through https://some.url/some/path and the server might be running on http://localhost:8080/something/else

tarekoraby commented 2 years ago

@dmitrilc can correct me if I'm wrong, but we are raising this issue from a purely academic standpoint. We just think that RouteConfiguration.getUrl() should be renamed to better indicate the returned value.

dmitrilc commented 2 years ago

@dmitrilc can correct me if I'm wrong, but we are raising this issue from a purely academic standpoint. We just think that RouteConfiguration.getUrl() should be renamed to better indicate the returned value.

That is correct.

dmitrilc commented 2 years ago

And for the record, the scheme + host URL is already retrievable using UI.getCurrent().getPage().fetchCurrentURL(). I think it would be better to rename RouteConfiguration.getUrl() to RouteConfiguration.getRoute().

There are already getRoute() methods in RouteConfiguration. Maybe it should be called getRouteString() or something similar.

Legioth commented 2 years ago

the route and not a valid URL

Not that it really matters in this case, but it could be noted that relative URLs are still valid URLs.