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

Apply defaultValue on fetch #433

Open
Prinzhorn opened this issue Sep 3, 2021 · 6 comments
Open

Apply defaultValue on fetch #433

Prinzhorn opened this issue Sep 3, 2021 · 6 comments

Comments

@Prinzhorn
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor Author

Prinzhorn commented Sep 13, 2021

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 🔥 without having to write migrations all the time just to add a default to existing docs.

@harryadel
Copy link
Member

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
Copy link
Contributor Author

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) 👍

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
Copy link
Member

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
Copy link
Member

StorytellerCZ commented Sep 27, 2021

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.

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

No branches or pull requests

4 participants