fogine / couchbase-odm

CouchbaseODM is a promise-based Node.js ODM for Couchbase
https://fogine.github.io/couchbase-odm
MIT License
12 stars 3 forks source link

Get refDoc or null #20

Closed mreis92 closed 7 years ago

mreis92 commented 8 years ago

Hi @fogine.

Is it possible to have a get method that returns null instead of throwing an error? For instance, when invoking a custom index of our model such as User.getByEmail, it would be nice to have something like User.getByEmailOrNull.

The rationale here is to have the possibility of doing something like a Promise.join and fetch all the values in the end without having to deal with promise rejections. Does this make sense to you? Thank you!

fogine commented 8 years ago

Hello @mreis92 ,

it makes totally sense! I've also repeatedly stumbled upon this problem as currently in some cases it's quite inconvenient to handle StorageNotFoundError. I've not made correct design decision when implementing "get" methods...
However, I've also spotted some inappropriate defaults, when I was dealing with recent reported issues, which should be changed in sake of better user experience.

So, the other option is to fix these inappropriate behaviors in next major release as soon as possible so we have solid core functionality on which we can build upon. That'd include changing behavior of all get methods so that they don't throw on StorageNotFoundError but return null value instead.

I still need to think about which road to take. Your approach is more sensible to the recent state of development...

fogine commented 8 years ago

Oh... and by the way @mreis92 , in meantime you can use the following polyfill :

This will add getByIdOrNull method to all Models:

var ErrorCodes = couchbaseODM.StorageAdapter.errorCodes;

var couchbase = new couchbaseODM({
    bucket: bucket,
    classMethods: {
        getByIdOrNull: function(id, options) {
            return this.getById(id, options).catch(function(err) {
                if(err.code == ErrorCodes.keyNotFound) {
                    return null;
                }
                return Promise.reject(err);
            }
        }
    }
});

In similar way you can add polyfill for getByRefDocOrNull methods...

mreis92 commented 8 years ago

Hello @fogine ! I understand then.

Nonetheless, the polyfill you suggested should work nicely; I hadn't thought about something like this. Thank you very much for your help!!

mreis92 commented 8 years ago

Hello @fogine! I would like to add something to your approach. I have added to every Model with indexes the classMethods property, in order to use a similar polyfill as the one above. An example on a model that has an index on the email would be:


getByEmailOrNull: (id: any, options: any) => {
                return User.getByEmail(id, options).catch((error: any) => {
                    if(error.code == ErrorCodes.keyNotFound) {
                        return null;
                    }
                    return Promise.reject(error);
                });
            }

Note that instead of using the this object, I user the model instead (User.getByEmail).

However, I wasn't able to retrieve the errors from the couchbaseODM (through couchbaseODM.StorageAdapter.errorCodes.keyNotFound). Please, let me know if this is the approach is the correct way to do it and if there's something I am missing regarding the errorCodes. Thank you!

fogine commented 8 years ago

Currently, couchbaseODM.StorageAdapter.errorCodes.keyNotFound is available on the develop branch only. It will be released in the next minor release. Current documenation reflects features which are available in the latest release (which is v1.0.0). Alternative way of accessing the information:

var couchbaseSDK = require('kouchbase-odm/node_modules/couchbase');
// couchbaseSDK.errors === StorageAdapter.errorCodes
fogine commented 7 years ago

Closing, solved in v2.0.0-rc.1