Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Treat empty string as null #899

Closed
ryan-morris opened this issue May 23, 2016 · 14 comments
Closed

Treat empty string as null #899

ryan-morris opened this issue May 23, 2016 · 14 comments
Assignees
Labels
feature New functionality or improvement
Milestone

Comments

@ryan-morris
Copy link

I searched and the closest issue I could find related to my question was #299 . If you feel this is an application level issue only just let me know, but I feel other users of Joi must experience the same type of use case.

We would like to be able to allow empty strings but have Joi convert them to null. Our specific hang up has been around requests in a REST API that feeds both HTML forms and 3rd party partners.

Let's take a user's email address for example. If they specify it then they must make it a valid email address, otherwise if the user omits that field it is not updated. In most cases the user is entering data into a web form of some kind so if they don't want their email in the system any more they clear out the input field and it becomes an empty string.

So we started looking at doing:

var jsonApiInput = {
    email: ''
};

var schema3 = Joi.object().keys({
    email: Joi.string().email().empty('').default(null)
}).required();

schema3.validate(jsonApiInput, function (err, value) {
    //validation ok, { email: null }
});

and this works fine except when a 3rd party implementer omits that field, it defaults to null, and values they did not expect to be updating are nulled in the database.

Right now our only option is to if(obj.email == '') { obj.email = null } all of our string fields either after Joi has validated the object (leaving .default(null) off), or on all the client side versions.

Ideally, we could do something like .empty('', null) to set the assigned empty value, or some kind of conditional default value.

Does anyone have any thoughts or suggestions on use cases like this?

@Marsup
Copy link
Collaborator

Marsup commented May 28, 2016

I think you should either have 2 separate endpoints for API and forms, or pre-process your forms before posting. I'm not completely opposed to the idea but I don't think it's super useful.

@Marsup Marsup added the request label May 28, 2016
@Marsup Marsup self-assigned this May 28, 2016
@ryan-morris
Copy link
Author

I think you are right that as far as Joi and a backend API would be concerned that it is not very useful. I do believe it would reduce the amount of work on client side implementations quite a bit. If you use something like Angular.JS for databinding, and you have a few fields in a form that are left blank, you have to do the pre-processing on every field on every form.

I actually think that by providing Joi a way to change an empty string to a null value that would save a lot of "full stack" developers quite a bit of time and maintenance effort when it comes to client side implementations.

I would be willing to try and get this working if you think this is something that would be accepted and had any suggestions on implementation.

@Marsup
Copy link
Collaborator

Marsup commented Jun 2, 2016

I'm already accommodating frontend users by providing empty which is a way to drop the fields that contain an empty string (most usual use case), you want explicit nulls which is to me a completely different thing. If you're using angular, I don't see anything wrong in using $http interceptors to adhere to a proper API definition, do you ?

@ryan-morris
Copy link
Author

ryan-morris commented Jun 2, 2016

We do utilize the empty as well, it works great for query parameters when they are filters for data in a query and they can just be dropped off if not specified. Exactly what we needed for cases like this.

In my experience a lot of DBAs just don't want empty strings in their databases, they would prefer null values, not saying they are right or wrong, just my experience dealing with them.

Our only option for these types of DBAs are .empty().default(null) which doesn't work with PATCH requests because we can't just assign default values since all properties are optional.

An angular interceptor is fine. It would be a little bit of overhead for a complex object with a lot of nested properties. You would then have to check types and values each step of the way on an object such as

{ name: "", address: { street: "", geography: { type: 'Point' } } }...

I just wanted to ask if this would be an option first, since we have multiple platforms outside of angular we must support as well. If Joi could handle this situation then it would solve the problem across the board for us but if it is in fact an uncommon and outside of the scope of Joi then that is no problem, with programming there are always a plethora of ways to accomplish the same goal.

@MarkHerhold
Copy link

This sounds like a good use-case for joi.extend() with a base of Joi.string(). You can define whatever special logic you want with that.

@buildjosh
Copy link

buildjosh commented Jun 21, 2016

@MarkHerhold I was loooking into using joi.extend() for converting empty strings to null, but I'm not sure how it would work since when null is returned by the pre() function means you left the value as it was?

From the example: (https://github.com/hapijs/joi/blob/v9.0.0-6/API.md#examples)

pre(value, state, options) {

        if (options.convert && this._flags.round) {
            return Math.round(value); // Change the value
        }

        return null; // Keep the value as it was
    },

Likewise, null returned from any of the rule validation functions means everything was ok.

@MarkHerhold
Copy link

@buildjosh I admit I'm not sure, but I think you should look into using the coerce function hook instead of pre because you are attempting to transform values to a different type. I'm not sure about the behavior of null in the coerce hook.

@ryan-morris
Copy link
Author

With pre I had the same probelm as @buildjosh . With coerce under 9.0.0-7, it does not appear to run if the value is an empty string, also if returning null it seems to mimic pre in that the value does not change.

var Joi = require('joi');

 var cumstomJoi = Joi.extend({
    base: Joi.string(),
    name: 'string',
    coerce: function(value, state, options) {
        console.log('coercing');

        if(value === '') {
            return 'hello2';
        } else {
            return 'bye';
        }
    },
    rules: []
});

cumstomJoi = cumstomJoi.extend({
    base: Joi.string(),
    name: 'emptyAsNullString',
    coerce: function(value, state, options) {
        console.log('coercing');

        if(value === 'test') {
            return null;
        } else {
            return 'WasNotNUll';
        }
    },
    rules: []
});

var testSchema = cumstomJoi.object({
    testProp: cumstomJoi.string().required().allow(['',null])
}).required();

var testSchema2 = cumstomJoi.object({
    testProp: cumstomJoi.emptyAsNullString().required().allow(['',null])
}).required();

var testObj = {
    testProp: 'test'
};

var testObj2 = {
    testProp: ''
};

//outputs:
// coercing
// with value:
// null
// { testProp: 'bye' }
cumstomJoi.validate(testObj, testSchema, function(err, result) {
    console.log('with value: ');
    console.log(err);
    console.log(result);
});

//outputs:
// with empty string:
// null
// { testProp: '' }
cumstomJoi.validate(testObj2, testSchema, function(err, result) {
    console.log('with empty string: ');
    console.log(err);
    console.log(result);
});

//outputs:
// coercing
// null
// { testProp: 'test' }
cumstomJoi.validate(testObj, testSchema2, function(err, result) {
    console.log(err);
    console.log(result);
});

@Marsup Marsup closed this as completed in a646761 Jun 25, 2016
@Marsup Marsup added this to the 9.0.0 milestone Jun 25, 2016
@Marsup Marsup added breaking changes Change that can breaking existing code and removed breaking changes Change that can breaking existing code labels Jun 25, 2016
@buildjosh
Copy link

Awesome, thank you @Marsup

@anderson-tyler
Copy link

What wast the solution for converting an empty string to null?

@buildjosh
Copy link

buildjosh commented Jul 14, 2016

@anderson-tyler Essentially in custom validations, instead of returning null to signal that everything is okay and you want to keep the original value, you now return the value itself. So now when you return null, null will be the value being returned which fixes the issue.

a646761

@anderson-tyler
Copy link

With the .allow('') coerce is never run, because an empty string is explicitly allowed, and validation is not performed.

without the .allow('''), the base seems to be running before the coerce, and results in this error: "testProp" is not allowed to be empty'

any suggestions?

var Joi = require('joi');

var cumstomJoi = Joi.extend({
    base: Joi.string().allow(null),
    name: 'string',
    coerce: function (value, state, options) {

        if (value === '') {
            return null
        }
        return value; // Keep the value as it was
    },
});

var testObj = {
    testProp: ''
};

var testSchema = cumstomJoi.object({
    testProp: cumstomJoi.string().allow(''),
}).required();

//output: { testProp: '' }
//expected: { testProp: null }

cumstomJoi.validate(testObj, testSchema, function (err, result) {

    if (!err) {
        console.log(result);
    } else {
        console.log(err);
    }
});

@Marsup
Copy link
Collaborator

Marsup commented Jul 24, 2016

@anderson-tyler #960 should help.

@hueniverse hueniverse added feature New functionality or improvement and removed request labels Sep 19, 2019
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New functionality or improvement
Projects
None yet
Development

No branches or pull requests

6 participants