Read json from request BODYFILE instead of BODY.

[mauritsvanrees]

This fixes a regression due to a low Zope form memory limit of 1MB used since Plone 6.0.7. See CMFPlone issue 3848 <https://github.com/plone/Products.CMFPlone/issues/3848>_ and Zope PR 1142 <https://github.com/zopefoundation/Zope/pull/1142>_.

Fixes https://github.com/plone/plone.restapi/issues/1730

[netlify[bot]]

✅ Deploy Preview for plone-restapi canceled.

Name	Link
🔨 Latest commit	07821b8123b3acf0d82f467656e76a63598b0321
🔍 Latest deploy log	https://app.netlify.com/sites/plone-restapi/deploys/6543ea7088bd8300084259bb

[mister-roboto]

@mauritsvanrees thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

[mauritsvanrees]

@davisagli This might be the good direction, but lots of tests fail. Locally (when running the tests from within the coredev 6.0 buildout) I even get an OSError because there are too many open files.

[davisagli]

@mauritsvanrees Okay, let's go with https://github.com/plone/plone.restapi/pull/1729 for now and come back to this.

[mauritsvanrees]

I have pushed a fix which helps: we need to make sure we read the bodyfile from the beginning. I can't tell yet if the open files error goes away with this.

[mauritsvanrees]

4 failures, 1 error. Not bad, but still needs some work. And it could mean just a few more test fixes, or it may mean possible breakage in third party code. So even if we go for this, it may mean a major release. So indeed perhaps first merge and release PR #1729, use that in Plone 6.0.8, and then come back to this one. cc @tisto.

I stop for today.

[mauritsvanrees]

The "too many open files" error is gone at least.

[davisagli]

@mauritsvanrees Not sure if this is the reason for the remaining test failures, but I think what we need to do is not always seek back to 0, but make sure that we restore the file pointer to the position it was before reading. The ValueDescriptor in Zope does this: https://github.com/zopefoundation/Zope/blob/master/src/ZPublisher/HTTPRequest.py#L1377-1379

[mauritsvanrees]

@davisagli I restore the file position now, but that was not it. Problem is with the @lock endpoint, where the BODYFILE is empty. json.load on an empty file gives a ValueError. So now I read the BODYFILE before passing its contents on, and use a dictionary if the contents are empty. That fixes the lock tests at least.

Yes, I said I would stop for today. ;-)

[mauritsvanrees]

@jenkins-plone-org please run jobs

[mauritsvanrees]

Okay, so this needs fixes in plone.volto. Maybe only updates to the tests, I don't know.

Let's go with PR #1729 for now and use that in Plone 6.0.8. It is approved already. If no one merges and releases it, I intend to do so myself Monday, and make 6.0.8 final.

Afterwards we can continue on the current PR.

[tisto]

@mauritsvanrees I merged #1729 and released it with plone.restapi 9.1.2. Feel free to ping me if there is anything else you would like me to do.

[mauritsvanrees]

Thanks!