strongloop / strong-remoting

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

type: 'any' in a remote method can coerce mongodb object ids into floats #299

Closed vkarpov15 closed 8 years ago

vkarpov15 commented 8 years ago

If you have a remote method with a parameter that's type: 'any', coerce can convert a mongodb object id into a float because mongodb object id hex strings can happen to be all numbers. Easily fixed on my end (use type: 'string') but this is an annoying rough edge in the API IMO.

richardpringle commented 8 years ago

Hey @vkarpov15, those types are LoopBack Types; 'any' is not a type according to the documentation... And now I see that we test for type: 'any' <- @crandmck, think we could add that to the above link?

@vkarpov15, there are several issue with coercion that are currently being "cleaned up" by @bajtos , I will make sure that we consider this as well!

Cheers

richardpringle commented 8 years ago

@bajtos, I keep adding the "coercion clean up" to related bugs. If you have a different workflow in mind please let me know. I'm not sure if your fixes will cover all cases, but at the very least I think we should add documentation for any remaining coercion behavior as a part of the milestone.

crandmck commented 8 years ago

Yes, I can add that.

Is the "Any" type name capitalized, as most of the other LoopBack types are?

Will it match just any primitive type, or will it also match Object, Array, and GeoPoint types?

vkarpov15 commented 8 years ago

It was lower case 'any' unfortunately - I assume the person that wrote it in the first place was anxious about whether the parameter would be a string or a full fledged mongodb ObjectId.

bajtos commented 8 years ago

It's any and it basically means "anything" - a string, a number (used for IDs in SQL databases), or an object like ObjectID, or even an array or a boolean.

I am pondering an idea of always encoding the id property as strings, see https://github.com/strongloop/loopback/issues/2046

crandmck commented 8 years ago

OK, I added it to the table in https://docs.strongloop.com/display/LB/LoopBack+types (and reordered the table alphabetically).

One final question: Will any match a null value?

vkarpov15 commented 8 years ago

IMO if you say "any" that should mean "no type coercion please, let me deal with it"

bajtos commented 8 years ago

This issue was fixed by https://github.com/strongloop/strong-remoting/pull/343 in LoopBack 3.0 (alpha). Model properties of "any" type sent in JSON request body are no longer converted from string to number. When the "any"-typed value comes from a string-only source like query-string, we convert to numbers only values that are "safe" to convert, meaning they we won't loose information as a result of the conversion. Most notably, integer values starting with 0 and values larger than MAX_SAFE_INTEGERS are preserved as strings.

vkarpov15 commented 8 years ago

Don't rush to release on our account, we've been loopback-free for a few months now and it's been a dream come true :)

bajtos commented 8 years ago

Don't rush to release on our account, we've been loopback-free for a few months now and it's been a dream come true

That's sad news. If I may ask you, what tool/framework/approach are you using now?

Tallyb commented 7 years ago

If anyone faces the same issue - here is a boot script that fixes the problem for all methods:

module.exports = function(app) {
    app.models().forEach(model => {
        if (model.settings.permissions) {
            //fixes the id type for all methods
            model.sharedClass._methods.forEach(method => {
                let idArg = method.accepts.find(elem => {
                    return elem.arg === 'id';
                });
                if (idArg && idArg.type === 'any') {
                    idArg.type = 'string';
                }
            });
            // Fixes the id type for the shared constructor
            // This runs on all models but will happen only once for each base model
            // As all models have the same base model shared constructor
            let classCtorIdArg = model.sharedClass.sharedCtor.accepts.find(elem => {
                return elem.arg === 'id';
            });
            if (classCtorIdArg && classCtorIdArg.type === 'any') {
                classCtorIdArg.type = 'string';
            }
        }
    });
};