strongloop / loopback-cli

LoopBack CLI tool for creating projects, models and more.
Other
105 stars 41 forks source link

New remote method generator removing matching named methods in other models #20

Open WoodyWoodsta opened 7 years ago

WoodyWoodsta commented 7 years ago

Bug or feature request

Repro steps of bug

Expected result

AFAIK, remote methods with the same name but in different models (ie. resulting in different URL paths) are valid.

Actual result (if bug)

Looback remote method generator is removing a remote method from any model upon generating a second one with the same name.

Additional information (Node.js version, LoopBack version, etc)

Loopback: 2.0.0 (generator-loopback@3.0.0) Node: v6.10.0

Update

This appears to also occur when performing any cli operation in a project with two remote methods with the same name (manually generated)

idushes commented 7 years ago

+1

bajtos commented 7 years ago

Nice catch. I suspect this is a bug in loopback-workspace and the way how it creates unique id for remote methods. See

@WoodyWoodsta @idushes would you like to contribute the fix yourself? I am happy to help you along the way, just mention my GH handle in the pull request description.

WoodyWoodsta commented 7 years ago

@bajtos If I manage to find time for it, I will certainly have a look.

karanssj4 commented 7 years ago

@bajtos from what i can make out in model-method.json, id refers to the name of the method hence it can't be repeated If we keep the id unique and introduce a new property, say 'name' then we wont need unique names, but i think that would introduce the name field in model definition json too due to this https://github.com/strongloop/loopback-workspace/blob/d9431392d4fcbf355a7e8f3765c201df41398e7a/common/models/model-definition.json#L97-L105 can we have a middleware that rewrites the method key in model definition file as the 'name' field or can we somehow mention in the model definition here https://github.com/strongloop/loopback-workspace/blob/d9431392d4fcbf355a7e8f3765c201df41398e7a/common/models/model-definition.json#L97-L105 that embed it against using the key 'name' and not 'id'

otherwise there might be some breaking changes and same method names in different models work fine in the projects currently

bajtos commented 7 years ago

Hello, I am afraid I don't have enough time to look into this in details. The value used for the id property is usually composed from multiple parts, I'd expect that a remote-method id should be something like {facet}.{model}.{method}, e.g. common.User.findById. A name property should already exist on all workspace models, IIRC it's inherited from WorkspaceEntity or Definition base model.

I don't think a middleware based approach would work, we have many places that's invoking the JavaScript API directly, loopback-cli being a prime example (via generator-loopback).