nlf / mudskipper

a resourceful router plugin for hapi
15 stars 3 forks source link

Missing routes. #11

Open aq1018caa opened 10 years ago

aq1018caa commented 10 years ago
'use strict';

var Hapi = require('hapi');
var server  = new Hapi.Server('127.0.0.1', 3000);
var fn = function(req, reply) { reply('ok'); };

var resources = {
    services: {
        index: fn,
        show: fn,
        create: fn,
        update: fn,
        destroy: fn,
        hasMany: {
            nodes: {
                index: fn,
                hasMany: {
                    statuses: {
                        index: fn,
                        create: fn,
                    }
                }
            }
        },
    },

    nodes: {
        index: fn,
        show: fn,
        update: fn,
        destroy: fn
    },

    statuses: {
        index: fn,
        create: fn
    }
};

server.pack.require('mudskipper', { namespace: 'api', resources: resources }, function (err) {
    if(!err) { console.log('resources loaded.'); }
});

server.start(function (err) {
    if(!err) { console.log('server started.'); }
});

Run:

curl http://localhost:3000/api/services/1/nodes/2/statuses

Result:

{"statusCode":404,"error":"Not Found"}

Expected:

ok
aq1018caa commented 10 years ago

With a slightly different configuration, the /api/nodes route was gone in my own project. This is really strange...

aq1018caa commented 10 years ago

I think internals.resources is the culprit here. If a nested resource and a root level resource have the same name, either one will be overwritten depending on the parsing order. internals.resources need to be scope in order to take into account resources with the same name but with different scoping levels.

nlf commented 10 years ago

Ah, that would indeed be the exact issue. I never accounted for someone creating an in-place child definition with the same name as a top level resource.

I'll have to think about how to handle this..

nlf commented 10 years ago

Note that the normal way to configure this to get the behavior you're expecting (based on only using the index/create methods in the hasMany definitions) is more like:

var resources = {
    services: {
        index: fn,
        show: fn,
        create: fn,
        update: fn,
        destroy: fn,
        hasOne: 'nodes'
    },

    nodes: {
        index: fn,
        show: fn,
        update: fn,
        destroy: fn,
        hasOne: 'statuses'
    },

    statuses: {
        index: fn,
        create: fn
    }
};

This would generate routes for

aq1018caa commented 10 years ago

I see. However, in my use case the business logic and handlers for the two node resources differ. Although, semantically it is still the same node representation. In the scope of mudskipper, they are two separate resources because they have completely different handlers for their distinct business logics.

nlf commented 10 years ago

I’m working on a fix so that you can define things like you’re expecting. Stay tuned. --  Nathan LaFreniere From: aq1018caa aq1018caa Reply: nlf/mudskipper reply@reply.github.com Date: January 28, 2014 at 9:13:04 AM To: nlf/mudskipper mudskipper@noreply.github.com Subject:  Re: [mudskipper] Missing routes. (#11)
I see. However, in my use case the business logic and handlers for the two node resources differ. Although, semantically it is still the same node representation. In the scope of mudskipper, they are two separate resources because they have completely different handlers for their distinct business logics.

— Reply to this email directly or view it on GitHub.

aaronmccall commented 9 years ago

@nlf, this is the problem I was mentioning today.