fortunejs / fortune

Non-native graph database abstraction layer for Node.js and web browsers.
http://fortune.js.org
MIT License
1.45k stars 135 forks source link

Incomplete implementation of RFC 6902 #37

Closed shaunatgit closed 9 years ago

shaunatgit commented 10 years ago

Is there a reason why only the "replace" operation of RFC 6902 was implemented in Fortune? The lack of add is preventing many-to-many operations from being possible.

If I have two users and one group and add both users to the group, the users each belong to one group and the group has two users. This is one-to-many.

If I create another group and try to add that group to one of the users, the new group should be added to to the specified user. Basically, we'd end up with the following: many-to-many relationship:

User A ....Group A ....Group B User B ....Group A Group A ....User A ....User B Group B ....User A

Instead, with only "replace", the PATCH will replace the old group for that user and each user will end up belonging to one group (different groups) and each group has one user. i.e.

User A ....Group B User B ....Group A Group A ....User B Group B ....User A

What I need to be able to do is PATCH add.

At this very moment I am looking at the code trying to determine the best way to create support for the "add" and "remove" operations of RFC 6902. If you or anyone else has already looked into this, I'd sincerely appreciate knowing about any gotchas or algorithmic limitations sooner than later. Thanks!

gr0uch commented 10 years ago

Yes, this is very much intentional, because of the JSON API specification:

For attributes, only the replace operation is supported at the current time.

http://jsonapi.org/format/

At this point you might be thinking "well fuck the specification." And I might agree with you. But I hope that a lot of the value in using Fortune vs. creating an API with any other framework is implementing all of the standard assumptions that JSON API makes for free.

Also:

The lack of add is preventing many-to-many operations from being possible.

Not true, you can simply replace the many array. I know it's not efficient, but it works. Are you sure you have setup a many-to-many relationship in your resouce definition, and not a one-to-many?

shaunatgit commented 10 years ago

