strapi / rfcs

RFCs for Strapi future changes
68 stars 33 forks source link

REST API #27

Closed alexandrebodin closed 2 years ago

alexandrebodin commented 3 years ago

Rest API v4

This PR introduces a draft version of the REST API in v4.

We want to get the conversation started around this topic to get feedbacks quickly so feel free to ask questions and give ideas :)

You can read it here

sunnysonx commented 3 years ago

Great RFC! That's exactly what I am expecting to see in v4

Stun3R commented 3 years ago

Hi!

First of all the new version of the REST API looks promising !

Here my first impression about it:

Finally I love the example with qs it will help me a lot for the next version of the SDK 👍🏼

PS: You are a beast, I can't wait to see the V4 😍

sunnysonx commented 3 years ago

What about prefixes for APIs, the same logic as Plugins have right now in routes.json (config.prefix) For example, when you have a blog, you will have multiple APIs(articles, categories, tags, and so on) and you may want to prefix all Blog's APIs under a unique route like /blog/articles, /blog/categories, /blog/tags

ScottAgirs commented 3 years ago

Hi!

First of all the new version of the REST API looks promising !

Here my first impression about it:

  • I really like the new meta object which handle all the pagination stuff & locales 😉
  • The query parameters are great, it allow a better flexibility for the queries
  • I don't really like the terminology pluralApiId since it's not an ID I would prefer to call it contentTypePluralized
  • For the sorting, the - prefix can be misleading. I prefer the current way to do it: sort=title:ASC or sort=title:DESC

Finally I love the example with qs it will help me a lot for the next version of the SDK 👍🏼

PS: You are a beast, I can't wait to see the V4 😍

Everything @Stun3R said! :) The as per naming I personally thing contentType would be sufficient, as pluralizing is a fairly common best practice anyway - it could be added as a note somewhere, but I'm not sure it's worth polluting the namespace to explicitly describe that - but I'm prolly nitpicking here, hehe.

This is very exciting! Can't wait to see the RFC for GraphQL. 🚀🔥 [rocket-fire!]

As per the request for feedback

Choosing to return the total number of documents

IMHO and experience, the count data is important 98% of the time when dealing with pagination, therefore I would like to say option[2] - always return by default and provide option to disable for the 2% of the time, haha. Unless the performance overhead is significant, which hopefully it wouldn't be 😅

derrickmehaffy commented 3 years ago

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/new-version-questions-thoughts-v4/4824/1

brettwillis commented 3 years ago

Regarding the total count data for pagination, I personally think it is better to keep the API performant by default, and elect to receive the total count only when explicitly required.

Perhaps an alternative is to allow the database connector to elect the default depending whether counting is expensive or not. However this would result in inconsistency so it's probably better to stick with a performant default.

No choice you make will suit everybody, so sticking with Strapi's configurable nature, it's probably good to allow the default to be configured, both globally and perhaps also per content type.

Stun3R commented 3 years ago

Regarding the total count data for pagination, I personally think it is better to keep the API performant by default, and elect to receive the total count only when explicitly required.

Perhaps an alternative is to allow the database connector to elect the default depending whether counting is expensive or not. However this would result in inconsistency so it's probably better to stick with a performant default.

Sticking with Strapi's configurable nature, it's probably good to allow the default to be configured, both globally and perhaps also per content type.

I totally agree with you, for me it should be disable by default to keep the performance but it could be configurable in order to choose if it's enable or disable by default.

derrickmehaffy commented 3 years ago

@brettwillis / @Stun3R I'm with both of you, keep it disabled by default but allow the user to configure this either globally ./config/api.js or at the model level (much like we do with timestamps)

Stun3R commented 3 years ago

What about prefixes for APIs, the same logic as Plugins have right now in routes.json (config.prefix)

For example, when you have a blog, you will have multiple APIs(articles, categories, tags, and so on) and you may want to prefix all Blog's APIs under a unique route like /blog/articles, /blog/categories, /blog/tags

@sunnysonx I'm so much with you! Because at the moment when you have articles, tags and categories contentType you have to go though /api/articles etc... And if your API contain other type of contentType it's not really clear & your API can become messy 🤔 So the prefix is a really good idea in order to group multiple contentType into one prefix

tspvivek commented 3 years ago

The RFC seems promising in many ways. Will there be a option to access ctx in lifecycle hooks in this new database layer?

