ui
ui copied to clipboard
Feature - Add Psr7
- [x] add Psr7 Response -
nyholm/psr7
- [x] add Response Emitter -
laminas/laminas-httphandlerrunner
- [ ] ~Rework tests to use directly
App->getResponse() : ResponseInterface
~ - [x] Emit response SSE
- [x] Add Test response vs output
- [x] Add Psr7 Request
- [ ] Update
Atk4\Ui\Persistence\Post
request must be handled as PSR request as well
laminas dep should be not needed if this translates to a few echo/header/http_response_code lines
I replace with code, but because of the small difference in performance, I always prefer the ready-made solutions, which are already tested in almost every situation, even those we don't know at the moment.
https://github.com/atk4/ui/blob/beb149f3e15ffdb681df9964cd5f48ed85971851/src/App.php#L1119 Are you sure that this cannot be changed to an Exception or other ways to not force exit the process? My fear is that if you use Atk4 in a routing/handler contest, that exit in place of an Error will stop the flow without an output, without running post route middleware or logging of the error, without considering that to avoid that line you must override via copy/paste the whole method just to remove it.
request must be handled as PSR request as well
laminas dep should be not needed if this translates to a few echo/header/http_response_code lines
I see the PR https://github.com/atk4/ui/pull/1671 which remove/assert that there is no global sticky.
i think in App __construct, if not injected via $defaults['request'], we create a Request using nyholm/psr7-server
and all sticky + $_GET/$_POST/$_FILES that are already present somewhere in the app must point to App->request, what you think?
But before this i need to understand why demos, except for SSE, works perfect, but not in tests.
request must be handled as PSR request as well laminas dep should be not needed if this translates to a few echo/header/http_response_code lines
I replace with code, but because of the small difference in performance, I always prefer the ready-made solutions, which are already tested in almost every situation, even those we don't know at the moment.
replacing literaly no more than several lines and doubling the non-dev dependencies, is IMHO really not helpful
https://github.com/atk4/ui/blob/beb149f3e15ffdb681df9964cd5f48ed85971851/src/App.php#L1119
Are you sure that this cannot be changed to an Exception or other ways to not force exit the process? My fear is that if you use Atk4 in a routing/handler contest, that exit in place of an Error will stop the flow without an output, without running post route middleware or logging of the error, without considering that to avoid that line you must override via copy/paste the whole method just to remove it.
can be put into a protected method that can be overriden with a custom exit handler, but as described, it is too late to have a full control of the output as partly sent already
some Tests are failing at the moment due to github problems, but I think all tests are successful.
the next step is to replace all direct calls to $_GET and $_POST. for ex : https://github.com/atk4/ui/blob/09ec6e11c3c709cb2e5ac78b71ceee65d836b3d0/src/Accordion.php#L151
can be replaced with : $this->getApp()->issetRequestQueryParam('__atk-dyn-section')
and this is ok, the problem is where is no reference to App, for ex. :
https://github.com/atk4/ui/blob/09ec6e11c3c709cb2e5ac78b71ceee65d836b3d0/src/Persistence/Post.php#L19
which cannot be changed with : $this->getApp()->getRequestFormParam($field)
because Persistence has no reference to App
IMHO every external param must be recovered from the request object, getting request vars directly from global $_GET or $_POST is wrong or at least makes usage of sanitizer or intrusion check middlewares useless.
@mvorisek what is your opinion on this?
IMHO every external param must be recovered from the request object, getting request vars directly from global $_GET or $_POST is wrong or at least makes usage of sanitizer or intrusion check middlewares useless.
@mvorisek what is your opinion on this?
Definitely, maybe even unset such env. vars to make sure, all accesses are done thru the App/PSR request interface
Thank you @abbadon1334 ❤️