janhommes / o.js

o.js - client side oData lib.
https://janhommes.github.io/o.js/example/
MIT License
238 stars 57 forks source link

Endpoint not considered #78

Closed emaborsa closed 5 years ago

emaborsa commented 5 years ago

I have a web page where have to call different services, so in the beginning I configure o followings: o().config({ autoFormat: false, version: 3, })

The following GET works: o(myUr/myEntity).get()

The following POST also works: o(myUrl/myEntity) .post(data) .save(onSuccess, onError)

The following PUT/PATCH does not work: o(myUrl/myEntity(theEntityId)) .patch(data) .save(onSuccess, onError)

I get a 404 with the wrong url. It takes the window.location.position instead the myUrl + '/myEntity(theEntityId)'

I debugged the o.js library v0.4.0, the problem seems to be on line 957: var endpoint = base.oConfig.endpoint + (endsWith(base.oConfig.endpoint, '/') ? '' : '/') + '$batch'; where base.oConfig.endpoint is null.

In the other scenario, configuring at the beginning as follows, it works fine: o().config({ endpoint: 'myUrl', autoFormat: false, version: 3, })

On line 120: function oData(res, config) { the passed res variable seems not be used.

Edit:

Sorry, I forgot that I use the batch as follows: o(myUrl/myEntity(theEntityId)) .patch(data) .batch(myUrl/myEntity(anotherEntityId)) .patch(anotherData) .save(onSuccess, onError)

janhommes commented 5 years ago

That is an issue. I guess it is on the lines 500-506. We try to be too smart here. Can you try to remove that line and see if it is working?

emaborsa commented 5 years ago

Do you mean the following block? if (resource.method === 'GET' && resource.data !== null) { var newResource = deepCopy(resource); //set the method and data newResource.method = 'PATCH'; newResource.data = resource.data; addNewResource(newResource); }

I think you mean another line, since in a PATCH scenario resource.method === 'GET' is never true. However, no it doesn't work....

janhommes commented 5 years ago

yes.

So the issue is that it never should get to 957 because that is only for BATCH request, which means which should only happen if you do multiple put or post without saving. So somehow you have more then one request pending.

Can you run the put in isolation without any other get/post/something? Does it work then?

emaborsa commented 5 years ago

Sorry, little mistake. I forgot to say that I use the batch. I updated the question. However, yes if I use a single command as PUT or PATCH it works.

Another unclear point, on line 1010 the variable routeName variable set to res. routeName does not exist in the entire script...

janhommes commented 5 years ago

yes, the routeName can be removed, that's right.

I guess there is nothing we can do about the endpoint. All Batch requests are sent to one endpoint, therefore it is not possible to define multiple URLs and that is why we just take into account the oConfig.endpoint. Which url would you suggest to use, the first one?

emaborsa commented 5 years ago

Well, I think the library should use the oConfig.endpoint as standard. IF another url is passed, it should be taken but not override the base one. Otherwise IMHO there should be a null check on the endpoint and throw an error when it is.

However, In my case I dont't have a different url, I did not define the url in the initial config but pass it as parameter instead.

Edit: I removed the initial config and edited the default values in the o.js in order to make tests. The batch does not work, it behavies as the other scenario. I guess/fear the batch does not work at all. Do you have a working example?

Edit2: I found a workaround. Everytime i do a call o() e reconfig it as follows: o().config({ autoFormat: false, endpoint: myUrl, version: 3, })

janhommes commented 5 years ago

should be fixed by: #83