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

Extension not working in Typo3 v9.5 #58

Closed
qbadev opened this issue Apr 22, 2019 · 23 comments
Closed

Extension not working in Typo3 v9.5 #58

qbadev opened this issue Apr 22, 2019 · 23 comments

Comments

@qbadev
Copy link

qbadev commented Apr 22, 2019

Typo3 9.5
Rest 3.5

I followed installation instructions from https://rest.corn.rest/Installation/.

Log says Too few arguments to function Cundd\Rest\BootstrapDispatcher::processRequest(), 1 passed and exactly 2 expected.
Looks like core Typo3 dispatcher does not pass Response object, and passing second argument response is deprecated and will be removed in T3 v10.

@cundd
Copy link
Owner

cundd commented Apr 22, 2019

Thank you for reporting.

Do you have a link to the deprecation in the TYPO3 docs?

@qbadev
Copy link
Author

qbadev commented Apr 22, 2019

Can't find corresponding deprecation note, but the diff is there: https://github.com/TYPO3/TYPO3.CMS/blob/v9.5.0/typo3/sysext/core/Classes/Http/Dispatcher.php#L38

T3 9.4 had completely different approach serving requests (using eID) through Dispatchar, than T3 9.5.

Looks like, in 9.5 You need to init own Response object in \Cundd\Rest\BootstrapDispatcher or in \Cundd\Rest\Dispatcher object

@cundd
Copy link
Owner

cundd commented Apr 23, 2019

I think I found the deprecation note. Unfortunately it only mentions Backend controllers in its title.

https://docs.typo3.org/typo3cms/extensions/core/Changelog/9.5/Deprecation-84196-BackendControllerActionsDoNotReceivePreparedResponse.html

@cundd
Copy link
Owner

cundd commented Apr 23, 2019

The bug should be fixed in version 4 (commit 064a641). Would be great if you could test it.

@qbadev
Copy link
Author

qbadev commented Apr 23, 2019

Sure, I will test, but please help me do this, as I don't know how to install dev branch version.
When are You going to deploy stable v4?

@cundd
Copy link
Owner

cundd commented Apr 23, 2019

The easiest way is to checkout the v4 branch from github. Or require as "cundd/rest": "dev-v4" in your composer.json. How is your TYPO3 installation deployed?

@cundd
Copy link
Owner

cundd commented Apr 23, 2019

I thought about releasing v4, but then your error report came in :)

@qbadev
Copy link
Author

qbadev commented Apr 25, 2019

Thanks, I use a Composer approach. Will give it a try and report results here.

@qbadev qbadev closed this as completed Apr 25, 2019
@qbadev qbadev reopened this Apr 25, 2019
@qbadev
Copy link
Author

qbadev commented Apr 29, 2019

Installed via Composer: v4.x-dev. It works fine! Thanks.
@cundd I started building React-Admin app, had succesfully configured REST ext, got connected from RA to REST, using React-Admin-Simple-Rest-Dataprovider, but it requires to receive a Content-Range header with total number of items in a list. I'd like to add support for this header to Your REST ext, so I also need to know, if extending Your ext without overwriting core files is possible. How can I achieve such solution?

@cundd
Copy link
Owner

cundd commented Apr 30, 2019

That's great!

I just made a change to the Dispatcher (30e4092)

You can specify a userFunc in the TypoScript configuration for additional response headers. Now the response will be passed in the parameters array sent to the custom userFunc. You can try to grab the response from there and count the entries.

The other solution is to create a custom Handler for your requests.

@qbadev
Copy link
Author

qbadev commented Apr 30, 2019

Thank You for all Your effort and quick fixes.

@qbadev qbadev closed this as completed Apr 30, 2019
@qbadev
Copy link
Author

qbadev commented Apr 30, 2019

I am familiar with using userFunc, also I just found a way to get content from Response object
$response->getBody()->rewind(); $newResponse->getBody()->getContents();
but still it is a mystery to me, where to attach userFunc in TypoScript configuration: is it plugin.tx_rest.settings.responseHeaders.userFunc?

@cundd
Copy link
Owner

cundd commented Apr 30, 2019

The following should work.

But I have to give in I never used it...

plugin.tx_rest.settings {
    responseHeaders {
        Content-Range {
            userFunc = xyz
        }
    }
}

@qbadev
Copy link
Author

qbadev commented Apr 30, 2019

It's clear now. Thanks. Will post back to confirm this method.
Getting response content again is not a perfect way counting elements, do You think it is possible and purposefull to set a number of items during listAll proceess? It would be useful to not process the result two times.

@cundd
Copy link
Owner

cundd commented Apr 30, 2019

Yes it's not optimal.

Is the Content-Range header as it is used by React-Admin-Simple-Rest-Dataprovider a "standard" header?

@qbadev
Copy link
Author

qbadev commented Apr 30, 2019

It is only required for GET_LIST requests

Note: The simple REST data provider expects the API to include a Content-Range header in the response to GET_LIST calls. The value must be the total number of resources in the collection. This allows react-admin to know how many pages of resources there are in total, and build the pagination controls.

Docs here

@cundd
Copy link
Owner

cundd commented Apr 30, 2019

I meant more if it is a standard HTTP header and if the format "posts 0-24/319" is common.

It looks like the header depends on a lot of other stuff:

  • Needs to have a way to get the correct noun ("posts" in the example)
  • Requires additional CORS headers
  • Maybe needs an additional request header (Accept-Ranges)

In contrast to the PSR7 ServerRequestInterface the ResponseInterface does not support attributes (https://www.php-fig.org/psr/psr-7/) which would have been handy to attach the total number of results to the response.

So unfortunately I don't see an easy general implementation possibility for the REST extension. Or do you have a good idea?

@qbadev
Copy link
Author

qbadev commented Apr 30, 2019

I used Content-Range while streaming videos to html5 player, but it looks like this header is not narrowed to bytes units. see Mozilla docs here

I will dig through the code of ra-data-simple-rest to find out what are its requirements about those noun names ie. 'posts' and if it needs to receive those names in API response.

Maybe a little tweak is required to ra-data-simple-rest to change its behaviour not checking Headers but instead try to implement CountAll method. By the way, is _count/? a standard in REST?

@cundd
Copy link
Owner

cundd commented Apr 30, 2019

_count/: No I don't think so 😉

@qbadev
Copy link
Author

qbadev commented Apr 30, 2019

It was discussed also on StackOverflow
Read here

@cundd
Copy link
Owner

cundd commented Apr 30, 2019

Thank's for the link.

I decided to go with /_count, because I didn't want to implement pagination and I also wanted to be able to send a request that only counts the results without really loading them from the DB (because of performance).
I chose _count as keyword because it should most likely not collision with an identifier/UID.

@qbadev
Copy link
Author

qbadev commented Apr 30, 2019

Looks like I need to do my own mods to ra-data-simple-rest or use different data provider. Thanks anyway for Your time.

@cundd
Copy link
Owner

cundd commented Apr 30, 2019

No problem!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants