wizcorp / express-rested Goto Github PK

From the documentation:

collection.addIndex(string propertyName)
Creates an index for all resources, current and future, in the collection.

and

Note 1: Creating an index is a heavy operation, so it's best done before adding/loading resources into a collection

We have a User resource and a Session resource. When creating a Session, we look up the corresponding userId by using a users.findOne() call, looking up the User resource by the username for which we have added an index.

However, if we then proceed to modify said User via REST a PATCH, we observe that the changes are persisted to our storage, but that the indexes are not updated to reflect the change.

Translated to our real life use case, this means that when we modify a user and change the username and password, the change is persisted, but when we log out and try to log in again using the modified credentials, we get a rested TypeError: Cannot read property 'id' of undefined as users.findOne() will return undefined as it cannot find the User using the modified username. When we kill our app and restart it again, the User collection and index are loaded from the persisted storage and we can proceed to log in using the modified information.

What is the best workaround or solution for this problem?

When users want to go cluster-wide, they need a backing store that a resource can be reloaded from with each request. Just like the persist-hook, we'll need a fetch-hook.

When edit() throws, it's quite likely that there was an input validation error. It's often vital that an error message is communicated back to the client (eg: "No name was provided"). We don't want to output a stack or anything, but error.message seems very reasonable. Perhaps text/plain is more appropriate here than a JSON object.

If an error object is thrown and it has a code property, we could add a response header: x-error-code for example, so clients can also programmatically deal with these errors.

That means:

• turn off all caching for non-GET/HEAD (apply http://richhewlett.com/2015/02/16/preventing-browser-caching-using-http-headers/)
• enable caching using last-modified timestamps (one for a resource, and the most recent one also at collection-level)

Update: this has been implemented at the collection level, not yet at resource level.

We should return error 500.

The same is probably be true for custom file type handlers.

Right now, certain details are unclear because they are not well documented (Location header, etc). Examples of each type of operation would be very nice. We also need to show how errors are communicated. Perhaps some cURL dumps would be good.

If you have a Resource that has properties in the form of an Array, you cannot perform a collection.findOne() or collection.findAll() on it. It will simply return an empty Array as the result.

Say you have a Resource that has a tags property, which is an Array of said tags, you might want to use findOne() or findAll() to find all the resources that match a given tag.

1:
  id: 1
  name: Heineken
  tags:
  - hoofdpijn
  - bagger
2:
  id: 2
  name: Grolsch
  tags:
  - bagger
3:
  id: 3
  name: Jupiler
  tags:
  - lekker
  - belgisch

beers.addIndex('tags');
beers.findOne('tags', 'belgisch'); // returns []
beers.findOne('tags', 'hoofdpijn'); // returns []
beers.findAll('tags', 'bagger'); // returns []

This is counter to common practice.
api/rest/mediaProfiles/ and api/rest/mediaprofiles/
Should be aliases of each other, not separate endpoints.

We accidentally ran into this today. I couldn't figure out how fix this at a glance, so I didn't. The discussion here at least confirms it should be insensitive, but it's not really a big deal.

We already use the query string for searches, so we might as well expose pagination (useful for large collections that have a paginated web UI in front of them).

Properties in the query string:

{
  "pageNum": 1,
  "pageAfter": "some ID",
  "pageSize": 10
}

You would browse using either pageNum or pageAfter. The former is more common, but the latter ensures no gaps exist between 2 pages while mutations happen. It does mean that after deletion of the resource pageAfter points to, it may not be able to paginate at all (404).

To enable this, we still need to first make getList() return a predictably-sorted array of resources.

It would be great to have an indexer built into collections to allow for quick lookups on more than just the ID.

Proposed API:

collection.addIndex(string propertyName);
collection.findOne(string propertyName, any value);
collection.findAll(string propertyName, any value);

This should simplify things a bit:

The Responder class seems rather redundant at the moment. If we give the ResponseContext a static fromReqRes(req, res) factory function, it could extract all information from the request, and call new ResponseContext() with that information. The instantiation of ResponseContext can happen straight from the routes file, and it can then pass the context object straight to operations/**/*.js handlers.

The collection.request method can then also disappear, and collections will be completely disconnected from REST in every way. You could even argue that the collections library could be externalized (but let's not go there for now).

I'm working on a project that basically does this:

const devices = require('../lib/resources').devices;
const prober = require('../lib/prober');

const device = devices.get('7fd590de-5726-436d-8c84-95e4738a9742');
prober.probeDevice(device);

When running this script in node, the device passed to probeDevice is still undefined.
when running these commands from the node REPL, it works fine.

Right now a GET /collection always returns a map. Sometimes an array may be more preferable and that should be a possibility.

This can be useful for example when dealing with a user object that contains credentials you wouldn't want to send over the wire. Imagine:

getJson(req, res) {
  const copy = deepCopy(this);
  delete copy.credentials;
  res.json(copy);
}

... because the rights default to false. So when people start an experiment they quickly run into trouble.

The version on npm says 1.5.1 but it's not actually the same as on github.
Telling was the missing piece of documentation on errors (and the missing event emitter in the actual source).

API could work as such:

const brewers = rest.add(Brewer, '/brewers');
const beers = brewers.embed(Beer, '/beers');  // a virtual collection type, because each brewer will have its own physical collection of Beer resources

URL: GET /rest/brewers/Suntory/beers/Premium

Challenges:

• brewers is a collection, not a Rest class and does not have access to the router (so can't make a sub-router either)
• We'll want resource classes like Brewer to stay simple and not get child properties for its child-collections.
• The Beer resource should maintain simple IDs, but they fall under the parent collection's resource's namespace. How do we deal with the addressing, loading and saving?
• add and embed do virtually the same, yet they're named differently.

Small sidenote:

We probably don't want to support the random creation of collections (eg: POST /brewers/Suntory), as it's a pain when it comes to rights management, as well as the fact that it doesn't fit this library at all. Great for a file system wrapper perhaps, not so great for a more DB like system (you don't let users add schema).

In the documentation:

collection.setAll(Class resources[], Function cb)
Deletes all resources not given, and creates or updates all resources given in the resources array. Triggers the persist callback.

In the source:

// collection.js
constructor() {
	this.map = {};
	// ...
}

setAll(resources, cb) {
	const previous = this.map;
	this.map = resources;
	// ...
}

The documentation says it should be an array, but it should actually be a object/map.
I'm not sure which is 'wrong'.

Since a Collection<Foo> keeps track of the prototype, it would be nice if it was automatically used for anything new. In a case where I iterate over a number of these resources I don't want to have to redo this every time, in every place I intend to use these methods.

I currently have code like this

for (const id of collection.getIds()) {
    let resource = collection.get(id);

    if (!(resource instanceof collection.Class)) {
        resource = Object.create(collection.Class.prototype, resource);
        collection.set(id, resource);
    }

    // safely use resource
}

If resource constructor() and edit() can accept an extra argument (callback), people can do asynchronous operations and send back asynchronous errors if there was a failure.

This one is low priority.

Document it with a big warning on how to use it.

It could be interesting to manage, say, JPEG images and whatnot. Also, in the case of video for example, one could imagine the resource being passed in or out could be a stream. This needs to be based on file extension first and foremost. What we can do is detect when there is a file extension, but it's not ".json".

In those cases, we can call:

resource.handle(ext, req, res);

And expect the resource to take care of the rest. If it throws, we can still return a 4xx error (when the file extension is not supported for example). Alternatively, that's the resource's responsibility and a throw returns a 5xx. We leave it to the function to deal with accepting or not the HTTP method uses (which is req.method).

One alternative is that we uppercase the file extension (or its first letter) and call methodExt:

resource.getJpeg(req, res);
resource.putJpeg(req, res);

That way we'll know before call-time whether the extension is supposed to be supported or not.

We need to apply encodeURIComponent, for example on the id in the Location response header.

One way we could implement search is this:

GET /rest/collection?foo=bar

The moment we receive a query string on a collection-get that can be parsed to an object, we call resource.matches(obj) on each resource in the collection and return only the matching resources. If matches is not implemented, we don't filter and return all resources.