meteor-rest icon indicating copy to clipboard operation
meteor-rest copied to clipboard

Add built-in support for versioning

Open aldeed opened this issue 10 years ago • 17 comments

Here are my rough thoughts regarding versioning.

GOALS:

  • HTTP and DDP use same version checking and same input/output migrations
  • Do not recommend versions in the path, but don't prohibit them. (Make more sense when trying to keep HTTP and DDP methods the same.)
  • Assume latest version
  • Allow REST requests to specify API version in multiple typical ways. Provide way for DDP request to specify API version, too.

IDEAS:

  • Regardless of whether DDP or HTTP, this.apiVersion will be set in the methods if a specific version is requested. For HTTP, middleware will do this based on checking "api-version" header or "v" query string param. (Does DDP spec provide a way to specify version?)
  • We do not care what format versions take. Could be semver, a date, etc. As long as app code understands what is meant.
  • If this.apiVersion is not set, assume latest version.
  • Provide simple way to convert input/output as method/pub options.

Example of last point:

Meteor.publish("widgets", function (all) {
  if (all) {
    return Widgets.find();
  }

  // could also check this.apiVersion in here for advanced cases

  return Widgets.find({userId: this.userId});
}, {
  convert: {
    in: function (apiVersion, args) {
      // In version 1, no args were accepted and "all" was the default
      if (apiVersion === 1) {
        return [true];
      }

      return args;
    },
    out: function (apiVersion, result) {
      // In version 1, property was called "foo_bar" instead of "fooBar"
      if (apiVersion === 1 && _.has(result, 'fooBar')) {
        result.foo_bar = result.fooBar;
        delete result.fooBar;
      }

      return result;
    }
  }
});

aldeed avatar May 26 '15 17:05 aldeed

IMO versioning DDP seems a little out of scope for this particular package - maybe there should be a different package that does API versioning?

stubailo avatar May 26 '15 19:05 stubailo

We are going to be exposing our ddp endpoints to customers and since we are reusing the publications for restpoints -- we would like to have the same pattern in place.

I agreed it could be implemented in a separate package but I think the pattern should be consistent across both.

jperl avatar May 26 '15 22:05 jperl

@stubailo how would you envision a separate package "plugging in" the versioning? For conversions, arguments and return values would need to be accessible. But I am generally in favor of separate packages.

The goal should really be to only ever have a single method being called, with adjustments being made for backwards incompatible changes.

DDP versioning is becoming more necessary with increased use of "reload on resume" for mobile and delayed refresh for browser, not to mention exposing DDP API for outside consumption. And insofar as simple:rest is aiming to have single pattern for both protocols, it needs to be at least aware of versioning, even if details are left up to separate packages.

aldeed avatar May 27 '15 02:05 aldeed

@aldeed @jperl I agree with everything you guys have said - I just think it would be weird if a package called "rest" was also responsible for versioning your DDP API. So we should think of what that new thing that is responsible for versioning DDP and REST would look like.

stubailo avatar May 27 '15 04:05 stubailo

@stubailo, I think the problem is that the Meteor.method pattern gives us options, which is a convenient place to handle versioning. So maybe Meteor.method API should be a separate package, which simple:rest depends on? That way we could build a versioning package on top of it, and you could use that to version DDP alone, if you don't need REST.

aldeed avatar May 27 '15 13:05 aldeed

Yeah, the Meteor.methods({}) function doesn't give you a place to put options at all, that's a good point. Although I'm not sure the world will be that much improved if every method definition has like 10 options hanging off of it?

Is switching over to Meteor.method the best way?

stubailo avatar May 27 '15 14:05 stubailo

It's certainly possible to do versioning without options. There would be a lot more logic lumped into the method itself, but that could be abstracted out.

I'm not sure how url could be specified without options, though, unless you call SimpleRest.setUrlForMethod(method, url) after, which seems uglier.

aldeed avatar May 27 '15 14:05 aldeed

Yeah, I guess Meteor.method sounds good - should we make a package called "meteor-method-options" or something, which gives people hooks to add options to this new syntax? Maybe it could work for publications as well. Maybe it also lets you add options to the context (like this.setUserId and this.userId)

Then, we can hook into that to do versioning for HTTP and DDP at once?

stubailo avatar May 27 '15 15:05 stubailo

In this new package, would it be possible to extend Meteor.methods to pass through options to the individual methods? I haven't thought it all the way through yet, but it would be very convenient for grouping endpoints. So I could, for instance, group all endpoints on a single route to share a set of options. It could help DRY up code where a bunch of endpoints are sharing a large subset of options, or just help users better organize their endpoints.

kahmali avatar May 27 '15 15:05 kahmali

That's a good point! But there are a lot of options that are specific to one method, for example versioning, argument parsing from query params, etc. On Wed, May 27, 2015 at 11:27 AM Kahmali Rose [email protected] wrote:

In this new package, would it be possible to extend Meteor.methods to pass through options to the individual methods? I haven't thought it all the way through yet, but it would be very convenient for grouping endpoints. So I could, for instance, group all endpoints on a single route to share a set of options. It could help DRY up code where a bunch of endpoints are sharing a large subset of options, or just help users better organize their endpoints.

— Reply to this email directly or view it on GitHub https://github.com/stubailo/meteor-rest/issues/30#issuecomment-105958795 .

stubailo avatar May 27 '15 15:05 stubailo

Whatever our solution for versioning, I hope it can do two important things:

  1. Allow an entire version of the API to share default configuration options (so if I want to make a major change across an entire version, like change the default auth mechanism, I can do that DRYly.
  2. Maintain the ability to auto-document the API between versions. If the schema of the request or response is changing between versions of an API (one of the most likely changes requiring a version bump, I imagine), that type of metadata will need to be stored on separate versions of the endpoint. I'm not quite sure this could work with what @aldeed is proposing as far as managing the version from within an endpoint. I really think the version needs to be a part of the endpoint metadata, and separate versions of the endpoint need to be maintained. If you're looking for the DRYest solution possible, maybe we can use cascading between versions of an endpoint, Grape-style? I don't think that's quite what @aldeed was referring to, but it solves some of the DRY problems.

EDIT: Also, just a side-note, but Meteor really dropped the ball calling those methods instead of endpoints.

kahmali avatar May 27 '15 15:05 kahmali

@kahmali why would you prefer endpoints? IMO a publication is also an endpoint.

stubailo avatar May 27 '15 16:05 stubailo

I would really like some concept of namespacing/nesting of methods and publications though. So basically you could define a namespace or module, and then apply options to all of those methods/publications at once.

stubailo avatar May 27 '15 16:05 stubailo

@stubailo: why would you prefer endpoints? IMO a publication is also an endpoint.

Touché. That was a fleeting thought from the REST perspective of the overlap with the HTTP "method", but from the DDP perspective it makes perfect sense. Shame on me. ::open mouth, insert foot::

I would really like some concept of namespacing/nesting of methods and publications though. So basically you could define a namespace or module, and then apply options to all of those methods/publications at once.

This is exactly what I was looking for! In addition to the methods/pubs, custom routes should also be allowed to be added to a namespace.

kahmali avatar May 27 '15 17:05 kahmali

I agree that namespaces and inheritance should be part of the picture, but I think we'll need to experiment a bit to figure out the best pattern for that.

@kahmali, regarding versioning, I like the idea of separate version converters, and maybe we can have code comments that override the base documentation for the current version:

/**
 * Get widgets for your userId or for everyone
 *
 * @param all {Boolean} Get all widgets?
 * @return Array of widget documents
 */
Meteor.publish("widgets", function (all) {
  if (all) {
    return Widgets.find();
  }

  // could also check this.apiVersion in here for advanced cases

  return Widgets.find({userId: this.userId});
}, {
  priorVersions: {
    /**
     * Get all widgets
     * @return Array of widget documents
     */
    "1": {
      in: function (apiVersion, args) {
        // In version 1, no args were accepted and "all" was the default
        return [true];
      },
      out: function (apiVersion, result) {
        // In version 1, property was called "foo_bar" instead of "fooBar"
        if (_.has(result, 'fooBar')) {
          result.foo_bar = result.fooBar;
          delete result.fooBar;
        }

        return result;
      }
    }
  }
});

Would require custom documentation tools, but just trying to think outside the box.

aldeed avatar May 27 '15 17:05 aldeed

We are considering Simple REST to build a REST API for our Meteor app, and share concerns around versioning. As noted above, methods, publications, etc might be subject to change, so our API consumers would appreciate some stability when building apps around our platform.

We are able to contribute development time to this issue, if we can reach a consensus around how Simple REST can support versioning.

brylie avatar Apr 20 '17 07:04 brylie

Maintain the ability to auto-document the API between versions. If the schema of the request or response is changing between versions of an API (one of the most likely changes requiring a version bump, I imagine), that type of metadata will need to be stored on separate versions of the endpoint.

@kahmali

To this idea, we have worked on an extension for Restivus that allows you to define an OpenAPI Specification (formerly Swagger) descriptor file for your API. We have discussed porting, or perhaps making again, this Restivus Swagger package in order to support Simple REST. Would that align with your needs?

See: https://github.com/stubailo/meteor-rest/issues/93

brylie avatar Apr 20 '17 07:04 brylie