upsert option with ID remapping is broken

[sinedied]

Steps to reproduce

See https://glitch.com/edit/#!/feathers-id-issue?path=README.md:3:60 for source code and repro:

• feathers g app with REST & NeDB option
• feathers g service -> scores with NeDB option
• add hook to scores.hooks.js to enable upsert for all PUT/PATCH requests
• add id remapping to playerId (or whatever prop) in scores.services.js
• try adding a new entry with a PUT: curl -H 'Content-Type: application/json' --request PUT -d '{"playerId":1,"score":123}' https://feathers-id-issue.glitch.me/scores --> ERROR

Expected behavior

Enabling the NeDB upsert option works with id option set on the model.

Actual behavior

When enabling the NeDB upsert option with id option set on the model, trying to perform an upsert results in an error:
{"name":"BadRequest","message":"You can not replace multiple instances. Did you mean 'patch'?","code":400,"className":"bad-request","errors":{}}

Without the id remapping, it works fine.

System configuration

Tell us about the applicable parts of your setup.

Module versions (especially the part that's not working):

 "dependencies": {
    "@feathersjs/configuration": "^2.0.5",
    "@feathersjs/errors": "^3.3.5",
    "@feathersjs/express": "^1.3.0",
    "@feathersjs/feathers": "^3.3.0",
    "compression": "^1.7.3",
    "cors": "^2.8.5",
    "feathers-nedb": "^4.0.0",
    "helmet": "^3.15.0",
    "nedb": "^1.8.0",
    "serve-favicon": "^2.5.0",
    "winston": "^3.1.0"
  },

NodeJS version: v8.11.1

Operating System: OSX 10.13

Browser Version: N/A

React Native Version: N/A

Module Loader: N/A

[daffl]

You are sending the request to the service without an id. The URL should be the id you want to upsert, e.g. https://feathers-id-issue.glitch.me/scores/someId

[sinedied]

In the case of an insert, you have no ID so the endpoint call should be the same as a regular POST, right?
Otherwise I'm confused about this upsert, given that ID are rarely generated client side.

Also, without ID remapping it is working as (I) expected.

[daffl]

Upsert is supposed to update or create an existing id. If you don't have an existing id, you should use create (POST).

[sinedied]

Upsert is supposed to update or create an existing id. If you don't have an existing id, you should use create (POST).

That's fine, but AFAIK it's it's not documented and the fact that it works without ID remapping is misleading.

Still feels strange to me that upserting with the ID within the request (and not the route) is not supported, which does not seems natural.

Either way, as with every design decision it's fine if you don't agree and do not plan to support it, but then it should at least throw the same error when trying to upsert without and ID in the route when ID remapping is not enabled, for consistency (as it currently works fine) 😃

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Apologies if the issue could not be resolved. FeathersJS ecosystem modules are community maintained so there may be a chance that there isn't anybody available to address the issue at the moment. For other ways to get help see here.