Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: wrap request body with data key #55

Merged
merged 10 commits into from
Feb 6, 2023

Conversation

cssmagic
Copy link
Contributor

Just a rough POC for #51 .

There are still some problems to resolve.

@cssmagic
Copy link
Contributor Author

cssmagic commented Dec 28, 2022

I just added a draft PR, remaining two problems to resolve/discuss:

Avoiding polluting non-Content API requests

In a middleware, ctx is incomplete before Strapi handles the request, so it's not easy to recognize Content APIs.

I don't have a perfect solution for this. But two ugly workarounds: 😔

  • Prepare a list of Strapi's built-in APIs, such as auth APIs and ones provided by other plugins. (Sounds bad)
  • Ask users to announce path names of their Content APIs, via a new config key. (Sounds better)

Possibility of "Smart" mode

In the beginning, I believed we could wrap with data key smartly -- if it has no data, we wrap; if it has, we ignore.

// in middleware
let reqBody = ctx.request.body;
if (!('data' in reqBody)) {  // 👈 "smart" detection
	reqBody = { data: reqBody };
	ctx.request.body = reqBody;
}

Unfortunately, this "smart" detection could be wrong, if an entry object has its own data field:

// body of a POST or PUT request
-{
	"foo": "...",
	"bar": "...",
	"data": { /* ... */ },
}

The above needs to be wrapped, but the "smart" detection failed to figure out.

Then I realized there's no way "smart" enough.

Might as well set this feature as "Force" mode -- if the config is on, we wrap; if not, we do nothing.

@ComfortablyCoding
Copy link
Contributor

ComfortablyCoding commented Jan 3, 2023

Great start! I have refactored some of the code (using your work as a base) to be more inline with the rest of the codebase.

Avoiding polluting non-Content API requests

I am not a fan of either of those options, I will need to think more on this. I was originally thinking we can check if the API prefix is present but that will lead to the same issue that the responseTransforms had in #25

Possibility of "Smart" mode

Good catch. I am okay with doing this in "force" mode. If anyone reports an issue we can revisit this functionality.

@cssmagic
Copy link
Contributor Author

cssmagic commented Jan 3, 2023

Thanks 😉

@ComfortablyCoding ComfortablyCoding changed the title feat: wrap request body with data key (rough POC for #51) feat: wrap request body with data key Feb 6, 2023
@ComfortablyCoding ComfortablyCoding added the enhancement New feature or request label Feb 6, 2023
@ComfortablyCoding ComfortablyCoding marked this pull request as ready for review February 6, 2023 05:54
@ComfortablyCoding ComfortablyCoding merged commit 80a7a8d into strapi-community:master Feb 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: wrapping POST or PUT requests' body within data key if not
2 participants