Keystone-classic: Call to action - Please review the new API Spec

[JedWatson]

I've written up a spec for the new Admin API and would _really_ like some feedback on it.

The endpoints in the spec are all implemented and in use, and I've cleaned them up slightly as I've been writing up how they work.

The next part of the plan is to make these generally available through List methods (so you can customise them and easily add user-facing versions for your app) so it's really important to get them right and make sure the design is one that we're happy to live with, so please shout out with any feedback you've got (PRs are also welcome!)

Specs are here: https://github.com/keystonejs/keystone/blob/master/admin/server/api/Readme.md

cc @mxstbr @molomby @josephg @wmertens @creynders

[wmertens]

You repeated the Sign In header

[JedWatson]

@wmertens thanks! fixed.

[wmertens]

How are errors for nested data handled? E.g. you have an schema with an array of objects and there should be at least 2 items in the array, and there is only one item and it is missing one of the values in its object?

[JedWatson]

That would be up to the Field to define how it provides validation errors - see the example for the password and email fields. An array-type field (e.g TextArray) would do something similar, for nested schema fields I expect it would nest a validation errors array.

[wmertens]

Suppose we make natural keys possible, e.g. a list of countries uses the international 2-letter code, how can we change the API? GET/POST /api/{list}/key/{key}?

[JedWatson]

Why would we want to do that for the Admin UI?

In user land you could bind the routes to whatever you like in the express router, and probably provide a custom function for querying an item (default to findById)

... but yes, given we've got a concept of autokey on lists, we could support that kind of URL if we wanted to.

[wmertens]

Also, I wonder if the API responses should not be a level lower. If a list has an error field, that could falsely trigger error code when getting an answer.
So responses could be {ok:<answer>}. If you don't get a JSON object with a single ok key back, you know you weren't talking to the Keystone API.

[wmertens]

I also wonder if it's really necessary to use HTTP error codes. A proxy could misunderstand, and if you do opportunistic getLoggedIn calls that respond with 401 the browser prints that in the console.

If however the answer is always a JSON object with extra keys for the various error conditions, you always know you are talking to the API and you don't try to parse what a captive portal sends you.

[wmertens]

Meaning, if the response is anything but 200, you know there is an error with the network or service. Actual API errors are handled by the UI code, not by the browser.

[wmertens]

Is there a reason to keep the item results in the legacy format? I never understood why the fields are sent as their own object under fields.

[geloescht]

Maybe the concept of expandRelationshipFields could be expanded a little, to also allow population of only certain relationship fields. Should the API return a top-level error if a field cannot be populated (due to a missing referenced item) or a nested error?
I agree with wmertens that itens should never be returned at the top level of JSON data. Apart from err, we might want to add other stuff such as timing statistics, that might collide with field names. I like status codes, but I am not an expert on how they interact with proxies.

[wmertens]

On Mon, Jul 25, 2016 at 6:03 PM geloescht [email protected] wrote:

I agree with wmertens that itens should never be returned at the top level
of JSON data. Apart from err, we might want to add other stuff such as
timing statistics, that might collide with field names. I like status
codes, but I am not an expert on how they interact with proxies.

The way I see it, HTTP status codes are meant for generic interaction
between browsers and servers. However, in this case we have very specific
interaction of a client implementing the Keystone API and Keystone. HTTP
error codes just add extra state to the interaction with no benefit I can
think of. In the end, the client expects the promise for data to settle
with the data or a rejection, and that collapses the errors again into a
single error state.

Another reason: Doing it this way makes the API transport agnostic, so you
could easily implement API calls in React server side rendering as direct
keystone calls and the reducers would get the same data.

[webteckie]

In the Query Params for Get Items, doesn't it make sense to have expandRelationshipFields be either a Boolean OR a list of fields? What if a client wants more than just {id, name} and wants to avoid extra round trips?

[webteckie]

In the Duplicate User case, why is it a good idea to return back all info about the duplicate user?

[wmertens]

There's also #558 - and the json-api spec also proscribes a sub-object for data.

I'm not sure that implementing the full JSON-API spec brings anything interesting though.

[christroutner]

As a developer that uses KeystoneJS on a daily basis, I just want to say
thank you for creating this spec. I can't wait to see it implemented. It's
going to make my life a lot easier!

[JedWatson]

Thanks @christroutner, glad to hear it! In good news: the spec is already implemented, so you can start testing it out today (although while it's still tied into the Admin UI, you need to be signed in which may or may not be viable depending on your project. going to give you the ability to expose a custom version of it in your application routes very soon!)

[JedWatson]

@wmertens

Is there a reason to keep the item results in the legacy format? I never understood why the fields are sent as their own object under fields.

To separate the "root" concepts of id and name, and the structured values in the fields. Keystone always wants to know the ID and Name of documents, regardless of what their paths in the schema are. This keeps the actual schema isolated.

Similar argument to the one JSON API makes (maybe we could align them more closely?)

The way I see it, HTTP status codes are meant for generic interaction
between browsers and servers. However, in this case we have very specific
interaction of a client implementing the Keystone API and Keystone. HTTP
error codes just add extra state to the interaction with no benefit I can
think of.

I'm actually hoping to standardise this as something that can be used in projects as well. So if you're writing a React Native app with a Keystone back-end, just tell keystone to generate you the API Endpoints you want and off you go. Ideally, everything (including status codes) should behave as close to "best practice" and consistently as possible

Trying to remove as much of the special behaviour in Keystone as possible, without overcomplicating things or losing the bits that make it really nice to work with. Hard line to walk :wink:

@geloescht @webteckie being able to specify only some paths in expandRelationshipFields makes sense in theory, but I couldn't actually think of a real-world case where it would be useful to support that complexity. If you want field data out of relationships, at the moment I think it's best to keep it simple in the API and make additional calls to fetch the related items. Otherwise things get recursive pretty quickly, and flattening out the options for that gets really complex / custom / magical. We can revisit that decision once 0.4 ships, though!

I agree with nesting data in the return structure, will update the spec accordingly. It'll make the GET {list} and GET {list}/:id endpoints more consistent too, I guess, which is nice!

In the Duplicate User case, why is it a good idea to return back all info about the duplicate user?

It's not, really, that's just me enumerating the complete data we get back from mongoose. Any lower-level error like that is passed back as mongoose (or the driver) provides it. Not ideal but I'd prefer not to add any additional middle-man processing at this stage on lower-level errors that could occur.

[wmertens]

Ok, so for status codes I suppose that they can reflect the error that is in the object, but the error itself should be sufficient to map to the status, and the rest of the application layer should have no idea about response objects or error codes.

[wmertens]

In another project I made subclasses of Error for various types of errors, and the rest of the code returns promises for values. Any errors are thrown, and the Express API has a wrapper to call the functions, returning the value as json and sending the errors as json too. Nice separation of concerns.

This is also more or less how Koa 2 works, as far as I understand.

[VinayaSathyanarayana]

Can we have the documentation as a swagger/open api spec --> http://swagger.io/
If there is a swagger spec, one can leverage automatic code generation facilities of swagger.

[ctaepper]

you are not following the REST specs, but i assume you know that and the whole thing is not supposed to be a full blown REST API?

• when creating a new item in list, one should POST /api/{list}
• when deleting an item, one should DELETE /api/{list}/{id}
• when updating an item, one should PUT /api/{list}/{id} containing the whole item, or PATCH /api/{list}/{id} containing only the properties which get updated

[wmertens]

Frankly, I have yet to see a benefit from closely adhering to rest. Api calls don't get easier, in fact the opposite. If you make all calls require the same verb, that's less code to maintain. Overlapping routes are cute, but each call could have its own endpoint and it would be less code. POST /list/create, POST /item/read/id etc. You could add aliases for GET so just browsing also works.

[ctaepper]

@wmertens i'm sorry, i don't want to start an argument, but you clearly never had to consume a large REST API or even developed one yourself. Surely, technically you can just use POST for all endpoints... you can also only use DIV's everywhere in you html... but then you totally lack semantic meaning of your markup, or - in this case - the route definitions of your API

[mxstbr]

but you clearly never had to consume a large REST API or even developed one yourself.

No need to start insulting people here @ctaepper, refrain from doing that again, keep it technical please. This is a professional discussion, not a mud-slinging contest.

[ctaepper]

sorry, this was not supposed to be an insult. i only tried to make my point about semantics as clear as possible :)