navaru commented 3 years ago

Here are a few of my conventions, maybe some would be useful.

I use the terms:

GET /api/resource      →  get a collection of records
GET /api/resource/:id  →  get a record

PUT and PATCH use cases:

HTTP response format:

Collection meta naming convention:

{
  offset: 20,     // the number of records skipped
  limit: 20,      // the number of records returned
  total: 100,     // the total number of records in the collection
  orderBy: {…},   // the order by field 
  groupBy: {…},   // how the request was grouped
  query: {…},     // how the request was filtered (the URL query object)
  fields: […],    // the fields which are included in the record
  lang: 'en',     // the selected language
}

All the above fields from the meta object prefixed with $ can be passed in the body or query of a HTTP request.

I started with a similar approach to { pagination: {...} } and ended up just adding the pagination fields in the meta.

HTTP response JSON error format:

{
  "id": "328f982h3i",
  "type":  "validation",
  "name": "AccountSchema",
  "message": "The account record is missing the field: name",
  "code": "E0132",
  "meta": [{
    "field": "name",
    "message": "The field is required",
    "expected": "string",
    "value": "null"
  }]
}
derrickmehaffy commented 3 years ago

The RFC seems promising in many ways. Will there be a option to access ctx in lifecycle hooks in this new database layer?

I believe (@alexandrebodin correct me if I'm wrong) but don't we plan to add query lifecycles that are executed around the controllers or services much like policies function now?

alexandrebodin commented 3 years ago

The RFC seems promising in many ways. Will there be a option to access ctx in lifecycle hooks in this new database layer?

I believe (@alexandrebodin correct me if I'm wrong) but don't we plan to add query lifecycles that are executed around the controllers or services much like policies function now?

This is something we are considering. It might not be released directly for v4.0.0 but there are good chances this will be added. For the lifecycles this is also another approach we are exploring. These could come and extend the REST and GraphQL API a bit later :)

tspvivek commented 3 years ago

@alexandrebodin Thanks

derrickmehaffy commented 3 years ago

For the fields, could we pass in relation field name we could return an array of IDs instead of an array of objects with the ID key.

Something like fields: ["categories"] which would populate only an array of ids.

Norbz commented 3 years ago

As mentioned above by @sunnysonx namespace would be so great. It can besom a mess right now not to be able to (easily) regroup endpoints and to have all the content type listed as on unique lost in the admin.

One thing I'd like to see also would be lifecycle hooks for components. If you start using components for recurrent data pattern (such as addresses, coordinates, whatever) but have to apply some operation on it, the only thing you can do right now is to alter the lifecycle on each content-type that is able to use the said component which is not a great design at all.

julian-btmx commented 2 years ago

Hi! Thanks for all the hard work. I'm working on a POC using version 4 beta and have a some questions regarding relationship population.

  1. How can one populate all fields at a depth greater than 1? http://localhost:1337/api/pages?populate=* seems to suggest only a single level
  2. How can one populate collectionType relations, not just components? The above API for example adds in component fields, but the response includes no related collectionType fields.

Thanks!

derrickmehaffy commented 2 years ago

Hi! Thanks for all the hard work. I'm working on a POC using version 4 beta and have a some questions regarding relationship population.

  1. How can one populate all fields at a depth greater than 1? http://localhost:1337/api/pages?populate=* seems to suggest only a single level
  2. How can one populate collectionType relations, not just components? The above API for example adds in component fields, but the response includes no related collectionType fields.

Thanks!

We currently have no method to populate everything (simply) past 1 level. Last time I discussed (internally) with @alexandrebodin was the potential to add a "multi-wildcard" like populate=*.* or something.

lexDevV commented 2 years ago

The changes you have made already are awesome!

Although I have a feeling we lost some functionality. I would've assumed that all info about localizations would be found in the 'meta' object. But it's never returning that for me. Is this blocked for the outside API (sanitized)? Or did you remove this on purpose?

I am asking this question because it's something I used pretty often.

The way the api's are structured and named now are very nice changes tho, and I cant wait to update to strapi v4.0

Kind regards

derrickmehaffy commented 2 years ago

The changes you have made already are awesome!

Although I have a feeling we lost some functionality. I would've assumed that all info about localizations would be found in the 'meta' object. But it's never returning that for me. Is this blocked for the outside API (sanitized)? Or did you remove this on purpose?

I am asking this question because it's something I used pretty often.

The way the api's are structured and named now are very nice changes tho, and I cant wait to update to strapi v4.0

Kind regards

The localization stuff should show up? I was just testing it on the latest beta we put out today (beta.12)

julian-btmx commented 2 years ago

* or something.

Thanks for the info @derrickmehaffy, in light of currently not being able to get a 'deeply' populated response from the REST API, I have looked at retrieving this data using the graphql API (currently using "@strapi/plugin-graphql": "4.0.0-beta.12"). Now a question I came across is how to query dynamic zones. Below is a query that works as expected until I uncomment the 'sections' field, which is a dynamic zone in my schema. When including the 'sections' query I get the below error. Query:

query {
    testPages {
        data {
            id
            attributes {
                name
                # sections {
                #     __typename
                # }
                singlecomponent {
                    __typename
                    name
                    reftest {
                        data {
                            attributes {
                                name
                            }
                        }
                    }
                }
                repeatablecomponent {
                    __typename
                    id
                    reftest {
                        data {
                            attributes {
                                name
                            }
                        }
                    }
                }
            }
        }
    }
}

Error when uncommenting 'sections':

{
    "errors": [
        {
            "message": "Cannot destructure property '_q' of 'params' as it is undefined.",
            "locations": [
                {
                    "line": 7,
                    "column": 17
                }
            ],
            "path": [
                "testPages",
                "data",
                0,
                "attributes",
                "sections"
            ],
            "extensions": {
                "code": "INTERNAL_SERVER_ERROR",
                "exception": {
                    "stacktrace": [
                        "TypeError: Cannot destructure property '_q' of 'params' as it is undefined.",
                        "    at transformParamsToQuery (/Users/jmeyer/projects/marketing/backend_v4/node_modules/@strapi/strapi/lib/services/entity-service/params.js:24:11)",
                        "    at Object.load (/Users/jmeyer/projects/marketing/backend_v4/node_modules/@strapi/strapi/lib/services/entity-service/index.js:283:35)",
                        "    at Object.load (/Users/jmeyer/projects/marketing/backend_v4/node_modules/@strapi/strapi/lib/services/entity-service/index.js:67:49)",
                        "    at Object.proto.<computed> [as load] (/Users/jmeyer/projects/marketing/backend_v4/node_modules/delegates/index.js:40:31)",
                        "    at /Users/jmeyer/projects/marketing/backend_v4/node_modules/@strapi/plugin-graphql/server/services/builders/resolvers/dynamic-zone.js:6:35",
                        "    at /Users/jmeyer/projects/marketing/backend_v4/node_modules/@strapi/plugin-graphql/server/services/content-api/policy.js:26:12",
                        "    at /Users/jmeyer/projects/marketing/backend_v4/node_modules/@strapi/plugin-graphql/server/services/content-api/wrap-resolvers.js:69:11",
                        "    at fieldDefinition.resolve (/Users/jmeyer/projects/marketing/backend_v4/node_modules/@strapi/plugin-graphql/server/services/content-api/wrap-resolvers.js:111:40)"
                    ]
                }
            }
        }
    ],
    "data": {
        "testPages": {
            "data": [
                {
                    "id": "1",
                    "attributes": {
                        "name": "home",
                        "sections": null,
                        "singlecomponent": {
                            "__typename": "ComponentTestComponent",
                            "name": "singlec",
                            "reftest": {
                                "data": {
                                    "attributes": {
                                        "name": "Hello"
                                    }
                                }
                            }
                        },
                        "repeatablecomponent": [
                            {
                                "__typename": "ComponentTestComponent",
                                "id": "6",
                                "reftest": {
                                    "data": {
                                        "attributes": {
                                            "name": "Hello"
                                        }
                                    }
                                }
                            },
                            {
                                "__typename": "ComponentTestComponent",
                                "id": "7",
                                "reftest": {
                                    "data": {
                                        "attributes": {
                                            "name": "Hello"
                                        }
                                    }
                                }
                            }
                        ]
                    }
                }
            ]
        }
    }
}

I hope this is still the right place to ask this question, but if someone could point out how to query a dynamic zone with graphql that'd much appreciated! Thanks

derrickmehaffy commented 2 years ago

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/strapi-v4-populate-media-and-dynamiczones-from-components/12670/2