Calling get().without() with enforce_missing:true causes an error

[mindjuice]

If you have defined the global enforce_missing:true flag, and you attempt to get a subset of fields of a document(s) by using the without() function, Thinky throws an error essentially saying that the fields you removed are missing:

[Error: The results could not be converted to instances of `User`
Detailed error: Value for [hash] must be defined.]

In my case, I've ported the **passport-local-mongoose* package to Thinky (*passport-local-thinky* will be on GitHub soon), and it adds a hash and salt and a few other fields to my User object. My User model looks like this:

var User = thinky.createModel('User',{
  id:        type.string().options({enforce_missing: false}),
  firstName: type.string(),
  lastName:  type.string(),
  email:     type.string(),
  role:      type.string(),
  schoolId:  type.string(),

  // Password-related fields
  hash:      type.string(),
  salt:      type.string(),
  last:      type.date(),
  attempts:  type.number()
});

I obviously want to strip those out before returning a User object to the client, so I do something like .without(['salt', 'hash', 'last', 'attempts']). To work around this, I've added .options({enforce_missing: false}) to each of the password-related fields.

I want those fields to exist when creating or updating the document, but I want to be able to query the documents without some of them.

Is this behavior intentional? If so, is there a better way to avoid the problem?

Thanks.

[neumino]

I started to write some doc about this behavior since #183 also somehow bring that up.

There seems to be two school of thoughts in when to validate a document:

1. Validate the document only when saving. That way you always save valid date, but you can retrieve data with missing fields/wrong types without problem. You just have to "fix" them before saving.
2. Validate the document when saving and retrieving. This gives you the guarantee that anything you save and get from your database match what you define in the schema.

Thinky currently implements the behavior described in 2. I recently (like two days ago) started to implement 1 as an option, but it's a bit more tricky than what it looks like. I still plan to do it, but it may take more time that the two other issues you opened.

The way to make it work for now is to use .option({enforce_missing: false}). I believe that implementing .optional() would make things way more convenient/nicer though.

Thanks @mindjuice for all your feedback!

[mindjuice]

No worries on the timing of this one. I can easily work around it for now.

I haven't looked at the code for this yet, but could you perhaps provide a wrapper around the without() calls and keep a reference to the parameters, then pass the parameters along to Rethink's without()? Then when you get the result and are turning it into a model document, don't do validation of any fields in the 'without' list?

[tlvenn]

Just stumble on this myself when using partial objects with fields that have default values. If I dont retrieve those fields, the default values are injected.

For my use cases, behaviour 1 would be very handy.

[tlvenn]

Hi Michel, any luck working on implementing behaviour 1 ? Right now, I have to fetch all the required fields of a document even if I only need 1 of them..

Thanks a lot in advance !

[neumino]

Sorry, I didn't have time to work on that yet.
I've been mostly updating things for 2.0 recently.

I plan to implement removeRelations for #194, then I'll take a look at that again.

[tlvenn]

No worry, glad to know it is still on the radar and cant wait to see all 'the good stuff that are coming soon' :)

[marshall007]

@neumino a third option would be to continue validating when retrieving, but only validate the fields that come back.

