Allow specification of navigation property name in `.ref` and `.removeRef`

[zhevron]

While looking into the API, I discovered that the .ref and .removeRef methods assume that the navigation property is always named the same as the entity set.

Say I have an API that has an entity set named People (just like the TripPinService example). Each of these people can have a set of people as their Friends.

It will always generate a request like this:

POST /odata/People('myusername')/People/$ref
{
    "@odata.id": "People('otherusername')"
}

When the request I really want is:

POST /odata/People('myusername')/Friends/$ref
{
    "@odata.id": "People('otherusername')"
}

The source in question can be seen here: https://github.com/janhommes/o.js/blob/b153570c37e5646e1fb7261be01d6ef483938ea1/o.js#L396-L416

Specifically:

resource.path.push({ resource: navPath, get: null });

and

var newResource = parseUri(navPath);

This tells me that the code assumes navPath will be the same for the navigation property and the entity set name.

I'm more than willing to fix this, but I'd like some input on what change would be more appropriate for the API. I have a couple of solutions in mind at the moment that would fix this:

1. Add another parameter to .ref and .removeRef that takes in the entity set name. You would then call the function like this:

   o('People(\'myusername\')').ref('Friends', 'People', 'otherusername')

2. Force the user to specify the navigation property name in the URL beforehand. In which case, the function call would look something like this:

   o('People(\'myusername\')/Friends').ref('People', 'otherusername')

[janhommes]

Did you test 2. and it did not work? I see, we basically need to remove line 408 or for make the second approach work, or?

I personally prefer the second solution, because I think also the following would be a valid $ref:

o('Product(1)').ref('Category', 2); --> POST /odata/Product(1)/$ref

Or isn't it?

[zhevron]

You will always need a navigation property when adding or removing references. In your example, the correct URL would most likely be POST /odata/Product(1)/Category/$ref

Relevant OData documentation: http://docs.oasis-open.org/odata/odata/v4.0/errata03/os/complete/part2-url-conventions/odata-v4.0-errata03-os-part2-url-conventions-complete.html#_Toc453752345

None of my proposed fixes work today because the .ref function applies the entity set name regardless.

o('People(1)/Friends').ref('People', 2); // POST /odata/People(1)/Friends/People/$ref

[janhommes]

Okay, then let us go with the second approach and document it right. We just need to take into account older odata versions. o.js supports v3 as well.

[zhevron]

I'll get on it then. :) It's actually the easier of the two solutions. I'll add some unit tests to go with it as well.

OData v3 also requires a navigation property name when using the $links convention. See: http://www.odata.org/documentation/odata-version-3-0/url-conventions/

[zhevron]

And I just noticed that supporting v3 here could be harder than I initially thought.

OData v4 URL:

http://services.odata.org/odata/Products(1)/Category/$ref

OData v3 URL:

http://services.odata.org/odata.svc/Products(1)/$links/Category

I suppose it could pop the last URL segment, push $links and then push the URL segment back to invert it, but it kinda feels a little dirty. Or should it just require that the user specifies the link?

o('Products(1)/$links/Category').ref('Category', 2)

[janhommes]

Okay, in this case, I would prefer the first solution, to have it consistent with all versions.

[zhevron]

Okay, so the API change is done and the tests added. However, the DELETE tests are failing for some weird reason. The URLs check out and they hit the correct endpoints in my production API, but the Trippin service seems to crash when using that request. Same happens when I issue the same request directly from Postman. I'll have to dig more into that.

Also, I'm not sure I 100% approve of the API change. Maybe the method should take 2 or 3 parameters? So that if the navigation property does differ in name, you need to specify it, otherwise it'll just use the entity set name?

For instance:

o('Product(1)').ref('Category', 2); // For when the name does not differ.
o('Category(1)').ref('Products', 'Product', 1); // For when the name does in fact differ.

This should also make it backwards compatible (if you care about that stuff).

[janhommes]

I like the change (+1 for backwards compatibility). If the delete $ref keeps failing on the TripPinService we might need to skip this test for now.

[zhevron]

I don't think these tests are going to work. I'll clear the WIP status and submit a suggestion for rewriting the unit tests to actually test the library and not some test service provided by someone else.

[janhommes]

So what is your suggestion on this? Should we wait for the mocked test or should I merge it in. I mean we don't do a version directly and I don't mind to merge it in, but for the CI build it would be good to skip the tests with xit.

[zhevron]

I can update the PR with skipped tests so it will pass. I can't see anything wrong with the generated request, yet the API keeps throwing 500 errors (even via manual postman requests).