addCollection exposes complete collection, cannot override

[satyavh]

Following the docs, I exposed only 1 endpoint for my collection:

  Restivus.addCollection Collections,
    endpoints:
      get:
        authRequired: true
        action: ->
          col = Collections.find(members: @userId)

          if col
            status: "success", data: col
          else
            statusCode: 400
            body: status: "fail", message: "Unable to get collections"   

but the complete collection is exposed. So I did some experiments - the following all expose the complete collection:

I added

excludedEndpoints: ['get']

but it still exposes the get call and the complete collection.

I tried returning just 'success', no data. Still I the call returns the complete collection

          if col
            status: "success"
          else
            statusCode: 400
            body: status: "fail", message: "Unable to get collections"   

I removed col = Collections.find(members: @userId) and it keeps exposing the complete collection.

Now it gets even more interesting, I removed

authRequired: true

and I still get everything!

So I thought, lets try this with a customer route, following the docs:

  Restivus.addRoute 'collections', authRequired: true,
    get:
      action: ->
          col = Collections.find(members: @userId)

          if col
            status: "success", data: col
          else
            statusCode: 400
            body: status: "fail", message: "Unable to get collections"  

And guess what, I always get a

{
  "status": "error",
  "message": "You must be logged in to do this."
}

while I just succesfuly authenticated via /login and the calling the endpoint with the headers as per the curl examples in the docs.

I don't know, this package seems very buggy. I also doubt @userid and @user being available in the endpoints. Doesn't log anything to console.

Any ideas?

[kahmali]

There is so much here I'm not quite sure where to begin. I may have to answer this in parts, but I'll try to get everything in one shot. I'll start by trying to resolve the actual issue (as specified in your title).

Per the documentation here and here, there are two ways to exclude a collection endpoint from being generated. You can either add the name to the list of excludedEndpoints or set the endpoint to false using the endpoints option, like this:

Restivus.addCollection Collections,
  endpoints:
    get: false
    post: false

Any other endpoints will be generated. So when you say:

Following the docs, I exposed only 1 endpoint for my collection

  Restivus.addCollection Collections,
    endpoints:
      get:
        authRequired: true
        action: ->
          col = Collections.find(members: @userId)

          if col
            status: "success", data: col
          else
            statusCode: 400
            body: status: "fail", message: "Unable to get collections"   

That is actually not what the docs state or the intended behavior. Here, Restivus did exactly what you told it to do. You told it to generate every endpoint with default options, except for the get endpoint, which you completely overrode the default behavior on by providing your own action. So in this case you get all the endpoints generated without auth or role requirements, except for the get, which requires auth and will have the behavior you've defined there.

Next, you say:

I added

excludedEndpoints: ['get']

but it still exposes the get call and the complete collection.

I tried returning just 'success', no data. Still I the call returns the complete collection

          if col
            status: "success"
          else
            statusCode: 400
            body: status: "fail", message: "Unable to get collections"   

I removed col = Collections.find(members: @userId) and it keeps exposing the complete collection.

Again, it appears you are misunderstanding the intended behavior (described in those links to the docs above) of the excludedEndpoints option. In addition to the documentation, which states that it's a list of "names of the endpoints that should not be generated", I would hope that the name "excludedEndpoints" would intuitively be a list of endpoints that will be excluded from the routes. So in your example, every endpoint except the get endpoint should be generated. I say "should be generated" and not "is generated" because here, you've actually detected a bug. So thank you for that!

The problem was that if you defined any excludedEndpoints, but did not configure options on any endpoints, a check was missed (in an attempt to prevent a bunch of unnecessary subsequent checks from being made) that caused all endpoints to be generated with the default behavior. If you need a temporary fix, you can just exclude endpoints by setting them to false, instead of using the excludedEndpoints option. I will push up a quick fix for this tonight (I've already fixed it, I just need to run a few more tests and then I'll publish). I'm in the process of putting together a testing suite with Tinytest, and these are the kinds of bugs that will be much easier to detect once that's ready (some version of the suite will be up in about a week or so). In fact, I located this bug by setting up a test to help me get to the bottom of it. Once the test suite is up, I would love it if folks wanted to contribute by extending the test suite to cover the things I'm sure I'll miss.

So I thought, lets try this with a custom route, following the docs:

  Restivus.addRoute 'collections', authRequired: true,
    get:
      action: ->
          col = Collections.find(members: @userId)

          if col
            status: "success", data: col
          else
            statusCode: 400
            body: status: "fail", message: "Unable to get collections"  

And guess what, I always get a

{
  "status": "error",
  "message": "You must be logged in to do this."
}

while I just succesfully authenticated via /login and the calling the endpoint with the headers as per the curl examples in the docs.

As far as your tests with the authentication, you'll have to provide me some more sample code or a reproduction repo. I think all of your other testing might have confused you by that point, and it makes sense to blame the package for everything that doesn't seem to work. All I can tell you is that I'm using the default authentication extensively in my own project, and I'm 100% confident that it works exactly as described in the docs.

I don't know, this package seems very buggy.

I'm not sure if you're aware, but all Meteor packages use semantic versioning. As you can see, this package (and every other REST framework in Meteor) is currently at a major version of 0, meaning the API is to be considered unstable. I can (sort of) understand the complaint if this were past it's 1.0, but by it's very definition it's going to be buggy. What I see here in this issue is a single bug that you've found in a package that is doing some heavy, repetitive lifting for you. I'm just a college student (and startup founder) that needed a REST solution for a what I hope to become a production level Meteor application. I'm one person trying to maintain this code and documentation and make this package powerful and flexible enough not just for my needs, but hopefully for the needs of many other Meteor developers. You're always welcome to try some of the other solutions out there (check out Meteorpedia for a pretty comprehensive list), and if none of those are suitable, enrich the Meteor community by rolling and releasing your own. I'm not sure what else to say here. If you look at all of the other issues raised thus far, I think you will see that I'm in no way defensive of constructive criticism (in fact, I think you'll see that I'm extremely responsive to it), but this isn't constructive.

I also doubt @userid and @user being available in the endpoints.

As I mentioned, I'm using the default authentication extensively in my own project, and I'm absolutely certain that @user and @userid both exist in an authenticated endpoint context. I checked out a few of my endpoints and @user and @userid are working perfectly with the default auth. If you don't authenticate properly, they won't be available. I also know of other users that have confirmed the availability of @user in authenticated, as in Issue #4.

Doesn't log anything to console.

To this day, I've only used a single Meteor package that has logged anything to the console, and that was CollectionAPI to notify the user that the API was up and running. I did the same for Restivus once it's configured, but even that I questioned. I'm no expert, I'm just trying to follow the example set by the rest of the community. The real solution is a complete testing suite, which I'm working on now. But I'm also working on graduating college and launching my startup, so any help is welcome. I will gladly accept pull requests. Meteor needs a great REST framework, and we, the community, have to take some responsibility for making that happen! I'm trying to do my part as best I can.

If you feel that the documentation is unclear or inaccurate, please raise an issue for those sections and I will do my best to make them as clear as possible. This project and the documentation are very dear to me, and my intention is for them to help a lot more than they hurt. I'll let you know once the fix is pushed. Thanks again for reporting the bug! Please feel free to open another issue if anything else comes up in the meantime.

[kahmali]

I just published v0.6.2, which should hopefully fix the issue. Let me know once you have a chance to update and test it out.

[satyavh]

Thanks for your extended answer. But I think I've not been clear. When I say 'exposed the collection', I don't mean exposing the endpoints.

In all my examples, Restivus is exposing all the fields (records) of the collection (database) when quering the api - regardless of any limitations on that query.

I'll make another issue for authentication and the @userid

[kahmali]

There seems to be a bit of confusion with your terminology. In Mongo, fields are not the same as records and collections are not the same as databases. A database is an "instance" of Mongo and consists of a set of collections (e.g., Meteor.users and any others you define with new Mongo.Collection()). Collections consist of a set of documents (anybody would know what you meant if you called them records though), each with a unique _id field. Each document contains a set of fields, which are the equivalent of fields on JSON objects.

That being said, is your issue that all the fields are being exposed on a document, or all the documents are being exposed on a collection? It sounds like the issue is that all the documents are being exposed on a collection. Can you show me what URL you are making the request to? None of your example code above would have the effect of preventing the entire collection (all the documents/records) from being returned. I have a feeling that you're making the request to GET api/collections even though the endpoint you're overriding is for GET api/collections/:id, which will get a document with the specified :id. If you want to override the endpoint that returns all the documents in the collection, you should be overriding the getAll endpoint. See the section on collection endpoints for a complete list of all endpoints, including a description of each, the HTTP method, and the corresponding URL path. If I'm still misunderstanding you, please just provide me with as much sample code (especially any request information) or a reproduction repo if it's not too much trouble, and we'll get to the bottom of this.

[satyavh]

Haha, terminology is not my strongest point, but you get what I mean. All the documents were exposed.