Thanks for the fast reply. Perhaps I'm not doing it right. I'm still pretty new to Fortune and the JSON API spec, so perhaps this is a noob error. Here is my Fortune script (it's REALLY simple):

var fortune = require('fortune')
  , app = fortune({
    db: 'bcat',
    flags: {
      inMemoryOnly: true
    }
  })
  .resource('user', {
    username: String,
    fullname: String,
    uid: Number,
    groups: ['group'] // "has many" relationship to groups
  })
  .resource('group', {
    groupname: String,
    gid: Number,
    //owner: 'person' // "belongs to" relationship to a person
    users: ['user'] // "has many" relationship to a users
  })
  .listen(1337);

To test it I'm using curl. Here is the output of my test script:

=========================================================

[ dump /users ]
GET /users
 ==> response payload:
{
  "users": []
}
 ==> response status: 200

[ dump /groups ]
GET /groups
 ==> response payload:
{
  "groups": []
}
 ==> response status: 200

=========================================================

[ add users user_A and jdoe ]
POST /users 
{
    "users": [
        {
            "username":"user_A",
            "fullname":"User A",
            "uid":"6358"
        },
        {
            "username":"jdoe",
            "fullname":"John Doe",
            "uid":"9999"
        }
    ]
}
 ==> response payload:
{
  "users": [
    {
      "id": "M6dM6lSuSNELmjWJ",
      "username": "user_A",
      "fullname": "User A",
      "uid": 6358
    },
    {
      "id": "xQQkhY6Teg11B0xP",
      "username": "jdoe",
      "fullname": "John Doe",
      "uid": 9999
    }
  ]
}
 ==> response status: 201

=========================================================

[ add user user_B ]
POST /users 
{
    "users": [
        {
            "username":"user_B",
            "fullname":"User B",
            "uid":"6995"
        }
    ]
}
 ==> response payload:
{
  "users": [
    {
      "id": "ApDN6YqIkvovPrY3",
      "username": "user_B",
      "fullname": "User B",
      "uid": 6995
    }
  ]
}
 ==> response status: 201

[ dump /users ]
GET /users
 ==> response payload:
{
  "users": [
    {
      "id": "ApDN6YqIkvovPrY3",
      "username": "user_B",
      "fullname": "User B",
      "uid": 6995
    },
    {
      "id": "M6dM6lSuSNELmjWJ",
      "username": "user_A",
      "fullname": "User A",
      "uid": 6358
    },
    {
      "id": "xQQkhY6Teg11B0xP",
      "username": "jdoe",
      "fullname": "John Doe",
      "uid": 9999
    }
  ]
}
 ==> response status: 200

=========================================================

[ delete /users/jdoe ]
[ locate id for jdoe ]
FindOne jdoe in /users
 ==> id="xQQkhY6Teg11B0xP"

DELETE /users/xQQkhY6Teg11B0xP
name: user
> resource: {"id":"xQQkhY6Teg11B0xP","username":"jdoe","fullname":"John Doe","uid":9999}
> request.body: {}
> response.body: undefined
resource: {"id":"xQQkhY6Teg11B0xP","username":"jdoe","fullname":"John Doe","uid":9999}
 ==> response status: 204

[ dump /users ]
GET /users
 ==> response payload:
{
  "users": [
    {
      "id": "ApDN6YqIkvovPrY3",
      "username": "user_B",
      "fullname": "User B",
      "uid": 6995
    },
    {
      "id": "M6dM6lSuSNELmjWJ",
      "username": "user_A",
      "fullname": "User A",
      "uid": 6358
    }
  ]
}
 ==> response status: 200

=========================================================

[ add group group_A ]
POST /groups 
{
    "groups": [
        {
            "groupname":"group_A",
            "gid":"1234"
        }
    ]
}
 ==> response payload:
{
  "groups": [
    {
      "id": "zidLhNaAe1Qaq9W4",
      "groupname": "group_A",
      "gid": 1234
    }
  ]
}
 ==> response status: 201

[ dump /groups ]
GET /groups
 ==> response payload:
{
  "groups": [
    {
      "id": "zidLhNaAe1Qaq9W4",
      "groupname": "group_A",
      "gid": 1234
    }
  ]
}
 ==> response status: 200

=========================================================

[ add group_A group to user user_A ]
[ locate id for user user_A ]
FindOne user_A in /users
 ==> id="M6dM6lSuSNELmjWJ"

[ locate id for group group_A ]
FindOne group_A in /groups
 ==> id="zidLhNaAe1Qaq9W4"

PATCH /users/M6dM6lSuSNELmjWJ 
[
  { "op": "add", "path": "/users/M6dM6lSuSNELmjWJ/links/groups", "value": ["zidLhNaAe1Qaq9W4"] }
]
 ==> response payload:
{
  "users": [
    {
      "id": "M6dM6lSuSNELmjWJ",
      "username": "user_A",
      "fullname": "User A",
      "uid": 6358,
      "links": {
        "groups": [
          "zidLhNaAe1Qaq9W4"
        ]
      }
    }
  ]
}
 ==> response status: 200

[ dump /users ]
GET /users
 ==> response payload:
{
  "users": [
    {
      "id": "ApDN6YqIkvovPrY3",
      "username": "user_B",
      "fullname": "User B",
      "uid": 6995
    },
    {
      "id": "M6dM6lSuSNELmjWJ",
      "username": "user_A",
      "fullname": "User A",
      "uid": 6358,
      "links": {
        "groups": [
          "zidLhNaAe1Qaq9W4"
        ]
      }
    }
  ]
}
 ==> response status: 200

[ dump /groups ]
GET /groups
 ==> response payload:
{
  "groups": [
    {
      "id": "zidLhNaAe1Qaq9W4",
      "groupname": "group_A",
      "gid": 1234,
      "links": {
        "users": [
          "M6dM6lSuSNELmjWJ"
        ]
      }
    }
  ]
}
 ==> response status: 200

=========================================================

[ add group_A group to user user_B ]
[ locate id for user user_B ]
FindOne user_B in /users
 ==> id="ApDN6YqIkvovPrY3"

[ locate id for group group_A ]
FindOne group_A in /groups
 ==> id="zidLhNaAe1Qaq9W4"

PATCH /users/ApDN6YqIkvovPrY3 
[
  { "op": "add", "path": "/users/ApDN6YqIkvovPrY3/links/groups", "value": ["zidLhNaAe1Qaq9W4"] }
]
 ==> response payload:
{
  "users": [
    {
      "id": "ApDN6YqIkvovPrY3",
      "username": "user_B",
      "fullname": "User B",
      "uid": 6995,
      "links": {
        "groups": [
          "zidLhNaAe1Qaq9W4"
        ]
      }
    }
  ]
}
 ==> response status: 200

[ dump /users ]
GET /users
 ==> response payload:
{
  "users": [
    {
      "id": "ApDN6YqIkvovPrY3",
      "username": "user_B",
      "fullname": "User B",
      "uid": 6995,
      "links": {
        "groups": [
          "zidLhNaAe1Qaq9W4"
        ]
      }
    },
    {
      "id": "M6dM6lSuSNELmjWJ",
      "username": "user_A",
      "fullname": "User A",
      "uid": 6358,
      "links": {
        "groups": [
          "zidLhNaAe1Qaq9W4"
        ]
      }
    }
  ]
}
 ==> response status: 200

[ dump /groups ]
GET /groups
 ==> response payload:
{
  "groups": [
    {
      "id": "zidLhNaAe1Qaq9W4",
      "groupname": "group_A",
      "gid": 1234,
      "links": {
        "users": [
          "M6dM6lSuSNELmjWJ",
          "ApDN6YqIkvovPrY3"
        ]
      }
    }
  ]
}
 ==> response status: 200

=========================================================

[ add group group_B ]
POST /groups 
{
    "groups": [
        {
            "groupname":"group_B",
            "gid":"2345"
        }
    ]
}
 ==> response payload:
{
  "groups": [
    {
      "id": "0ZvenzWbIepCQRMz",
      "groupname": "group_B",
      "gid": 2345
    }
  ]
}
 ==> response status: 201

[ dump /groups ]
GET /groups
 ==> response payload:
{
  "groups": [
    {
      "id": "0ZvenzWbIepCQRMz",
      "groupname": "group_B",
      "gid": 2345
    },
    {
      "id": "zidLhNaAe1Qaq9W4",
      "groupname": "group_A",
      "gid": 1234,
      "links": {
        "users": [
          "M6dM6lSuSNELmjWJ",
          "ApDN6YqIkvovPrY3"
        ]
      }
    }
  ]
}
 ==> response status: 200

=========================================================

[ add group_B group to user user_B ]
[ locate id for user user_B ]
FindOne user_B in /users
 ==> id="ApDN6YqIkvovPrY3"

[ locate id for group group_B ]
FindOne group_B in /groups
 ==> id="0ZvenzWbIepCQRMz"

PATCH /users/ApDN6YqIkvovPrY3 
[
  { "op": "add", "path": "/users/ApDN6YqIkvovPrY3/links/groups", "value": ["0ZvenzWbIepCQRMz"] }
]
 ==> response payload:
{
  "users": [
    {
      "id": "ApDN6YqIkvovPrY3",
      "username": "user_B",
      "fullname": "User B",
      "uid": 6995,
      "links": {
        "groups": [
          "0ZvenzWbIepCQRMz"
        ]
      }
    }
  ]
}
 ==> response status: 200

[ dump /users ]
GET /users
 ==> response payload:
{
  "users": [
    {
      "id": "ApDN6YqIkvovPrY3",
      "username": "user_B",
      "fullname": "User B",
      "uid": 6995,
      "links": {
        "groups": [
          "0ZvenzWbIepCQRMz"
        ]
      }
    },
    {
      "id": "M6dM6lSuSNELmjWJ",
      "username": "user_A",
      "fullname": "User A",
      "uid": 6358,
      "links": {
        "groups": [
          "zidLhNaAe1Qaq9W4"
        ]
      }
    }
  ]
}
 ==> response status: 200

[ dump /groups ]
GET /groups
 ==> response payload:
{
  "groups": [
    {
      "id": "0ZvenzWbIepCQRMz",
      "groupname": "group_B",
      "gid": 2345,
      "links": {
        "users": [
          "ApDN6YqIkvovPrY3"
        ]
      }
    },
    {
      "id": "zidLhNaAe1Qaq9W4",
      "groupname": "group_A",
      "gid": 1234,
      "links": {
        "users": [
          "M6dM6lSuSNELmjWJ"
        ]
      }
    }
  ]
}
 ==> response status: 200

Thanks!

gr0uch commented 10 years ago

Can you explain how trying to add in a PATCH request works for you? It isn't even supported: https://github.com/daliwali/fortune/blob/master/lib/route.js#L446

I guess to answer your question, you would include all of the groups of user_B in your last step:

[
  { "op": "replace", "path": "/users/0/links/groups", "value": ["zidLhNaAe1Qaq9W4", "0ZvenzWbIepCQRMz"] }
]

Note that the path should actually be /users/0/links/groups, not the ID, it actually refers to the JSON structure and not a URL. The examples in the JSON API documentation reflect this.

shaunatgit commented 10 years ago

Yeah, I was looking at the same code. After playing with the code in the routes.js earlier today, I came to the conclusion my requirements were too significant a deviation from the JSON API to implement server-side. Instead, I would have to do something client-side, like get the document to get the current IDs then append the patch with all of the groups, just as you indicated. This works fine, but it requires the client to do more and is far from being automatic population.

PATCH /users/VOKXH0cE8i5WDjAS 
[
  { "op": "add", "path": "/users/0/links/groups", "value": ["YWJ7JRKjgyBwglGT","oMjfspJyNnMjbnkp"] }
]
 ==> response payload:
{
  "users": [
    {
      "id": "VOKXH0cE8i5WDjAS",
      "username": "user_B",
      "fullname": "User B",
      "uid": 6995,
      "links": {
        "groups": [
          "YWJ7JRKjgyBwglGT",
          "oMjfspJyNnMjbnkp"
        ]
      }
    }
  ]
}
 ==> response status: 200

[ dump /users ]
GET /users
 ==> response payload:
{
  "users": [
    {
      "id": "N5WBfi7PmTUimPrB",
      "username": "user_A",
      "fullname": "User A",
      "uid": 6358,
      "links": {
        "groups": [
          "YWJ7JRKjgyBwglGT"
        ]
      }
    },
    {
      "id": "VOKXH0cE8i5WDjAS",
      "username": "user_B",
      "fullname": "User B",
      "uid": 6995,
      "links": {
        "groups": [
          "YWJ7JRKjgyBwglGT",
          "oMjfspJyNnMjbnkp"
        ]
      }
    }
  ]
}
 ==> response status: 200

[ dump /groups ]
GET /groups
 ==> response payload:
{
  "groups": [
    {
      "id": "YWJ7JRKjgyBwglGT",
      "groupname": "group_A",
      "gid": 1234,
      "links": {
        "users": [
          "N5WBfi7PmTUimPrB",
          "VOKXH0cE8i5WDjAS"
        ]
      }
    },
    {
      "id": "oMjfspJyNnMjbnkp",
      "groupname": "group_B",
      "gid": 2345,
      "links": {
        "users": [
          "VOKXH0cE8i5WDjAS"
        ]
      }
    }
  ]
}
 ==> response status: 200

Thank you very much for all of your assistance. You've done a great job implementing the JSON API and you're very responsive as well. You may close this issue.

jokeyrhyme commented 10 years ago

@steveklabnik says we should just use RFC 6902, but doesn't indicate whether or not everything there will eventually be part of the JSON API specification: https://github.com/json-api/json-api/issues/167

PATCH "add" and "remove" support will be super efficient when combined with MongoDB's atomic operations. :)

steveklabnik commented 10 years ago

Re-opening over there due to misunderstanding. Stay tuned.