search

strongloop / strong-remoting

Communicate between objects in servers, mobile apps, and other servers.
www.strongloop.com
Other
105 stars 93 forks source link

Allow both object and array values for an argument (create/createMany) #360

Closed davidcheung closed 8 years ago

davidcheung commented 8 years ago

continuing the conversation for this comment https://github.com/strongloop/strong-remoting/pull/347#issuecomment-246680808

Since #347, strong-remoting rejects array values for arguments of type object. As a result, it's no longer possible to send an array of objects via POST /api/my-model-/create and expect the server to create them all in a batch mode.

Background

with https://github.com/strongloop/strong-remoting/blob/master/3.0-RELEASE-NOTES.md#conversion-and-coercion-of-input-arguments Array values are not allowed for arguments of type object. https://github.com/strongloop/loopback/blob/ed8b651b988799b5b42289bddd9f568398776bc8/lib/persisted-model.js#L643

Why its a problem

I think the biggest breaking change brought from this would be the ability to do batch POST operations

Possible options

@strongloop/loopback-dev

bajtos commented 8 years ago

Since #347, strong-remoting rejects array values for arguments of type object. As a result, it's no longer possible to send an array of objects via LoopBack's built-in POST /api/my-model-/create and expect the server to create them all in a batch mode.

Option 1 add flag to allowArrays for object coercion

I think the discussion in #347 reached an agreement that a union type ("object" or "array") is better than "allowArrays" flag.

In that light, I am proposing to go with Option 3, as it provides API that's easier to describe in Swagger: Create POST /createMany and take array of objects

As for Option 2, change type of persistetedModel methods that supports batch operations to 'any', I am afraid this would break the swagger description completely.

@richardpringle @Amir-61 @STRML any opinions?

richardpringle commented 8 years ago

Type JSON!

There should be a type that accepts any valid JSON which is pretty much the same as saying "array or object"!

I do not think that more endpoints is the answer, especially just to deal with strong-remoting types... the function is the exact same yet we have two endpoints because we have to allow two different input types.

I think that the default Persisted model REST API should be simple and minimalist. Adding a new endpoint to solve a problem should be an absolute last resort.

richardpringle commented 8 years ago

... on second thought, I guess any is the same thing

STRML commented 8 years ago

Union types are great, but that also could make coercion pretty difficult - needs to be separation of type validation and coercion, which I believe there now is in the latest remoting beta.

In that vein, I can't recommend https://github.com/gcanti/tcomb more for type validation - we've been using it for some internals and it really can express just about anything.

Amir-61 commented 8 years ago

IMO option 1 would be the ideal solution but using union ("object", "array"); however, I'm not very familiar with Swagger and its restriction, hence I cannot comment on that unfortunately. I would preferably avoid introducing a new endpoint like POST / createMany.

bajtos commented 8 years ago

There should be a type that accepts any valid JSON which is pretty much the same as saying "array or object"! ... on second thought, I guess any is the same thing

Yes, any accepts any value that can be encoded in JSON. Not only arrays and objects, but also numbers and strings.

option 1 would be the ideal solution but using union ("object", "array"); however, I'm not very familiar with Swagger and its restriction, hence I cannot comment on that unfortunately. I would preferably avoid introducing a new endpoint like POST / createMany.

Swagger is using a subset of JSON Schema Validations. Unfortunately the keyword oneOf (one of "array", "object") is not supported by Swagger/OpenAPISpec (yet), see https://github.com/OAI/OpenAPI-Specification/issues/57

In that light, I think our best option now is to implement allowArrays as a short-term fix to get back the feature that's already available in LoopBack 2.x. When Swagger 3.0 is released with support for oneOf, we can revisit and implement a better solution.

In that vein, I can't recommend https://github.com/gcanti/tcomb more for type validation - we've been using it for some internals and it really can express just about anything.

tcomb looks interesting! I can imagine we could use it in both strong-remoting and loopback-datasource-juggler to unify the type system(s) used in LoopBack. cc @raymondfeng @strongloop/loopback-dev

Cyperwu commented 7 years ago

I tried to add allowArray flag in my own defined remote method and it didn't work. Is there any other way to add this in my remote method?

bajtos commented 7 years ago

@Cyperwu please open a new issue and provide us with an example showing what you are trying to do, ideally a small app reproducing the problem - see http://loopback.io/doc/en/contrib/Reporting-issues.html#bug-report.