[edit]: not sure how you would handle virtual fields in that case though (i.e. if they depend on properties that weren't returned).

[timhaak]

Passwords are another example of where you would like to ensure the data exists in the document and for most queries don't want to retrieve it.

let Users = thinky.createModel('Users', {
id: type.string(),
email: type.string().email().lowercase().required(),
password: type.string().required(),
created_at: type.date().default(r.now),
updated_at: type.date().default(r.now)
});

Users.defineStatic('getView', function () {
return this.without('password');
});

When calling Users.getView().run(); you get the following error

Document failed validation: Value for [password] must be defined.

[neumino]

There are a few solutions:

• Set password as optional
• Use execute instead of run
• Define the method getView with define

Model.define('getView', function(user) {
  delete user.password;
  return password
})
Users.get(1).run().then(function(user) {
  return user.getView();
})

[tlvenn]

Hi @neumino , sorry to bug you with this issue once more. With the coming of FalcorJS and GraphQL, many client apps will start fetching only what they need, down to the attributes of a given model.

It really becomes quite necessary and important to be able to load partial objects with Thinky I believe.

[neumino]

Ok, I have a rough proposal. It needs more work to polish exactly the syntax, but we can make the validation on retrieval optional. In this case you get a document that is not savable.

This addresses my main concern:

Users.get("mail@example.com").pluck("name").run().then(function(user) {
    // do stuff with user
    user.save() // this is what I'm worried about.
})

Then the question is whether

• thinky should look for without and pluck to decide whether documents retrieved will be validated and whether they should be savable.
• we should add a new command like run/execute that retrieve non savable documents on which no validation is done. We could add an option maybe?

[tlvenn]

I wonder if both implicit and explicit read only document would not be useful... Maybe have an explicit command that is implicitly called should you detect without or pluck ?

[tlvenn]

@neumino any news on this ? It would be awesome to solve this.

[aguegu]

run into the same problem.

[nfantone]

Same over here. Had to declare fields as optional or _.pluck away after retrieval with lodash.

[tlvenn]

Hi @neumino , sorry to bump this issue again. Any chance you can give us an update on it ? Thanks a lot in advance !

[neumino]

So there isn't much progress because I actually don't have a clean API in mind. I basically don't see what's a better alternative than using execute.

If some fields are missing, what do we do about:

• Per field
• Cross validation
• Hooks
• Joined documents

[mattkrick]

IMO, there is no need to guarantee that you can call save from a get. Doing so implies that save is guaranteed inside a get, but not outside, and I don't see why the assumption should change based on callee location. Similarly, as I understand it, joins & hooks currently don't have a guarantee (eg unless i require my foreign key in my validator, it's possible that the FK is undefined & the join will fail for that row). So excluding a field is no different than that field being undefined. Throwing an error would be nice, though.

With graphql in mind, I imagine we'll shift away from defining join logic in the model & push that into the graphql query, so no worries regarding future strategy.

[mindjuice]

I agree with @mattkrick that we don't need a guarantee that you can call save after getting a doc.

If I get a doc and call pluck or otherwise include/exclude fields, I'm almost certainly retrieving it to send back over the wire to the user rather than getting it to modify it.

Just document the fact that the onus is on the dev to make sure that a doc is savable.

[emensch]

Agreed with @mindjuice and @mattkrick on this. It makes little sense to me that validation is enforced on document retrieval. The validation behavior on calling save is already known and expected; why create additional constraints?

[nfantone]

What's the approach here of other ORM with validators? Let's take at look a those for a reference. Or maybe make it configurable?

It really is a tad annoying if you had your models modified by some external source and discover that your application's endpoints simply stop working.

[emensch]

If I remember correctly, neither Mongoose nor Sequelize run validators on retrieval. Both validate models only prior to saving them (implicit in update/create/etc operations).

Those are the only two I've used; are there other ORMs that follow Thinky's validation behavior?

[nfantone]

What about Bookshelf or Sequelize?

EDIT: @emensch I just read you already mentioned Sequelize on your previos comment. My bad.

[emensch]

Sequelize does not. I've not used Bookshelf, but a quick look at the docs leads me to believe that it's up to the dev to hook into events (e.g. saving/creating/updating) and run their own validations manually.

[nfantone]

@emensch Haven't use Sequelize, but I can backup you up on Bookshelf. Does not validate models automatically.

It seems like the common trend is not to validate or do so on saving/updating.

@neumino Any thoughts on this? How hard would it be to de-activate validation on fetching (or making that an opt-in feature)?

[dhax]

any progress or future proof workaround for this? Also noticed that I can't have virtual attributes based on stripped attributes using without. Like this example won't work, as provider object is removed before evaluation of linkedProvider:

const User = thinky.createModel('User', {
  id: type.string(),
  email: type.string().email().required().allowNull(false),
  provider: type.object().default({}).schema({
    facebook: type.object().schema({
      id: type.string(),
      token: type.object().schema({...})
    }),
    google: type.object().schema({
      id: type.string(),
      token: type.object().schema({...})
    })
  }),
  linkedProvider: type.virtual().default(function () {
    return {
      facebook: !!this.provider.facebook,
      google: !!this.provider.google
    }
  })
}, {
  enforce_missing: false,
  enforce_extra: 'remove',
  enforce_type: 'strict'
})

User.defineStatic('getAccount', function() {
 return this
    .without('id')
    .without('provider')
})

[neumino]

No update, feel free to send PR : )