Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for attribute functions to REST interfaces #349

Merged
merged 9 commits into from
Oct 23, 2013
Merged

Add support for attribute functions to REST interfaces #349

merged 9 commits into from
Oct 23, 2013

Conversation

mihails-strasuns
Copy link
Contributor

Actual change necessary for funcattr support is minimal (e6a98a9), which indicates that current vibe.utils.meta.funcattr design is quite solid. Some defects in original pull have been found and fixed (361005b)

However, there are also plenty of somewhat unrelated changes in vibe.http.rest as I couldn't pass by by without yet another attempt to make it more readable:

  1. adjusted to http://vibed.org/style-guide
  2. converted large example blocks in comments into documented unittests (which test nothing but are at least guaranteed to compile now)
  3. reduced those examples as there is now a much more complete one in test suite
  4. implemented a standard way for path formatting (always single "/" between nodes, no trailing "/" unless it is a nested interface index) and covered it with unittests

Each commit should be compilable on its own and can be reviewed independently.

Example of resulting API copied from test suite:

@rootPathFromName
interface Example5API
{
    import vibe.utils.meta.funcattr;

    @before!authenticate("user") @after!addBrackets()
    string getSecret(int num, User user);
}

User authenticate(HTTPServerRequest req, HTTPServerResponse res)
{
    return User("admin", true);
}

struct User
{
    string name;
    bool authorized;
}

string addBrackets(string result, HTTPServerRequest, HTTPServerResponse)
{
    return "{" ~ result ~ "}";
}

class Example5 : Example5API
{
    string getSecret(int num, User user)
    {   
        import std.conv : to; 
        import std.string : format;

        if (!user.authorized)
            return ""; 

        return format("secret #%s for %s", num, user.name);
    }   
}

static this()
{
    auto routes = new URLRouter();
    registerRestInterface(routes, new Example5());
    auto settings = new HTTPServerSettings();
    settings.port = 8080;
    settings.bindAddresses = ["::1", "127.0.0.1"];
    listenHTTP(settings, routes);

    setTimer(dur!"seconds"(1), {
        scope(exit)
            exitEventLoop(true);
        auto api = new RestInterfaceClient!Example5API("http://127.0.0.1:8080");
        auto secret = api.getSecret(42, User.init);
        assert(secret == "{secret #42 for admin}");
    }
}

@mihails-strasuns
Copy link
Contributor Author

I am unlikely to do any coding in next 4 or 5 days so please don't hurry with reviewing and merging this ;)

@s-ludwig
Copy link
Member

Sorry, I interfered with the pull request when fixing two small regressions. But it looks good AFAICS and I'll test and merge in the evening today. The only thing is the "index" deprecation that I'd keep for a little longer.

DDOX will also need to get proper support for documented unit tests (not sure what DMD writes to the JSON file in that case). But this needs to be done anyway, of course.

@mihails-strasuns
Copy link
Contributor Author

P.S. Are there any additional documentation updates needed somewhere? (I have never paid much attention to vibed.org update process)

@s-ludwig
Copy link
Member

Since there is no high level (non-DDOC) documentation for the rest module, nothing needs to be updated currently. Otherwise it would be in docs.dt.

My brief tests have shown no issues, so this is good to be merged. Can you issue a rebase of your branch? In case you get a conflict, my only change was in the first line of adjustMethodStyle to exchange assert(name.length > 0); with if (name.length == 0) return null;, so you can basically discard my changes in favor of yours.

Dicebot added 3 commits October 21, 2013 17:14
Change DDOC example for registerRestInterface into documented unit-test, reformat it for better readability.
Add method for URLRouter to check registered routes.
Implement standard way for URL formatting using stand-alone function
@mihails-strasuns
Copy link
Contributor Author

Since there is no high level (non-DDOC) documentation for the rest module, nothing needs to be updated currently.

Maybe I should add one? It has got rather complex...

Rebased and also removed index deprecation commit for now.

Dicebot added 6 commits October 21, 2013 18:27
Adapt everything to vibe.d standard code style
Localize imports
Improve formatting where it helps readability
1) Add new calling overload in AttributedFunction so that it won't
try merging parameter and attribute type tuples when incoming and
target parameter type tuples match exactly. In that cases relevant
parameters are simply overwritten by attached attribute functions.

2) Add diagnostic that all attached InputAttribute functions conform
to stored argument list.

3) static imports added for qualified symbols
Also makes test suite exit upon both success and failure
s-ludwig added a commit that referenced this pull request Oct 23, 2013
Add support for attribute functions to REST interfaces
@s-ludwig s-ludwig merged commit 43370be into vibe-d:master Oct 23, 2013
@s-ludwig
Copy link
Member

Maybe I should add one? It has got rather complex...

It definitely makes sense to have a section for this, but I'd keep it relatively high level there (since that page is something more like a quick overview of everything).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants