loopbackio / loopback-next

LoopBack makes it easy to build modern API applications that require complex integrations.
https://loopback.io
Other
4.95k stars 1.07k forks source link

DefaultCrudRepository and rest connector #2788

Closed mgabeler-lee-6rs closed 3 years ago

mgabeler-lee-6rs commented 5 years ago

Note: this should probably serve as a replacement for #1696 (@nikivanov is a coworker of mine).

Description / Steps to reproduce / Feature proposal

  1. Define a DataSource using the rest connector with the crud flag enabled
  2. Define a Model / Entity appropriate for the remote resource
  3. Define a Repostory using DefaultCrudRepository to connect the two
  4. Try to use any update-related method to modify a remote entity instance

Current Behavior

All update-related methods from DefaultCrudRepository seem to map to updateAll, which maps via loopback-datasource-juggler's dao to connector.update ... which the rest connector does not implement, and I don't think it really could in the general case.

Expected Behavior

It ought to be possible to bind a Model to a REST-connected datasource somehow.


It seems that updateById ought to map to updateAttributes at the connector level, or maybe save. Though save at the juggler bridge level maps to replaceById (which the rest connector also doesn't implement, though that might be possible to do).

Looking through the source, it seems that the semantics for updateAttributes and updateById are the same, so I get the feeling having the legacy bridge map update and updateById to updateAttributes instead of updateAll might work ... but that feels like it'd require lots of checking for regressions.


I think I can work around this locally by adding a method like this to my custom DataSource class:

async updateAttributes(
    id: ID,
    data: DataObject<T>,
    options?: Options,
): Promise<void> {
    const fakeInstance = new this.modelClass({[this.modelClass.definition.idName()]: id});
    try {
        await fakeInstance.updateAttributes(data, options);
    } catch (err) {
        if (err.statusCode === 404) {
            throw new EntityNotFoundError(this.entityClass, id);
        }
        throw err;
    }
}

But I'm not sure this is exactly a portable or ideal solution, even if it works for my particular service. One thing I notice, which is harmless for me but maybe not so much for others, is that this results in the id value getting included in the PUT body of attributes to update.

dhmlau commented 5 years ago

@mgabeler-lee-6rs, if you'd like to call another REST APIs using the REST connector, you might want to use Service instead of Repository. Please see this docs page: https://loopback.io/doc/en/lb4/Calling-other-APIs-and-web-services.html for more detailed instructions. I have this repo https://github.com/dhmlau/loopback4-external-apis as an example too.
Hope it helps.

mgabeler-lee-6rs commented 5 years ago

I've done that for other remote services for sure, where they implemented something other than a standard (LB) CRUD API. But in this case, where the other hand is in fact another LB service using standard CRUD setup, it seems that the boilerplate that seems to be designed to handle that ... ought to handle it?

mgabeler-lee-6rs commented 5 years ago

I found another problem with this setup. If you issue a find query with a filter, it doesn't map in a way that works with LB2/3's REST exposure, nor the default crud setup for LB4's cli generator.

If you issue find({where: ...}), it maps it to /Model?where... instead of /Model?filter.... It's a bit hard to write out since it uses "bracket" syntax in the query string instead of url-encoding the JSON object. For example, find({where: {x: 1}}) becomes /Model?where%5Bx%5D=1 instead of /Model?filter%5Bwhere%5D%5Bx%5D=1. The net result is that the entire filter parameter is completely ignored, always.

~I'm going to report this over in loopback-connector-rest, but it's going to take time to build a repro sandbox.~ Reported as https://github.com/strongloop/loopback-connector-rest/issues/136

bajtos commented 5 years ago

@raymondfeng IIRC, you are the author of the REST connector, could you please take a look and help?

tonychangcheng commented 5 years ago

I just you guys may have fixed my issue. I am tring to all the post external api. How do I create a datasource.json? How can I specify the payload for creation?

"operations": [ { "template": { "method": "GET", "url": "https://xxx/products/{productId}" }, "functions": { "getById": [ "productId" ] } }, { "template": { "method": "GET", "url": "xxx/products" }, "functions": { "getAll": [] } }, { "template": { "method": "POST", "url": "xxx/products" }, "functions": { "create": [ "product" ] } } ]

dhmlau commented 4 years ago

@tonychangcheng, i have created a sample app: https://github.com/dhmlau/loopback4-github-analytics2/blob/master/src/datasources/githubds.datasource.json. It's calling out GitHub API. See if it helps.

stale[bot] commented 3 years ago

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

stale[bot] commented 3 years ago

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.