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

Add a preValidateProperty hook that allows features like type coercion t... #68

Merged

Conversation

hugowetterberg
Copy link
Contributor

...o be implemented by users of the module.

I'm currently implementing request & response validation using jsonschema for hapi. A problem I hit was the strict type-checking of jsonschema. I realise that this is a good thing in itself, but to be able to use schemas for query strings and path segments I needed some type coercion. As this isn't something that should be done in the jsonschema module I propose that we add the possibility to hook in before property validation using a function on the options object.

In the case of my module I would then simply provide a preValidateProperty function like this:

function (instance, property, schema, options, ctx) {
  var value = instance[property];

  // Skip nulls and undefineds
  if (value === null || typeof value == 'undefined') {
    return;
  }

  // If the schema declares a type and the property fails type validation.
  if (schema.type && this.attributes.type.call(this, instance, schema, options, ctx.makeChild(schema, property))) {
    var types = (schema.type instanceof Array) ? schema.type : [schema.type];
    var coerced = undefined;

    // Go through the declared types until we find something that we can
    // coerce the value into.
    for (var i = 0; typeof coerced == 'undefined' && i < types.length; i++) {
      // If we support coercion to this type
      if (lib.coercions[types[i]]) {
        // ...attempt it.
        coerced = lib.coercions[types[i]](value);
      }
    }
    // If we got a successful coercion we modify the property of the instance.
    if (typeof coerced != 'undefined') {
      instance[property] = coerced;
    }
  }
}.bind(validator)

...that takes care of all the ugliness.

@KyleCartmell
Copy link

Can we please merge this change?

@wtfdaemon
Copy link

We could use this... any reason it's stale and unmerged?

@tdegrunt
Copy link
Owner

tdegrunt commented Nov 2, 2017

Hmm, it seems the build failed on this. Also: we need documentation on this that it's available for use.
Once both are fixed I'm not against this (anymore?). Though I want to protect the scope of this plugin: just validate schemas, but this doesn't seem to break this.

I really don't have the time to fix/look at both issues, so is someone will step up and check these, I'll merge and publish a new version.

@KyleCartmell
Copy link

@tdegrunt I opened #239 which includes these changes plus new documentation. The build succeeded so either the failure was temporal or there have been changes in the product over the past two years that have resolved the problem.

@tdegrunt tdegrunt merged commit 78a33bf into tdegrunt:master Dec 11, 2017
@tdegrunt
Copy link
Owner

Closed in favour of #239

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants