Meteor Guide: Methods

Hi guys,
the link in “consult the Security article” (in Basic method) is not good
-> http://guide.meteor.com/security.md#secret-code

I think it should be
-> http://guide.meteor.com/security.html#secret-code

1 Like

In working to take advantage of ValidatedMethod, I was disturbed by the repetition of schema validation code that was already present elsewhere in the validate member of the method. A short look at SimpleSchema revealed that the “pick” function should provide the answer to that. I even saw several clues that efforts were made to enable exactly the approach I was imagining with ValidatedMethod and SimpleSchema and I wonder why they weren’t illustrated in the guide. IMO, best practice would never have duplicated schema definitions that could be shared.

Anyway, I set out to utilize pick to reuse my schema and thought that all was going well… until I tested.

I want my “insert” method to require “name”, allow “description” to be undefined, and “visibility” to default to “private” if not specified. This is exactly what my SimpleSchema definition provided for, so I figured all would be good. I set validate to “Groups.schema.pick([‘name’, ‘description’, ‘visibility’]).validator()” and thought all was good. But, the “visibility” option wasn’t defaulting as defined by my Groups schema.

After digging through the docs, I found that the defaultValue is implemented in the clean stage of SimpleSchema processing and that stage isn’t run by validator(). So, my problem isn’t with “pick”, it is with “validator”. Luckily, aldeed added a “clean” option to validator() in December. Turning on “clean” allows the defaultValue to have effect. Awesome.

The problem I now have is that the “clean” stage is too thorough. It is cleaning away undefined keys without giving any error indication to the caller. This can cause a lot of head pounding when a developer has a typo in a key.

So, does anyone know how to get SimpleSchema to validate while both implementing defaultValue and complaining about fields that aren’t present in the schema?

Hopefully, I’ve excerpted enough of my code below to give a flavor of what is happening.

Groups.schema = new SimpleSchema({
  name: {
    type: String,
  },
  description: {
    type: String,
    optional: true,
  },
  visibility: {
    type: String,
    allowedValues: [
      'public',
      'community',
      'private',
    ],
    defaultValue: 'private',
  },
});

export const insert = new ValidatedMethod({
  name: 'groups.insert',
  validate: Groups.schema.pick([
    'name',
    'description',
    'visibility']).validator({ clean: true }),
  run({ name, description, visibility }) {
    const group = {
      name,
      description,
      visibility,
    };
    return Groups.insert(group);
  },
});

// this fails without "{ clean: true }" on validator call above.
groupId = insert._execute(
  { userId: 'test-user' },
  { name: 'goodname', description: 'good description' }
);

// this works with "{ clean: true }", but I don't want it to.
insert._execute(
  { userId: 'test-user' },
  { name: 'goodname', badField: 'no such field' }
);

If you figure out a better way to do it that works well I’d be happy to include it in the guide!

Thanks sashko! Of course, better is always subjective and I’ve yet to feel fully confident about this one. After reading what I’ve written below, do you see any downsides to this approach or ways to make it simpler?

Your response prompted me back to one more look and … I think I’m guilty of coding while tired :weary:

The SimpleSchema documentation clearly pointed the way in stating “If you want to change any of the default cleaning options, you can pass in those, too.” The “filter” option is the first of the cleaning options and is the key to the problem.

Setting validate to:

  validate: Groups.schema.pick([
    'name',
    'description',
    'visibility']).validator({ clean: true, filter: false }),

behaves as hoped.

So, I think the key to this approach is to always run validator with { clean: true, filter: false } in this context.

In looking at the clean options, I also noted “extendAutoValueContext”, I wish I had seen that a few days ago as it may lead to a solution to another problem I had in server-side testing of a ValidatedMethod in the context of a with Mocha. The _execute put the userId I had defined in my execution context into the Method, but that didn’t make it into the SimpleSchema validation attached to my collection. I was referencing userId in a defaultValue setting in there and it was coming up undefined. Perhaps I should have forwarded this.userId into SimpleSchema using “extendAutoValueContext” as an option on the insert. If so, should this also be illustrated somewhere in the guide? It’s probably not unusual for defaultValue and autoValue functions in SimpleSchema definitions to reference userId.

I probably should be writing a wrapper of some type that takes a SimpleSchema and calls validator with the proper settings for clean, filter, and extendAutoValueContext and returns a function… hmmm.

I’ve thrown some excerpts from my code that should pretty well reveal the pattern I’m now using below.

const GROUP_ID_ONLY = new SimpleSchema({
  groupId: {
    type: String,
    regEx: SimpleSchema.RegEx.Id,
  },
});

export const insert = new ValidatedMethod({
  name: 'groups.insert',
  validate: Groups.schema.pick([
    'name',
    'description',
    'visibility']).validator({ clean: true, filter: false }),
  run({ name, description, visibility }) {
    //...
  },
});

export const archive = new ValidatedMethod({
  name: 'groups.archive',
  validate: GROUP_ID_ONLY.validator({ clean: true, filter: false }),
  run({ groupId }) {
    //...
  },
});

export const rename = new ValidatedMethod({
  name: 'groups.rename',
  validate: new SimpleSchema([
    GROUP_ID_ONLY,
    Groups.schema.pick(['name']),
  ]).validator({ clean: true, filter: false }),
  run({ groupId, name }) {
    //...
  },
});
1 Like

You can easily write a ValidatedMethod mixin! https://github.com/meteor/validated-method#mixins

Exploring.

I like the idea of mixins, but don’t like that they have to be stated in every Method definition. I feel like I’m replacing something that has to be remembered with something else that has to be remembered. The whole point of ValidatedMethod is to hide boilerplate.

I guess I could wrap ValidatedMethod and specify the mixin in the wrapper, but it feels wrong to add another layer of wrapping.

It seems like the ValidatedMethod package should have a method, perhaps “defaultOptions”, that could specify default options including default mixins. Perhaps there was a conversation on this at some point that I could look back at to understand the choice?

1 Like

Not in every call - in every definition. That’s a very important distinction!

I think the easiest thing to do would be to make your own wrapper for ValidatedMethod, like MyAppMethod that just passes the mixins you always use.

Properly chastised and changed :slight_smile:

Somewhat related since I’m essentially writing a SimpleSchema mixin, just what is being imagined beyond the schemaMixin example given in the guide when the guide says

It could be nice to have a SimpleSchema mixin which just lets you specify a schema option rather than having to pass a validator function into the validate option.

Just package infrastructure and some type checking maybe? Perhaps the ability to specify an array of SimpleSchema objects that get automatically combined into one validator function as I did once in my example above? Maybe a check that “validate” wasn’t also specified? (unless arrays of validate functions are supported…)

Maybe the code sample is all there is - but I didn’t want to sign up for maintaining another package!

One thing that keeps coming up in forums and chat rooms is Meteor methods where they want to return the result of a callback.

Nothing is mentioned here, Also, I’m not sure where its explicitly said that you can return a Promise from a Meteor method. Which is what I use.

1 Like

Which promise package are you using?

just the built in one in ES6

from a freshly created project… in the server main.js


then in the client console…

then in the client console…

4 Likes

The React Todos example app imports both method files (/lists/methods.js & /todos/methods.js) on start up. They are imported on the server by /server/register-api.js. Is it necessary to import method files on start up?

It seems like you would only need to import a specific method from client code that calls it.

You need to import them on the server on startup, otherwise when the client calls it there won’t be any method to call!

Thanks, I’m still trying to wrap my head around the importing and exporting. I thought that importing from the client would also register the server side. Sort of like how calling a method runs it both on the client and server.

I’m looking at the code for validated-method and I was wondering, can it be implemented as an npm module rather than an atmosphere module?

1 Like

Not at the moment because it calls Meteor.methods internally. What would be the benefit of making it an NPM module?

Primarily to help migrate away from having another packaging system unless needed. For example if code is meant to be only run on server or on client without resorting to Meteor.isServer check.

My classification for something that would be candidates to go npm are.

Client only packages much like angular blaze templates directive.

Code that can run on both client and server with no modification or if Meteor.isServer.

Code that only has dependencies to meteor-base since there is no need for package resolution then. So meteor/meteor is still game.

Just my two c.

1 Like

Yeah, so this package has deps on meteor-base. So seems like it doesn’t fit your filter?