You were 100% right, I really thought Restivus followed the standard http methods, and as there is no http GetAll method, I was overriding Get instead. Sorry, but the docs didn't make it clear to me that GET needs :id or otherwise returns all documents of the collection. I thought without id Get would just return everything and you could override that behaviour in the Get method.
(This makes me think, following Restivus logic, without :id 'Get' should actually return nothing, shouldn't it?)

So the good news is, I now get the data out with CURL incl authentication headers - which is super awesome.

And you're right, I shouldn't have called this package buggy. I really appreciate your quick responds and help, and putting effort and time in making meteor packages!

[kahmali]

Thank you, Satya. Just to explain the design philosophy behind the collections in Restivus (and thus, why the endpoint names do not all map directly to the HTTP methods):

As I explain in the docs, when you add a collection in Restivus (and it would be the same in any other framework that follows basic REST conventions), two routes need to be created: one for operations on the entire collection (adding a new entity, retrieving and deleting the entire collection), and another for operations on a single entity (updating, retrieving, and deleting an entity).

There is only one other package that I know of that exposes CRUD endpoints on collections, CollectionsAPI, which this package is partially inspired by. If you've used CollectionAPI or check it out, you'll see that it has very limited functionality. One of the things that bothered me about it was that you didn't have separate control over getting a single entity or getting the entire collection (they didn't allow you to delete an entire collection, which I will address in a bit). You either enabled both, or disabled both.

The goal of Restivus is to provide a clean, flexible framework that adheres to the best practices in REST, and in order to do so it needs to allow for separate configuration of each of the collection endpoints. That way you can set different auth and role requirements (and any other future configuration options) on each endpoint. In designing the API, I was faced with a few decisions, a few that I think will stick all the way to 1.0, and a few that are very likely to change. One that I think will stick was the decision to keep the collections at a slightly higher level of abstraction than the custom routes. The custom routes do in fact map their endpoints directly to their corresponding HTTP methods, which makes perfect sense in that context since you're defining a single route (which should support the full range of methods). With the collections, only specific endpoints should be defined on each route in keeping things RESTful, but there are overlapping HTTP methods that need to be supported for the two routes that are generated (GET and DELETE). I could have set the API up like this:

Restivus.addCollection Items,
  collectionEndpoints:
    get:
      authRequired: false
    delete:
      roleRequired: 'admin'
  entityEndpoints:
    get: 
      authRequired: false
    delete:
      roleRequired: 'admin'

But then that would require the user to be aware of which endpoints were supposed to be defined on collections vs entities (dragging users down to a lower level of abstraction), and would also make for a dirtier API. In trying to abstract as much away from the user while using the best REST practices, I went with the existing solution, which simply appends "All" to the colliding endpoint names (e.g., getAll and deleteAll). I feel like the names are appropriate, and I do believe the documentation makes this very clear. This way you get a clean API, internally and externally. I will of course continue to review the docs (I'm constantly updating them) and see if there's any way I can make things clearer, but I don't think anything should be assumed about the API without reading the docs (which I work very hard to make complete and awesome).

One of the parts of Restivus that will likely be changed or removed completely is the deleteAll endpoint. It's not a REST standard, and I only ever use it for development purposes. I will probably make it opt-in at the very least, but I'm leaning towards removing it entirely. If you or any other users have any opinion on that I'd be glad to hear it.

So that was some general philosophy. Just to wrap things up and address your response directly:

I really thought Restivus followed the standard http methods, and as there is no http GetAll method, I was overriding Get instead.

Restivus does use the standard HTTP methods in custom routes and under the hood in collection routes (and on the surface as much as possible), but there is a collision on the GET endpoints that need to be defined -- one on the collection and one on the single entity.

Sorry, but the docs didn't make it clear to me that GET needs :id or otherwise returns all documents of the collection.

If you have a way for me to make it any clearer than this, I will gladly accept a pull request. That's the best I can do for now, and it seems very clear to me. If I get more complaints or confused users I will certainly address it. Here are the snippets from the docs on the GET endpoints:

• getAll Endpoint
  • GET /api/collection
  • Return a list of all entities within the collection (filtered searching via query params
    coming soon!).
• get Endpoint
  • GET /api/collection/:id
  • Return the entity with the given :id.

I thought without id Get would just return everything and you could override that behaviour in the Get method.

Without an :id, GET requests do return everything. It would be limiting, confusing, and pretty terrible under the hood to do what you're suggesting. You would only have a single endpoint for two separate routes, which means the same function running on two different routes, and then using a check within the route to see if an :id were provided, and then having some disgusting if-else that users would then need to understand and use. Absolutely not. That's not getting anywhere close to this API. If this is unclear to you, just think of how you would implement this and it should become much clearer. You need two separate endpoints for getting a single entity vs getting an entire collection. In addition to the actual endpoint action that's executed, your suggestion would also limit you to applying the same configuration options on both routes.

(This makes me think, following Restivus logic, without :id 'Get' should actually return nothing, shouldn't it?)

Not sure what you mean here, but no, without an :id, GET should return the entire collection, as per the standard REST methods.

I'm glad we got to the bottom of your issue, and even accidentally discovered a bug along the way! I'm going to go ahead and close this issue now that it's resolved, but feel free to open another issue if you need to further discuss the design philosophy of Restivus. I'm very open to suggestions from the Restivus community.