Meteor-Community-Packages / meteor-collection2

A Meteor package that extends Mongo.Collection to provide support for specifying a schema and then validating against that schema when inserting and updating.
https://packosphere.com/aldeed/collection2
MIT License
1.02k stars 108 forks source link

Apply `defaultValue` on `fetch` #433

Open Prinzhorn opened 2 years ago

Prinzhorn commented 2 years ago

Is your feature request related to a problem? Please describe.

I'm defining a schema as a contract between my application and the database. So that the application can be sure to always receive data according to the schema.

Coming from Mongoose I was expecting defaultValue to be always applied. It is not. Contract broken.

Describe the solution you'd like

Always apply defaultValue

Describe alternatives you've considered

I assume everyone is writing migrations for every new property? The beauty of default values in the schema is that you don't need to migrate every single document when adding a new optional property (it's optional after all, why force the default value to be persisted in the db?). They can just be missing in the db and you're good.

Additional context

I'm not sure if this is out-of-scope for this particular project? As I said, I'm used to Mongoose which does a lot more than just the schema. Even if it's in-scope, but not sure if this is even possible given the design of Meteor to intercept every fetch?

harryadel commented 2 years ago

Coming from Mongoose I was expecting defaultValue to be always applied. It is not. Contract broken. How's it not applied?

How's it not applied? Please provide a clear code example of what you're trying to achieve.

A thorough explanation is always best. I look up "defaultValue" in mongoose docs but I was only able to find this. I'm not familiar with mongoose so please

Additional context I'm not sure if this is out-of-scope for this particular project? As I said, I'm used to Mongoose which does a lot more than just the schema. Even if it's in-scope, but not sure if this is even possible given the design of Meteor to intercept every fetch?

Just to be clear, Collection2 is mere wrapper around simple-schema to ensure consistent data according to your defined schema. Maybe you need to look deeper into simpl-schema.

Prinzhorn commented 2 years ago

Please provide a clear code example of what you're trying to achieve.

meteor create --minimal defaults
cd defaults
meteor npm i mongoose simpl-schema
meteor add mongo
meteor add aldeed:collection2

server/main.js

import { Meteor } from 'meteor/meteor';
import { Mongo } from 'meteor/mongo';
import SimpleSchema from 'simpl-schema';
import mongoose from 'mongoose';

mongoose.connect(process.env.MONGO_URL);

const Books = new Mongo.Collection('books');

Books.attachSchema(
  new SimpleSchema({
    title: {
      type: String,
      defaultValue: '',
    },
  })
);

const Bookoose = mongoose.model('books', {
  title: {
    type: String,
    default: '',
  },
});

Meteor.startup(async () => {
  await Books.rawCollection().insert({});

  console.log('Collection2:');
  console.log(Books.find().fetch());

  console.log('Mongoose:');
  console.log(await Bookoose.find().exec());
});
$ meteor run

Collection2:       
[
  { _id: ObjectID { _str: '613f1af210ed928c897f4956' } },
  { _id: ObjectID { _str: '613f1b018b16e18cf59f5beb' } },
  { _id: ObjectID { _str: '613f1b4fa07c418e95ca8489' } },
  { _id: ObjectID { _str: '613f1b5c9f34a68f65578d53' } },
  { _id: ObjectID { _str: '613f1b7a177d0e900cf0669a' } },
  { _id: ObjectID { _str: '613f1b810295cb904fbe0488' } },
  { _id: ObjectID { _str: '613f1b931d346f90c248cfea' } },
  { _id: ObjectID { _str: '613f1c8c979bba91a3fb47e4' } },
  { _id: ObjectID { _str: '613f1cc1d0872b92783c7ad7' } },
  { _id: ObjectID { _str: '613f1cca89a5ee92dc71d47e' } },
  { _id: ObjectID { _str: '613f1cd348bdf7933396c57d' } }
]
Mongoose:
[
  { title: '', _id: new ObjectId("613f1af210ed928c897f4956") },
  { title: '', _id: new ObjectId("613f1b018b16e18cf59f5beb") },
  { title: '', _id: new ObjectId("613f1b4fa07c418e95ca8489") },
  { title: '', _id: new ObjectId("613f1b5c9f34a68f65578d53") },
  { title: '', _id: new ObjectId("613f1b7a177d0e900cf0669a") },
  { title: '', _id: new ObjectId("613f1b810295cb904fbe0488") },
  { title: '', _id: new ObjectId("613f1b931d346f90c248cfea") },
  { title: '', _id: new ObjectId("613f1c8c979bba91a3fb47e4") },
  { title: '', _id: new ObjectId("613f1cc1d0872b92783c7ad7") },
  { title: '', _id: new ObjectId("613f1cca89a5ee92dc71d47e") },
  { title: '', _id: new ObjectId("613f1cd348bdf7933396c57d") }
]

to ensure consistent data according to your defined schema

But only on insert/update. Hence the feature request to apply this to documents when they are fetched as well. You can see that even though there is no title in the database Mongoose returns one. This makes changing the schema :fire: without having to write migrations all the time just to add a default to existing docs.

harryadel commented 2 years ago

Well first off, I wanna thank you for taking the time to recommend ways to improve Collection2. I really appreciate it! :)

hmm, I could definitely see some value but I'm unsure how this would affect current usage. We need more opinions.

cc @StorytellerCZ @coagmano @copleykj

Prinzhorn commented 2 years ago

Well first off, I wanna thank you for taking the time to recommend ways to improve Collection2. I really appreciate it! :)

Thanks for looking into them!

but I'm unsure how this would affect current usage

It could be an option that defaults to false (maybe turn it true in a major release) :+1:

There definitely needs to be some thought put into this. It won't work with the default clean options, but that's perfectly fine. E.g. I've set all of them to false, including removeEmptyStrings. Someone that uses removeEmptyStrings wouldn't expect to receive an empty string when fetching objects either. Not sure about other options (haven't used simpl-schema before stumbling into Meteor and Collection2), so maybe it would be as simple as calling clean on fetched object, assuming we can hook into fetching (I didn't look into how Collection2 intercepts insert/update calls)

copleykj commented 2 years ago

In my opinion this is not in the scope of this package. Migrations are the standard way to to ensure integrity when making schema modifications. If something prevents you from using migrations, I would suggest delegating this task to a model controller.

StorytellerCZ commented 2 years ago

I partially agree with @copleykj. Currently it is our of the scope of this package, but I would be open for discussing inclusion of this functionality in the next major version.