Repository navigation
Conversation
(cherry picked from commit 60f8537)
Contributor
|
@thojou please have a Look for T&A! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The PSR-7 implementation
guzzlehttp/psr7is meant to be an implementation detail of the HTTP component, but several components instantiated or typed against its classes directly (GuzzleHttp\Psr7\Response,ServerRequest,Uri,UploadedFile, ...). This PR removes all of these usages, so the library can be updated or replaced in HTTP alone, and adds a code rule that keeps it that way.HTTP:
MessageFactoryILIAS\HTTP\Message\MessageFactorywithrequest(),response()anduri(), implemented byMessageFactoryImplon top of Guzzle.$use[MessageFactory::class]), available to legacy code as$DIC->http()->messages()(new method onGlobalHttpState).Components
ilCtrlgets theMessageFactoryinjected instead of creating a GuzzleResponse.Psr\Http\Messageinterfaces.TestQuestionPool\RequestDataCollectorno longer reads the private property$fileof Guzzle'sUploadedFileviaClosure::bind(); it usesgetStream()->getMetadata('uri'), the identifierFileUploadImpluses for its results.SettingsScoringGUIrelayed the whole serializedServerRequestthrough the hidden field of the recalculation confirmation and unserialized it with the Guzzle classes inallowed_classes. It now relays only the parsed body as JSON and restores it withwithParsedBody(), so no objects are unserialized from client input any more.XapiProxy,XapiProxyRequest,XapiProxyResponse) creates requests and responses with theMessageFactoryand types against the PSR interfaces. The commented-outcreateProxyRequest()is removed.MessageFactoryImpl, the interfaces or stubs instead of Guzzle classes.xAPI proxy in trunk
The xAPI proxy in trunk still had
XapiProxyResponse::checkResponse()accessing$response['state']on aResponseobject, which ends inError: Cannot use object of type GuzzleHttp\Psr7\Response as array. The fix 60f8537 ("fix xAPI sending statements") exists onrelease_11but never reached trunk, so it is cherry-picked (-x) as the first commit, unchanged. @Uwe-Kohnle could you confirm this is the intended state for trunk?Code rule
New PHPStan code rule
ilias.wrappedLibraryUsage(scripts/PHPStan/Rules/WrappedLibraries/WrappedLibraryUsageRule.php), part of thecode-rulesgate: a wrapped library may only be referenced inside the component that wraps it. The mapping is a constant in the rule, currentlyGuzzleHttp\Psr7→HTTP; other wrapped libraries can be added there. Exemptions work like for the superglobal rule, with#[AllowWrappedLibraryUsage('reason', 13)]or an inline@phpstan-ignore ilias.wrappedLibraryUsage.v12, and expire with the next major.list_exemptions.shknows the new attribute.Testing
[OK] All static artifacts are build!components/ILIAS: green; a plantednew \GuzzleHttp\Psr7\Uri()makes it fail.release_11.ilCtrl), scoring settings with recalculation confirmation (confirm and cancel), images in matching questions, tile image in the common object settings, xAPI/cmi5 statements through the proxy.@dsstrassner @kergomard for Test/TestQuestionPool, @thibsy for UICore, @Uwe-Kohnle for CmiXapi.
If there are no objections, I will merge this before Coding Completed.