[wmertens]

@ctaepper I have used the biggest APIs around (Google, AWS, …) and they don't even bother with REST, they provide native bindings.

I also worked with the Lithium API, very REST-y, but they allowed using POST instead of GET to keep the cooe simple, and they suffered from not having a version number in their API as well as putting huge collections in recursive relations that then could not be paged properly.

So, REST imho is pretty useless and works just as well as other things,

@JedWatson we definitely need to prepend a version number to the API. Also, calls to multiple versions should remain possible until deprecation.

[wmertens]

Sorry, Lithium does have version numbers but they refused to upgrade their API.

Contrast that with GraphQL which is clearly superior to REST in everything except ease of use from an address bar. No versions needed, just add mutations and deprecate old ones.

[JedWatson]

@ctaepper yep, I'm aware this is only "restish", and whether that's a good idea or not is part of the peer review I'm looking for here.

TL;DR:

I don't think we can conform entirely to formal REST so I've used GET for fetching data and POST for everything else and tried to put semantic meaning in the URL structure instead.

Background:

As far as I'm aware, there are _still_ ways in which using REST verbs properly lead to painful interactions with browsers and proxies. Keystone's not aware of the environment it's operating in (could be running inside any infrastructure / behind any proxy) so keeping to the "safe" methods (GET, POST) seemed prudent. I don't have any examples to cite off the top of my head, if that's an outdated position I'm happy to hear it.

Aside from that, formal REST seems too limited / too specific to me...

Take the delete endpoint for example: you can delete a single ID or an array of IDs (provided in the POST body). Using the DELETE method causes problems here; you can DELETE /api/{list}/{id} but you shouldn't send body data with DELETE requests so the multi-delete would need a different verb. I find that really counter-intuitive.

Another example is updating items:

• I haven't added a spec for it but we have a draft implementation of an endpoint you can call to update multiple items. It's currently /api/{list}/update, and an array of items (including ID and new data) are sent in the body. This doesn't fit either PUT or PATCH semantically; POST is generally accepted as the "perform an action" method, which would be correct here. Updating a single Item with PUT and multiple items with POST seems more inconsistent than just saying "this API is REST inspired" and using POST for actions that modify data.
• The API as designed actually doesn't discern between PUT and PATCH requests; if you don't provide data for a field, it won't be updated. I'm not sure which one would be semantically correct in this case.

There are other areas where we may even need to support POST when we mean GET - an example that comes to mind is fetching exports, where the possible combination of options and filters you can specify could exceed what's generally considered safe to encode in a URL. This kind of fills me with dread and I don't like it, but REST is unfortunately a really limited spec for modern apps.

All of these concerns, taken as a whole, suggest to me that we're better off putting our verbs in the URL where we control the semantic meaning, using GET where possible and POST where the action (a) has side-effects, or (b) needs to send data in the body.

you can also only use DIV's everywhere in you html... but then you totally lack semantic meaning of your markup

This is thankfully no longer true; additions have been made to the HTML spec that allow us to define semantic meaning with any markup, which doesn't mean you shouldn't use semantic markup where possible but allows you to work around it when the standards / implementations are insufficient or have unintended side-effects[1].

I guess I'm saying that I view specifying a "restish" API the same way; we're using the parts that consistently fit (status codes, GET for fetching data, POST for actions with side-effects) but it's not a total buy-in because I think that would be just as confusing.

[1] for example, we don't use <input type="number"> for Numeric inputs because on iOS you can't enter a decimal, which is really broken.

[JedWatson]

By the way I'm open to being told I'm wrong here (with a convincing argument :wink:), I'm just explaining my reasoning for the current design.

[ctaepper]

@JedWatson actually you are right, the company proxy of one of our clients doesn't allow DELETE for regular users, so this can always be an issue. We had to workaround using POST

anyways, i already assumed your spec didn't come out of the blue and everything is intended the way it is. i'm just a REST fan using REST frameworks, and it always sucks to discover APIs, which call themselves REST APIs, but don't follow REST specs. You never claimed this is a REST API, so everything is fine :)

Also this only seems to regard the Admin API. I have a custom Frontend API layer in my keystone instances anyway, and this is the part where i need my REST compliant API.

[wmertens]

For reference, here is the cosmicjs api: https://cosmicjs.com/docs

Note their use of a version number.

Btw @jedwatson, you can alias everything too. No reason to only allow one
way for calling the API.

On Wed, Jul 27, 2016, 12:40 PM ctaepper [email protected] wrote:

@JedWatson https://github.com/JedWatson actually you are right, the
company proxy of one of our clients doesn't allow DELETE for regular
users, so this can always be an issue. We had to workaround using POST

anyways, i already assumed your spec didn't come out of the blue and
everything is intended the way it is. i'm just a REST fan using REST
frameworks, and it always sucks to discover APIs, which call themselves
REST APIs, but don't follow REST specs. You never claimed this is a REST
API, so everything is fine :)

Also this only seems to regard the Admin API. I have a custom Frontend API
layer in my keystone instances anyway, and this is the part where i need my
REST compliant API.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/keystonejs/keystone/issues/3209#issuecomment-235550090,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADWlqnIxCZmJOpftPEB-f5bcvNbbHkDks5qZzWfgaJpZM4JUObx
.

[webteckie]

@JedWatson above you said:

[1] for example, we don't use for Numeric inputs because on iOS you can't enter a decimal, which is really broken.

but we went thru it once and added type inference.. For example, see https://github.com/keystonejs/keystone/blob/master/fields/types/number/NumberField.js

If that's not what you are referring to my apologies :=)

[JedWatson]

@webteckie oh for goodness sake... that's a bug. The constraints (input should be [0-9\.\-]*) should be enforced in React, and that type=number shouldn't be there. I also spoke to @jossmac about stepping the value up and down on arrow-up/down keypress. But I'll open another issue for it, we're getting off topic :)

[christroutner]

I recently had to get over this non-REST API hurdle the last few months
trying to integrate the ConnextCMS http://connextcms.com Backbone front
end to KeystoneJS. Once I figured out how to write my own API handlers in
Backbone, it wasn't a big deal, but it was a painful experience to go
through. That's why I posted my API tutorial
https://groups.google.com/forum/#!searchin/keystonejs/api$20chris.troutner/keystonejs/7ub1euleVOY/4EO5KOySAwAJ
on the Keystone mailing list.

I think the obvious value of sticking to the REST specification is that the
API will work with frameworks out-of-the-box (Angular, Backbone, Ember,
React?, etc). If the goal is to have KeystoneJS adopted by as many
developers as possible, then making the API REST-compliant seems to be in
line with that goal. I like the idea of creating aliases so that we can
have a our cake and eat it too.

-Chris Troutner

On Wed, Jul 27, 2016 at 4:54 AM, Jed Watson [email protected]
wrote:

@webteckie https://github.com/webteckie oh for goodness sake... that's
a bug. The constraints (input should be [0-9.-]*) should be enforced in
React, and that type=number shouldn't be there. I also spoke to @jossmac
https://github.com/jossmac about stepping the value up and down on
arrow-up/down keypress. But I'll open another issue for it, we're getting
off topic :)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/keystonejs/keystone/issues/3209#issuecomment-235563557,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AJaejz_iz1hDy_lk6HRimfnsmrQbbaRzks5qZ0bfgaJpZM4JUObx
.

[ctaepper]

I think the obvious value of sticking to the REST specification is that the
API will work with frameworks out-of-the-box

this 👍

[VinayaSathyanarayana]

Please see my comment https://github.com/keystonejs/keystone/issues/1612#issuecomment-193369557 and @ericelliott response https://github.com/keystonejs/keystone/issues/1612#issuecomment-193671350

[wmertens]

So in theory using a generic rest api would be nice, but in practice I have
not seen that in use. I would happilly be proven wrong.

On Wed, Jul 27, 2016, 17:26 VinayaSathyanarayana [email protected]
wrote:

Please see my comment #1612 (comment)
https://github.com/keystonejs/keystone/issues/1612#issuecomment-193369557
and @ericelliott https://github.com/ericelliott response #1612 (comment)
https://github.com/keystonejs/keystone/issues/1612#issuecomment-193671350

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/keystonejs/keystone/issues/3209#issuecomment-235621960,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADWlmfb423Hrhnp6ySU1p_TDIdLFrRkks5qZ3i8gaJpZM4JUObx
.

[christroutner]

Here is a link to the documentation that the Backbone.js expects to work
out-of-the-box with a REST API:

http://backbonejs.org/#API-integration

On Wed, Jul 27, 2016 at 10:15 AM, Wout Mertens [email protected]
wrote:

So in theory using a generic rest api would be nice, but in practice I have
not seen that in use. I would happilly be proven wrong.

On Wed, Jul 27, 2016, 17:26 VinayaSathyanarayana <[email protected]

wrote:

Please see my comment #1612 (comment)
<
https://github.com/keystonejs/keystone/issues/1612#issuecomment-193369557>
and @ericelliott https://github.com/ericelliott response #1612
(comment)
<
https://github.com/keystonejs/keystone/issues/1612#issuecomment-193671350>

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<
https://github.com/keystonejs/keystone/issues/3209#issuecomment-235621960
,
or mute the thread
<
https://github.com/notifications/unsubscribe-auth/AADWlmfb423Hrhnp6ySU1p_TDIdLFrRkks5qZ3i8gaJpZM4JUObx

.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/keystonejs/keystone/issues/3209#issuecomment-235654997,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AJaej5eaxeQp1Y6Dlm_EqbcfZXYQ-FIkks5qZ5JLgaJpZM4JUObx
.

[VinayaSathyanarayana]

Should we have an option that has "restApi" : ["strict" or "relaxed"] so that we can support our user base

[JedWatson]

Should we have an option that has "restApi" : ["strict" or "relaxed"] so that we can support our user base

I don't think this is an issue, because users will be able to configure their API however they like, and the Admin API is for our own internal use.

Soon, the middleware behind each endpoint will be available so you can create your own API from the functionality I've described. This means the HTTP methods and endpoint locations will be completely within everybody's control. It will also have (at minimum) the ability to customise options like fields to be updated / returned.

e.g.

app.get('/api/users', User.createAPI('list', { excludeFields: 'password' }));
app.get('/api/users/:id', User.createAPI('details', { excludeFields: 'password', idParam: 'id' }));
app.post('/api/users', User.createAPI('create', { fields: 'name, email', data: { isAdmin: false } }));
app.put('/api/users/:id', User.createAPI('update', { fields: 'name, email' }));
app.delete('/api/users/:id', User.createAPI('delete'));

More relevant is the expectations of data structure in & out of each endpoint. Backbone, for example, wouldn't work out of the box because it expects all data to be nested inside a root object, whereas we nest data inside a fields object.

What I really want thoughts around is the internal structure of data the endpoint is working with.

e.g.

For the GET endpoint, should it look like:

// GET Post example
{
  id: '123',
  name: 'Hello World',
  fields: {
    title: 'Hello World',
    slug: 'hello-world',
    content: '<p>hello</p>',
    image: {
      /* Image field data */
    }
  }
}

What about the Update endpoint? How do we deal with JSON vs. FormData structures?

etc.

[wmertens]

@JedWatson, I still don't understand why we need to support FormData in
what was previously a private API.

Summary of what I think should go in the spec/implementation:

• calls should have version in the path somewhere
• result object should be sufficient to determine HTTP response
  
  
  
  • Express should be a layer independent from the rest of Keystone,
    
    allowing for multiple ways to interact with KS
  
  
  • Incoming files should be presented as a file stream
  
  
  • Outgoing files too I suppose, with metadata
  
  
• Json post, or optionally get with url params for read-only ops (params
  mapped to incoming request object)

On Thu, Jul 28, 2016 at 6:47 AM Jed Watson [email protected] wrote:

Should we have an option that has "restApi" : ["strict" or "relaxed"] so
that we can support our user base

I don't think this is an issue, because users will be able to configure
their API however they like, and the Admin API is for our own internal use.

Soon, the middleware behind each endpoint will be available so you can
create your own API from the functionality I've described. This means the
HTTP methods and endpoint locations will be completely within everybody's
control. It will also have (at minimum) the ability to customise options
like fields to be updated / returned.

e.g.

app.get('/api/users', User.createAPI('list', { excludeFields: 'password' }));
app.get('/api/users/:id', User.createAPI('details', { excludeFields: 'password', idParam: 'id' }));
app.post('/api/users', User.createAPI('create', { fields: 'name, email', data: { isAdmin: false } }));
app.put('/api/users', User.createAPI('update', { fields: 'name, email' }));
app.delete('/api/users/:id', User.createAPI('delete'));

More relevant is the expectations of data structure in & out of each
endpoint. Backbone, for example, wouldn't work out of the box because it
expects all data to be nested inside a root object, whereas we nest data
inside a fields object.

What I really want thoughts around is the internal structure of data the
endpoint is working with.

e.g.

For the GET endpoint, should it look like:

// GET Post example
{
id: '123',
name: 'Hello World',
fields: {
title: 'Hello World',
slug: 'hello-world',
content: '

hello

',
image: {
/* Image field data */
}
}
}

What about the Update endpoint? How do we deal with JSON vs. FormData
structures?

etc.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/keystonejs/keystone/issues/3209#issuecomment-235799935,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADWlkVCjPaWcRk-muKYq-9_jJ6m3TWBks5qaDRJgaJpZM4JUObx
.

[ttsirkia]

In my opinion, the current API looks very good.

However, when this API becomes more general than just for Admin views, what is the planned or suggested way for access control? For example, after logging in all items in a list could be retrieved but user must not be able to change other users' personal data.

[ttsirkia]

Another question. Is this the suggested way of updating data in the future and should be used instead of the current UpdateHandler because UpdateHandleris mentioned to be removed?

[JedWatson]

@ttsirkia thanks! re: Access control, we'll work that out when we're spec'ing the options for custom endpoints. I expect you'll be able to specify a function that takes the request (which could inspect req.user) and configures the options on the endpoint (so, include or exclude the password field for processing)

e.g.

app.put('/api/users/:id', User.createAPI('update', (req, res) => {
  // only let users update their own password
  fields: req.user.id === req.params.id ? 'name, email, password' : 'name, email'
}));

@wmertens not sure what you mean re: FormData. Because we have file uploading, we need FormData to send that to the server from the Admin UI. How that's handled is more specific to the field types that will process the data (there's an implementation in place already but this is next on my list to spec)

The basic chain of processing goes:

route(req, res) > List.updateItem(item, req.body) > each Field.updateItem(item, data)

... So despite this API being specific to the Admin UI, the underlying stuff will all be common and would be used to process incoming data from many different places including update scripts, custom endpoints (see my example above), csv imports, etc.

@ttsirkia to answer your question about the UpdateHandler, we're going to rewrite it to use the same underlying functionality. So it will be another way of invoking List.updateItem(), with some additions for dealing with the request the way the current implementation does.

Backwards compatibility is really important there, because (afaik) that's a very widely used feature of Keystone.

---

Back to FormData:

One idea is to support a combination json data field with file fields, where data effectively replaces req.body when we're processing the request. That's nonstandard but given we want to submit structured data, I'm not sure a better way to do it. In that case a File field would probably "point" at another field. e.g. uploading an image to a Post:

POST /api/post/:id
data={"title": "great post with an image", "image": "upload:file_xyz123", "state": { "isPublished": true } }
file_xyz123=(data)

The data would be deserialised early on so a simple data structure is passed down through the various layers that process it. We'd have to pass req.files through as well, so that the File field could recognise an image is being uploaded and process it.

If we were just sending the data without serialising it as JSON to a field, like we currently do, this is how it currently works:

POST /api/post/:id
title=great post with an image
image=(data)
state.isPublished=true

That's more standard but there are several challenges:

• nested fields aren't really nested (we already deal with this and I don't see us removing that support)
• it's hard to discern between resubmitting stored data and invoking an action like "upload this file" without hacks like supporting an image_upload field, recognising a string like "upload:{file}" where {file} is the name of a field in the multipart data is clearer imo although also tacky
• boolean values are false if they're not present. This is the part of the HTML spec I hate the most. Boolean fields already suffer for this, we can't tell if they're not being submitted (i.e. not present in the form) or actually being set to false. It makes generic update processing _much_ harder to implement, and they're the only fields type that behaves like this. All the other fields don't update their stored value when value === undefined

If anybody has any good ideas on how we can get this right without inventing too much magic, please let me know!

[christroutner]

I really like the idea of a field for submitting generic JSON data. This
could very well be the solution to the three challenges you listed in the
bullet points. I worked for a software company that did this same thing and
it really saved their butts later on. As the software grew beyond the
horizon they could see when developing the spec, they were able to use this
generic JSON field to expand the API by inserting new, optional key/value
pairs.

Another point I was going to raise was nested arrays. Jed, you're more
familiar with the current state of handling nested arrays in KeystoneJS
than I am. I recently had to dive through this and these are conversations
I was able to find in the mailing list:

• This thread
  https://groups.google.com/forum/#!searchin/keystonejs/model$20array/keystonejs/ybnMtNjTzWU/IC-jwnTgc2QJ
  shows how to connect multiple models using nesting
• This thread
  https://groups.google.com/forum/#!searchin/keystonejs/model$20array/keystonejs/XlyPDRxWzmY/Jr5OBLSNX9EJ
  shows how to implement an array
• This thread
  https://groups.google.com/forum/#!searchin/keystonejs/array$20model/keystonejs/fchL0EcNeQ0/lSnGze0KhEYJ
  has another good example of nested models

Not sure what the solution is, but I thought I'd point it out as it has
been a stumbling point for many users.

I use KeystoneJS for a lot of file and image uploads. I may be doing it
wrong, but I never could figure out how to upload a file/image AND the
metadata (name, alttag, or any other form field) in the same API call. I
always have to make a second call to upload the metadata. Since you
mentioned file/image handling, I thought I'd bring this up in case it may
pose an issue. I'm not clear on the internal mechanics of how the upload
works.

-Chris Troutner

On Thu, Jul 28, 2016 at 8:18 AM, Jed Watson [email protected]
wrote:

@ttsirkia https://github.com/ttsirkia thanks! re: Access control, we'll
work that out when we're spec'ing the options for custom endpoints. I
expect you'll be able to specify a function that takes the request (which
could inspect req.user) and configures the options on the endpoint (so,
include or exclude the password field for processing)

e.g.

app.put('/api/users/:id', User.createAPI('update', (req, res) => {
// only let users update their own password
fields: req.user.id === req.params.id ? 'name, email, password' : 'name, email'
}));

@wmertens https://github.com/wmertens not sure what you mean re:
FormData. Because we have file uploading, we need FormData to send that to
the server from the Admin UI. How that's handled is more specific to the
field types that will process the data (there's an implementation in place
already but this is next on my list to spec)

The basic chain of processing goes:

route(req, res) > List.updateItem(item, req.body) > each Field.updateItem(item, data)

... So despite this API being specific to the Admin UI, the underlying
stuff will all be common and would be used to process incoming data from
many different places including update scripts, custom endpoints (see my
example above), csv imports, etc.

@ttsirkia https://github.com/ttsirkia to answer your question about the
UpdateHandler, we're going to rewrite it to use the same underlying
functionality. So it will be another way of invoking List.updateItem(),
with some additions for dealing with the request the way the current
implementation does.

Backwards compatibility is really important there, because (afaik) that's

a very widely used feature of Keystone.

Back to FormData:

One idea is to support a combination json data field with file fields,
where data effectively replaces req.body when we're processing the
request. That's nonstandard but given we want to submit structured data,
I'm not sure a better way to do it. In that case a File field would
probably "point" at another field. e.g. uploading an image to a Post:

POST /api/post/:id
data={"title": "great post with an image", "image": "upload:file_xyz123", "state": { "isPublished": true } }
file_xyz123=(data)

The data would be deserialised early on so a simple data structure is
passed down through the various layers that process it. We'd have to pass
req.files through as well, so that the File field could recognise an
image is being uploaded and process it.

If we were just sending the data without serialising it as JSON to a
field, like we currently do, this is how it currently works:

POST /api/post/:id
title=great post with an image
image=(data)
state.isPublished=true

That's more standard but there are several challenges:

• nested fields aren't really nested (we already deal with this and I
  don't see us removing that support)
• it's hard to discern between resubmitting stored data and invoking
  an action like "upload this file" without hacks like supporting an
  image_upload field, recognising a string like "upload:{file}" where
  {file} is the name of a field in the multipart data is clearer imo
  although also tacky
• boolean values are false if they're not present. This is the part of
  the HTML spec I hate the most. Boolean fields already suffer for this, we
  can't tell if they're not being submitted (i.e. not present in the form) or
  actually being set to false. It makes generic update processing _much_
  harder to implement, and they're the only fields type that behaves like
  this. All the other skip the update when value === undefined

If anybody has any good ideas on how we can get this right without
inventing too much magic, please let me know!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/keystonejs/keystone/issues/3209#issuecomment-235927454,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AJaej96FFgOQsr53bY4TCdUBaye_DCw7ks5qaMhdgaJpZM4JUObx
.

[wmertens]

Ok @jedwatson I think we're in agreement, the only sane way forward is
posting JSON bodies (worst case, JSON in query strings, perhaps using jsurl
to shorten them cheaply)

In other words, nested JSON data comes in "instantly" and is processed in
its entirety before responding.

For files, they are a special case since they are not instant and not
representable in JSON. Furthermore, if you do a server-bypassing S3 upload,
there is not even a file. So this should be special-cased by Keystone. For
example, to allow a smooth upload experience, the upload of a file can
start as soon as you drop it in the UI, and the form gets the upload key.
You can submit the form but in reality it will wait for all files to finish
uploading before it submits with the upload ids.

Files that are uploaded but not saved into a form get garbage collected
eventually.

That way, it is still just saving forms and the storage adapter worries
about the transfer to the server and the garbage collection. A direct-to-S3
storage adapter would store the S3 key, presumably.

[wmertens]

Draft proposal for the local file uploads with tokens. LFSA is the Local File Storage Adapter

UI

Uploads

HTML5 file area, allowing dropping files or clicking to open a selection box. When you drop or select files, they start uploading with a progress bar and a thumbnail as provided by the browser. There is also an X button to delete the upload during upload or once completed. The formdata stores the opaque LocalFile data.

Upon save, the new uploads get converted into existing files.

Existing Files

A nice view of the file with metadata and a thumbnail. Actions: rename, delete.

Protocol / Implementation

For every file being uploaded, a new XHR request is started (in parallel) to the LFSA upload endpoint, providing the filesystem name and the file. The XHR request provides progress and cancellation. The file is stored in a temporary location during upload. The exact FormData layout is TBD but not very important.

Once a file is uploaded and accepted (virus scanning, file size, type, …), it is moved to its final location (for example using the content hash as the filename), and the File data is returned to the browser.

To retrieve the file, the LFSA provides a download location where authorized requests can download the file. The Content-Disposition header is set to the original name. Authorization can be fleshed out later.

When an uploaded file is not saved with the form, or when the database is changed from outside, there may be unreferenced and/or missing files. The LFSA thus needs a consistency checker process which can warn about these situations in a UI location that the admin can see.

Notes

• For authorization, we could require the listname, item id, and file identifier (at least unique to the item), and then have a per-list function for determining if a file may be downloaded by the request user. I already have this working and it is nice but it requires files to be in the arrayField "attachments".
• File post-processing would be done with a post:upload hook.
  
  
  
  • We could make a virus scanning plugin which would get a filesystem path.
  
  
  • Not all Storage Adapters would have a filesystem path
  
  
  • How about a thumbnail generator?
  
  
• A MongoGrid Storage Adapter might store the file into MongoDB while uploading, or after virus scanning. The protocol would remain the same.
• An S3 Storage Adapter might perform the signaling needed to store the file directly in S3 and create redirect URLs for downloading the file after authorization.
• An Image field would use some Storage type to store the file. It augments the metadata with image size and type, and could allow resizing/cropping on download, and a nice UI for the field, maybe allowing edits of the image file.
• An Images field could simply be an array of Image fields, or it could be a custom type that internally is an array of Images but provides gallery functions.

[wmertens]

I updated the file proposal, getting rid of the uploadIds. I realized they were not adding any value and making things more complex. Now it only uses File objects.

[JedWatson]

@wmertens I like where you've gone with that but I think we're going to need an interim solution so this can get shipped ASAP, which means processing file uploads as part of a single submission.

I've added a spec for the file field that illustrates how I'm planning to implement this; short version is that we leave muster in the update POST /api/{list}/{id} route, then pass the body and the files objects from req as separate arguments into List.updateItem(item, data, options, callback). To keep the arguments from getting too weird, I actually think files should probably be part of the options argument, e.g.

List.updateItem(item, data, { files: req.files }, callback)

Please let me know if you have any feedback on how I've specified update handling here: https://github.com/keystonejs/keystone/blob/master/fields/types/file/Readme.md

We'll move completely away from the current upload detection / custom fields {path}_upload that are implemented in 0.3 and current master when I merge this in.

[wmertens]

• It would be nice if a default storage instance could be defined for Keystone, and that gets used if there is not one defined on a File field.
• Can we at least pretend that we already have separate file handling, by letting the form update hook first store the files and then merge the File data into the posted data? That way we're ready for the real split and it seems to me that that is a fairly minimal code change?

[JedWatson]

It would be nice if a default storage instance could be defined for Keystone, and that gets used if there is not one defined on a File field.

IMO that's better done in user land with modules. It's trivial to abstract your storage instance in another file, and just require it into your model config. I'm trying to make keystone simpler to follow and less "magical"

Can we at least pretend that we already have separate file handling, by letting the form update hook first store the files and then merge the File data into the posted data?

This would complicate the API for field types that process file uploads, and remember we still want those to support "old school" form submissions and uploading (I don't want to make Keystone exclusively for javascript front-end apps)

however this API is actually forwards-compatible; when we implement client-side pre-uploading that API would return the data to store in the field, and then the update action just submits the new value like with any other field.

We can have both 😄

[wmertens]

IMHO letting non-JS clients upload forms is a very limiting constraint, and I don't see why Keystone should have it. The Keystone Admin UI uses JS and the API was, until now, private.

So allowing regular form POSTs actually seems like a new feature, one that can be added by using a separate endpoint at some future point in time. This endpoint would take the carefully-named fields, recompose those into the desired JSON object including coercing types, wait for the file uploads to complete and then pass everything on to the regular API.

[wmertens]

BTW 👍 on using modules for a default storage implementation.

Also, I think the proposed File implementation got leaked-in abstractions from POST uploads. Perhaps the processing of a form upload into a File object could be done with a _ function or by the Local File Storage Adapter?

[JedWatson]

IMHO letting non-JS clients upload forms is a very limiting constraint, and I don't see why Keystone should have it. The Keystone Admin UI uses JS and the API was, until now, private.

To clarify: this isn't to do with the new Admin API, the UpdateHandler uses the same underlying functionality. This is a major feature of Keystone, we can't break it.

We don't need to support form posting to the API we're talking about here but everything in List.updateItem() and Field.updateItem() needs to understand how to handle it.

[wmertens]

I think List and Field are too far down the abstraction ladder to worry about HTTP protocols. I feel strongly that any code that handles API posts in any form should convert the posted data into an internal form that is free from other abstractions.

This leads to cleaner code that is easy to maintain and reuse.

[JedWatson]

They're not worried about the HTTP protocol, they're worried about which way(s) you can pass them data to update their field value in an item.

so, File fields need to understand how to:

• Update their value with structured data
• Process an upload and update their value with the result
• Reset their value
• Delete the stored file and reset their value

... at a minimum. The question is, what is the format of data that they accept to understand this? That's what I've tried to answer in the File/Readme.md spec.

For security reasons they need uploaded files (e.g. data extracted from a multipart form) to be provided separately from raw field values (e.g. images uploaded client-side with metadata submitted in the update)

Whatever we wrap that with in terms of APIs, we still need to figure out what that common API looks like for any field type that wants to handle file uploads.

This endpoint would take the carefully-named fields, recompose those into the desired JSON object including coercing types, wait for the file uploads to complete and then pass everything on to the regular API.

Whatever's doing the upload still needs to be exposed on the field, because it's the field that knows what to do with the file (e.g. File fields pass it to their storage instance for handling)

I feel strongly that any code that handles API posts in any form should convert the posted data into an internal form that is free from other abstractions.

Yep, it's the spec for that internal form that I'm trying to figure out 😄

[wmertens]

This is what I have in mind:

and I think this is what you have in mind:

IMHO, that's more concerns per component and less clean…

[wmertens]

I just realized that Storages are configured per field. I think I would prefer storages to be configured together with Keystone and then referred to by instance name in the field.

This also makes it that Field is the only one knowing about what to do with incoming files and thus it must be passed the files as received by Express.

Of course, sharing storage by exporting and creating file upload endpoints on /api/mystorage would more-or-less allow the first diagram to happen, but it would not be out-of-the-box.

[christroutner]

I thought I'd provide some feedback on the current Admin API implementation. I figured this would be the best place to do that.

Following the steps documented on this blog post, I was able to get the latest build of Keystone to install.

I then created this GitHub repository with a test script and cloned it to the public/ directory so that it would be served up by Keystone as a static page. The index.js file has the test code, which can be accessed by bring up that page in a web browser with a JavaScript console.

I mention the above in case anyone else wants an example of how to interact with the new API.

I've only tested a couple calls, but I noticed that calling a list that doesn't exist throws a ReferenceError on the server. In the script I linked to above, I made this call:

 //This call intentially creates an error
$.get('/keystone/api/test', '', function(data) {debugger});

The 'test' list doesn't exist. The API responds with code 500 as per the spec, but I noticed this ReferenceError pop up on the command line of the server:

Error thrown for request: /keystone/api/test
ReferenceError: Unknown keystone list "test"
    at list (/home/trout/generator/node_modules/keystone/lib/core/list.js:7:21)
    at initList (/home/trout/generator/node_modules/keystone/admin/server/middleware/initList.js:3:22)
    at Layer.handle [as handle_request] (/home/trout/generator/node_modules/keystone/node_modules/express/lib/router/layer.js:95:5)
    at next (/home/trout/generator/node_modules/keystone/node_modules/express/lib/router/route.js:131:13)
    at Route.dispatch (/home/trout/generator/node_modules/keystone/node_modules/express/lib/router/route.js:112:3)
    at Layer.handle [as handle_request] (/home/trout/generator/node_modules/keystone/node_modules/express/lib/router/layer.js:95:5)
    at /home/trout/generator/node_modules/keystone/node_modules/express/lib/router/index.js:277:22
    at param (/home/trout/generator/node_modules/keystone/node_modules/express/lib/router/index.js:349:14)
    at param (/home/trout/generator/node_modules/keystone/node_modules/express/lib/router/index.js:365:14)
    at Function.process_params (/home/trout/generator/node_modules/keystone/node_modules/express/lib/router/index.js:410:3)
    at next (/home/trout/generator/node_modules/keystone/node_modules/express/lib/router/index.js:271:10)
    at exports.keystoneAuth (/home/trout/generator/node_modules/keystone/lib/session.js:244:2)
    at Layer.handle [as handle_request] (/home/trout/generator/node_modules/keystone/node_modules/express/lib/router/layer.js:95:5)
    at trim_prefix (/home/trout/generator/node_modules/keystone/node_modules/express/lib/router/index.js:312:13)
    at /home/trout/generator/node_modules/keystone/node_modules/express/lib/router/index.js:280:7
    at Function.process_params (/home/trout/generator/node_modules/keystone/node_modules/express/lib/router/index.js:330:12)
GET /keystone/api/test 500 7.597 ms

[christroutner]

As a side question, has any of the new file upload capability (#3228) been implemented yet? I figure I might as well extend my test script to go through all the API calls, including file uploads.

[mxstbr]

As a side question, has any of the new file upload capability (#3228) been implemented yet? I figure I might as well extend my test script to go through all the API calls, including file uploads.

We are extremely hard at work on that, in fact Jed is working on this as we speak! It's going to be done super duper soon, but it's not yet enabled – see #3269 for instructions on how to test that. (you'll need to change two lines in a file)

[mxstbr]

The index.js file has the test code, which can be accessed by bring up that page in a web browser with a JavaScript console.

Also, this is awesome! 🎉

[creynders]

@jedwatson Cool! Great job with the APi doc.

However, I _am_ a fan of real REST API's, for these reasons:

• Using verbs in URL's limits and cripples them. For instance I like to provide both a id-based and slug-based identification mechanism, e.g.
  GET /api/users/muthafucka aliases GET /api/users/123456. However, this means that with POST /api/users/create for instance, you'll need to sanitize the slugs on specific keywords
• REST is far more humanly readable IMO, compare POST /api/users/123456/delete?filter[foo]=whatever&q=kjhkjashkdj to DELETE /api/users/123456?filter[foo]=whatever&q=kjhkjashkdj This applies to calls made from non-browser clients, log streams etc. It also makes consuming them a lot easier
• I don't see the benefits of having a fields field for returned resources. It breaks the symmetry, since the returned resource data structures don't match the sent resource data structures.

In my experience REST works really great. You request resources, which are returned in full or partially. And you send resources in full or partially to update them. It's the symmetry and simpleness which is great. It also respects the separation of concerns by extracting the action from the URI.
And it allows for a lot of future expansion w/o getting in the way, which I've encountered with almost all other schemes, where your API grows and you need to start coding around your own API to allow the features you want. It gets really hard to manage and keep clean both code and APIwise.

I've consolidated and modified a number of guides, picking what I've found to work best and compiled it to a guide, maybe it can provide some inspiration: https://iminds-livinglabs.github.io/rest-api-recommendations/index.html

[creynders]

@JedWatson

I haven't added a spec for it but we have a draft implementation of an endpoint you can call to update multiple items. It's currently /api/{list}/update, and an array of items (including ID and new data) are sent in the body.

That's what PATCH is for. Many people think it's only used to partially update a single resource directly, but that's not correct. A collection is also a resource, i.e. if you want to update multiple resources in a collection it's a partial update of the collection, which is why PATCH is allowed to do this. E.g. you could

PATCH /api/users
[
    {
        id: 123456,
        name: "Jules Winfield"
    },
    {
        id: 789123,
        name: "Mia Wallace"
    }
]

Actually, the way PATCH normally is handled in most frameworks is incorrect. PATCH is supposed to describe a set of changes applied to one or more resources.

The PATCH method affects the resource identified by the Request-URI, and it also MAY have side effects on other resources; i.e., new resources may be created, or existing ones modified, by the application of a PATCH.

See RFC-5789

That said, obviously it's an easy way of updating a single resource by simply sending out the updated values and using the ID from the request URI.
Aaaaanyway, what I'm working towards in my projects is to allow PATCH be a compositional action, where:

1. if an ID is present in the request URI, it applies value updates to a single resource
2. if the request URI points to a collection it's a set of changes existing out of a composition of REST actions, e.g.

PATCH /api/users [ { verb: "PATCH", data: { id: 123456, name: "Jules Winfield" } }, { verb: "DELETE", data: { id: 789123 } }, ]

Which solves all problems of updating and deleting multiple resources.

And finally, any operations on entities that aren't resources are POST's to actions, e.g. POST /api/search

Not saying this is what _should_ be implemented in keystone. Just defending the sanity and flexibility of REST API's 😁

[christroutner]

I just wanted to update this thread with a compliment on the implementation of the /api/session call. This is going to make it so much easier to detect if a user is logged in and control the content displayed to them. Also, the fact that api calls for non-logged-in callers return the sign-in page is perfect.

I started a new repository last night, keystone-api-tests, where I'll be writing a test script for the new Admin API and Files field. The code used in this test script will also be used in the API documentation I'll be contributing. It's very simple right now, with hard to read code and only three tests, but I will be expanding and improving it.

FYI, the /api/sessions call returns an empty Object for a user that is not logged in, which is detectable on the front end with jQuery using the command $.isEmptyObject(data). However, according to the spec, it should return undefined. I'm not sure if that is the same thing.

[mxstbr]

Awesome @christroutner, looking forward to some other discrepancies you'll find!

[christroutner]

After reading through the spec and this thread again, I'm not sure what the proper way to UPDATE a list item is. I've tried both of the following and neither one works:

$.post('/keystone/api/users/update', userItem, function(data) {debugger;})
$.post('/keystone/api/users/'+userItem.id+'/update', userItem, function(data) {debugger;})

I tried to dig into the code, but couldn't figure out the correct path. I'm also using a version of the master branch that is a few weeks old.

The first instruction will return a 403 error, so I'm included to think this is the one that should work. The second one returns a 404 error, but is how I've seen it done before.

I've updated the keystone-api-test repository and included test cases for this situation. If you want to try it out, just clone the repo into the public/ directory of a working Keystone install, then open ip:3000/keystone-api-tests/tests.html in your browser.

[mxstbr]

It's POST /keystone/api/users/userId, no /update! (see admin/client/utils/List.js for all the API calls we make from the admin interface)

[christroutner]

Thanks for the quick reply, Max. I fixed the URL in my POST, but I'm still getting a 403 forbidden error. I made sure I'm logged into the system. I even updated my installation of Keystone to the latest commit (cbf9ff46b4529fc9ca52da7c55ca5c4e82a8d74c).

I've updated the keystone-api-test repo to reflect the change is POST URL. I've also tried doing a Update to other list items besides just the User model, and it doesn't work for those either.

I'll try to fire up node-inspector and step through the code in admin/client/utils/List.js to figure out what's going on.

[wmertens]

@christroutner note that Node v6.3+ has the --inspect flag now, which works wonderfully.

[christroutner]

I ran the test script in the keystone-api-tests repository while running KeystoneJS under node inspector. Keep in mind that I'm running the latest master commit (cbf9ff4).

When I ran the post(/keystone/api/users/userID) test again, the file keystone/admin/client/utils/List.js didn't get executed at all. Instead, keystone/admin/server/api/item/update.js was executed. That file was throwing a 403 error because of an invalid CSRF token. Here is the code from that file. I stopped it at the debugger statement:

(function (exports, require, module, __filename, __dirname) { module.exports = function (req, res) {
    var keystone = req.keystone;
    if (!keystone.security.csrf.validate(req)) {
        debugger;
        return res.apiError(403, 'invalid csrf');
    }
    req.list.model.findById(req.params.id, function (err, item) {
        if (err) return res.status(500).json({ err: 'database error', detail: err });
        if (!item) return res.status(404).json({ err: 'not found', id: req.params.id });
        req.list.validateInput(item, req.body, function (err) {
            if (err) return res.status(400).json(err);
            req.list.updateItem(item, req.body, { files: req.files }, function (err) {
                if (err) return res.status(500).json(err);
                res.json(req.list.getData(item));
            });
        });
    });
};

});

I manually ran req.keystone.security.csrf.getSecret(req) and it returned the CSRF token string "CunSnobyfWscEg==". Next I manually ran req.keystone.security.csrf.validate(req), which returned false. I ran req.keystone.security.csrf.requestToken(req) and it returned an empty string.

During this entire time, I'm logged in and can access the Admin UI at /keystone, so this clearly seems to be a CSRF issue.

Any thoughts?

[christroutner]

If anyone can try out a POST call to update a list item successfully, let me know. If there is some environment variable I'm missing I'll dig into it. I'd be curious to see if other people get the same experience I did.

[danielmahon]

@JedWatson @wmertens Just wanted to throw my hat in the ring here. What happened with the GraphQL discussion? I've been working on a React/Keystone app for a bit now and was able to rather easily use https://github.com/RisingStack/graffiti-mongoose to hook up the models to the GraphQL API. This was simple because I didn't need to create a single GraphQL schema, it converts the mongoose one. There are only a few fields which it doesn't seem to like (createdBy, relationships, etc). I'm looking into if that's an easy fix from either end. Thoughts?

[danielmahon]

OK, I lied. Must of had a typo somewhere. Now it seems to work perfectly (so far) with keystone:

const keystone = require('keystone');
const _ = require('lodash');
const graphqlHTTP = require('express-graphql');
const { getSchema } = require('@risingstack/graffiti-mongoose');

// Setup Route Bindings
exports = module.exports = (app) => {
  // Import keystone schemas
  const options = {
    mutation: false, // mutation fields can be disabled
    allowMongoIDMutation: false, // mutation of mongo _id can be enabled
  };
  const models = _.map(keystone.lists, list => list.model);
  const graphQLSchema = getSchema(models, options);

  // GraphQL
  app.all('/api/graphql', graphqlHTTP({ schema: graphQLSchema, graphiql: true }));

};

[wmertens]

nice!

There is a problem with this though, mongoose is not the final arbiter of
permissions here. So with this endpoint you are allowing anyone full access
to the DB, or am I mistaken?

This would be a great plugin, something like
"require('keystone-graphql')(myKeystoneInstance)". It would need to lazily
build the schema on first connect due to the ordering of the settings I
think.

On Fri, Sep 30, 2016 at 5:09 AM Daniel Mahon [email protected]
wrote:

OK, I lied. Must of had a typo somewhere. Now it seems to work perfectly
(so far) with keystone:

const keystone = require('keystone');
const _ = require('lodash');
const graphqlHTTP = require('express-graphql');
const { getSchema } = require('@risingstack/graffiti-mongoose');

// Setup Route Bindings
exports = module.exports = (app) => {
// Import keystone schemas
const options = {
mutation: false, // mutation fields can be disabled
allowMongoIDMutation: false, // mutation of mongo _id can be enabled
};
const models = _.map(keystone.lists, val => val.model);
const graphQLSchema = getSchema(models, options);

// GraphQL
app.all('/api/graphql', graphqlHTTP({ schema: graphQLSchema, graphiql: true }));

};

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/keystonejs/keystone/issues/3209#issuecomment-250648413,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADWlrnajm4p3L_3cYC8A5aHGjmEGnn2ks5qvH1sgaJpZM4JUObx
.

[LorbusChris]

Regarding GraphQL, another approach that I've been thinking of making is to wrap Keystone's API in it (and that neat Dataloader) like described in this post on the graphql blog (see the server-side REST wrapper part).

That way permissions (and possibly roles in the future) would be enforced.

This may even be worth automating with a getGraphQLSchema function specifically for the Keystone API, too.

[zenflow]

Regarding GraphQL, another approach that I've been thinking of making is to wrap Keystone's API in it (and that neat Dataloader) like described in this post on the graphql blog (see the server-side REST wrapper part).

@LorbusChris The intro to that blog post seems to imply that that approach is suggested mostly as a shortcut to upgrade legacy APIs. I can see it has it's drawbacks in portability, complexity and performance (the application making lots and lost of localhost http connections to itself). Hosts like Heroku don't support this. As a framework I think it would be better to follow graphql.org's recommended best practices:
https://graphql.org/learn/thinking-in-graphs/#business-logic-layer

[JedWatson]

This can be closed now; the API in v4 is stable and v5 is using GraphQL

[zenflow]

and v5 is using GraphQL

@JedWatson Exciting words! Is there anywhere we can follow and or contribute to v5 yet?

[gautamsi]

https://github.com/keystonejs/keystone-5