REST (and possibly web) generator doesn't handle functions overload properly

[Geod24]

Pinging @Dicebot

import vibe.d;

@rootPathFromName
interface ITestAPI {
        string getBug();
        string getBug(string par);
        string getBug(int par);
}

class TestAPI : ITestAPI {
override:
        string getBug() { return "foo"; }
        string getBug(string par) { return "bar"; }
        string getBug(int par) { return "foobar"; }
}

shared static this()
{
        auto settings = new HTTPServerSettings;
        settings.port = 8080;
        settings.bindAddresses = ["::1", "127.0.0.1"];

        setLogLevel(LogLevel.debug_);
        auto routes = new URLRouter();
        registerRestInterface(routes, new TestAPI());

        listenHTTP(settings, routes);
        setTimer(1.seconds, () {
                        auto api = new RestInterfaceClient!ITestAPI("http://127.0.0.1:8080");
                        assert("foo" == api.getBug());
                        assert("bar" == api.getBug("Hi"));
                        assert("foobar" == api.getBug(42));
        });

Results in:

Match tree has 17 nodes, 3 terminals
REST call: GET http://127.0.0.1:8080/i_test_api/bug -> 200, "foo"
REST call: GET http://127.0.0.1:8080/i_test_api/bug?par=Hi -> 200, "foo"
Task terminated with unhandled exception: Assertion failure
Full error: core.exception.AssertError@source/app.d(42): Assertion failure

That's because overload will only work in the following conditions:

• Overloads don't resolve to the same templated path (i.e., when there are different path parameters);
• Overloads don't use the same method (i.e. with @property).

Will work on a solution.

[mihails-strasuns]

To fix that you would need to get list of all overloads and generate bunch of if conditions for them to match incoming args. Shouldn't be very difficult as basic iteration is already there - just something that slipped through originally.

[Geod24]

FYI (a bit off-topic though) : I found this bug when I was working on a way to provide the context of the request (e.g. the HTTPServerRequest/HTTPServerResponse). An example of the way I was heading can be found here: https://issues.dlang.org/show_bug.cgi?id=13830

w.r.t that bug, in addition to fixing the rest module, is there a reason why the router should not assert on this (as first route registered will always get matched) ?

[piyushchauhan2011]

Hi @Geod24 . I want to contribute to this project too. Can you give me suggestion, where to start ? I tried reading source code, but seems like a lot of code. Obviously I'm a beginner here in open source contribution. Will appreciate your help!

[Geod24]

Depends of what you want to do ;) I would suggest that you email me directly with which changes you have in mind, and we can discuss it.

[piyushchauhan2011]

@Geod24 I don't have any right now, is there anything open or needs work ?

[mihails-strasuns]

@piyushchauhan2011 do you want to contribute specifically to REST generator enhancements or vibe.d in general?

[piyushchauhan2011]

vibe.d in General :relieved: I've heard recent advancements in D programming language and I thought, this project would be a nice place to start testing D lang.

[mihails-strasuns]

There are 133 open issues right now in vibe.d : https://github.com/rejectedsoftware/vibe.d/issues Unfortunately those are not tagged by difficulty so finding easiest place to start may be tricky. Unless @s-ludwig has something in mind, I'd suggest to just go through the list until you notice something that feels doable to you.

[s-ludwig]

I don't have anything specific in mind, there are a few old issues that are not particularly difficult, such as #266 or #876, but the more recent ones are usually simpler to solve. So apart from those two, I'd rather look only at the issues from this year.

[s-ludwig]

@Geod24 w.r.t. URLRouter asserting that a route is only registered once; the router is especially designed so that routes can fall through to the next matching route, regardless if the route string is the same or different. This enables some very useful use cases.

BTW, since the data comes in as a string, overloads that are generally not ambiguous may very well be in the context of the REST interface. How would you know for example which of two overloads is the right one both accept two different UDTs with fromString methods. Checking for exceptions during the string conversion would be an option, but would also yield to completely different overloading rules than for normal D code.

Currently I tend to think that any possibly ambiguous overloads should be forbidden - in the simplest conservative approach, overloads could simply be enforced to always differ in their @path or @method. This would be a bit less convenient, but at least we avoid obscure behavior... these interface generators are already quite complex after all

[MartinNowak]

Currently I tend to think that any possibly ambiguous overloads should be forbidden - in the simplest conservative approach, overloads could simply be enforced to always differ in their @path or @method.

I agree, overloading by query params is a bad idea, because we're not dealing with typed data here.