Merge pull request 1EdTech#61 from packbackbooks/request-types

PODB-563 Move type and method constants to ServiceRequest
JanHolger · Jul 14, 2022 · c441757 · c441757
2 parents 86b43e1 + 1c166ea
commit c441757
Show file tree

Hide file tree

Showing 10 changed files with 120 additions and 78 deletions.
diff --git a/src/Interfaces/ILtiServiceConnector.php b/src/Interfaces/ILtiServiceConnector.php
@@ -16,16 +16,14 @@ public function makeServiceRequest(
         ILtiRegistration $registration,
         array $scopes,
         IServiceRequest $request,
-        ?int $requestType = null,
         bool $shouldRetry = true
     ): array;
 
     public function getAll(
         ILtiRegistration $registration,
         array $scopes,
         IServiceRequest $request,
-        string $key,
-        ?int $requestType = null
+        string $key
     ): array;
 
     public function setDebuggingMode(bool $enable): void;

diff --git a/src/Interfaces/IServiceRequest.php b/src/Interfaces/IServiceRequest.php
@@ -19,4 +19,6 @@ public function setBody(string $body): self;
     public function setAccept(string $accept): self;
 
     public function setContentType(string $contentType): self;
+
+    public function getErrorPrefix(): string;
 }
diff --git a/src/LtiAbstractService.php b/src/LtiAbstractService.php
@@ -36,24 +36,22 @@ public function setServiceData(array $serviceData): self
 
     abstract public function getScope(): array;
 
-    protected function makeServiceRequest(IServiceRequest $request, ?int $requestType = null): array
+    protected function makeServiceRequest(IServiceRequest $request): array
     {
         return $this->serviceConnector->makeServiceRequest(
             $this->registration,
             $this->getScope(),
             $request,
-            $requestType
         );
     }
 
-    protected function getAll(IServiceRequest $request, string $key = null, ?int $requestType = null): array
+    protected function getAll(IServiceRequest $request, string $key = null): array
     {
         return $this->serviceConnector->getAll(
             $this->registration,
             $this->getScope(),
             $request,
-            $key,
-            $requestType
+            $key
         );
     }
 }
diff --git a/src/LtiAssignmentsGradesService.php b/src/LtiAssignmentsGradesService.php
@@ -42,11 +42,15 @@ public function putGrade(LtiGrade $grade, LtiLineitem $lineitem = null)
         $pos = strpos($scoreUrl, '?');
         $scoreUrl = $pos === false ? $scoreUrl.'/scores' : substr_replace($scoreUrl, '/scores', $pos, 0);
 
-        $request = new ServiceRequest(LtiServiceConnector::METHOD_POST, $scoreUrl);
+        $request = new ServiceRequest(
+            ServiceRequest::METHOD_POST,
+            $scoreUrl,
+            ServiceRequest::TYPE_SYNC_GRADE
+        );
         $request->setBody($grade);
         $request->setContentType(static::CONTENTTYPE_SCORE);
 
-        return $this->makeServiceRequest($request, LtiServiceConnector::SYNC_GRADE_REQUEST);
+        return $this->makeServiceRequest($request);
     }
 
     public function findLineItem(LtiLineitem $newLineItem): ?LtiLineitem
@@ -64,24 +68,32 @@ public function findLineItem(LtiLineitem $newLineItem): ?LtiLineitem
 
     public function updateLineitem(LtiLineItem $lineitemToUpdate): LtiLineitem
     {
-        $request = new ServiceRequest(LtiServiceConnector::METHOD_PUT, $this->getServiceData()['lineitems']);
+        $request = new ServiceRequest(
+            ServiceRequest::METHOD_PUT,
+            $this->getServiceData()['lineitems'],
+            ServiceRequest::TYPE_UPDATE_LINEITEM
+        );
 
         $request->setBody($lineitemToUpdate)
             ->setContentType(static::CONTENTTYPE_LINEITEM)
             ->setAccept(static::CONTENTTYPE_LINEITEM);
 
-        $updatedLineitem = $this->makeServiceRequest($request, LtiServiceConnector::UPDATE_LINEITEM_REQUEST);
+        $updatedLineitem = $this->makeServiceRequest($request);
 
         return new LtiLineitem($updatedLineitem['body']);
     }
 
     public function createLineitem(LtiLineitem $newLineItem): LtiLineitem
     {
-        $request = new ServiceRequest(LtiServiceConnector::METHOD_POST, $this->getServiceData()['lineitems']);
+        $request = new ServiceRequest(
+            ServiceRequest::METHOD_POST,
+            $this->getServiceData()['lineitems'],
+            ServiceRequest::TYPE_CREATE_LINEITEM
+        );
         $request->setBody($newLineItem)
             ->setContentType(static::CONTENTTYPE_LINEITEM)
             ->setAccept(static::CONTENTTYPE_LINEITEM);
-        $createdLineItem = $this->makeServiceRequest($request, LtiServiceConnector::CREATE_LINEITEM_REQUEST);
+        $createdLineItem = $this->makeServiceRequest($request);
 
         return new LtiLineitem($createdLineItem['body']);
     }
@@ -100,7 +112,11 @@ public function getGrades(LtiLineitem $lineitem = null)
         $pos = strpos($resultsUrl, '?');
         $resultsUrl = $pos === false ? $resultsUrl.'/results' : substr_replace($resultsUrl, '/results', $pos, 0);
 
-        $request = new ServiceRequest(LtiServiceConnector::METHOD_GET, $resultsUrl);
+        $request = new ServiceRequest(
+            ServiceRequest::METHOD_GET,
+            $resultsUrl,
+            ServiceRequest::TYPE_GET_GRADES
+        );
         $request->setAccept(static::CONTENTTYPE_RESULTCONTAINER);
         $scores = $this->makeServiceRequest($request);
 
@@ -114,12 +130,13 @@ public function getLineItems(): array
         }
 
         $request = new ServiceRequest(
-            LtiServiceConnector::METHOD_GET,
-            $this->getServiceData()['lineitems']
+            ServiceRequest::METHOD_GET,
+            $this->getServiceData()['lineitems'],
+            ServiceRequest::TYPE_GET_LINEITEMS
         );
         $request->setAccept(static::CONTENTTYPE_LINEITEMCONTAINER);
 
-        $lineitems = $this->getAll($request, null, LtiServiceConnector::GET_LINEITEMS_REQUEST);
+        $lineitems = $this->getAll($request);
 
         // If there is only one item, then wrap it in an array so the foreach works
         if (isset($lineitems['body']['id'])) {
@@ -135,7 +152,11 @@ public function getLineItem(string $url): LtiLineitem
             throw new LtiException('Missing required scope', 1);
         }
 
-        $request = new ServiceRequest(LtiServiceConnector::METHOD_GET, $url);
+        $request = new ServiceRequest(
+            ServiceRequest::METHOD_GET,
+            $url,
+            ServiceRequest::TYPE_GET_LINEITEM
+        );
         $request->setAccept(static::CONTENTTYPE_LINEITEM);
 
         $response = $this->makeServiceRequest($request)['body'];

diff --git a/src/LtiCourseGroupsService.php b/src/LtiCourseGroupsService.php
@@ -14,8 +14,9 @@ public function getScope(): array
     public function getGroups(): array
     {
         $request = new ServiceRequest(
-            LtiServiceConnector::METHOD_GET,
-            $this->getServiceData()['context_groups_url']
+            ServiceRequest::METHOD_GET,
+            $this->getServiceData()['context_groups_url'],
+            ServiceRequest::TYPE_GET_GROUPS
         );
         $request->setAccept(static::CONTENTTYPE_CONTEXTGROUPCONTAINER);
 
@@ -30,8 +31,9 @@ public function getSets(): array
         }
 
         $request = new ServiceRequest(
-            LtiServiceConnector::METHOD_GET,
-            $this->getServiceData()['context_group_sets_url']
+            ServiceRequest::METHOD_GET,
+            $this->getServiceData()['context_group_sets_url'],
+            ServiceRequest::TYPE_GET_SETS
         );
         $request->setAccept(static::CONTENTTYPE_CONTEXTGROUPCONTAINER);
 

diff --git a/src/LtiMessageLaunch.php b/src/LtiMessageLaunch.php
@@ -278,8 +278,11 @@ public function getLaunchId()
 
     private function getPublicKey()
     {
-        $keySetUrl = $this->registration->getKeySetUrl();
-        $request = new ServiceRequest(LtiServiceConnector::METHOD_GET, $keySetUrl);
+        $request = new ServiceRequest(
+            ServiceRequest::METHOD_GET,
+            $this->registration->getKeySetUrl(),
+            ServiceRequest::TYPE_GET_KEYSET
+        );
 
         // Download key set
         try {

diff --git a/src/LtiNamesRolesProvisioningService.php b/src/LtiNamesRolesProvisioningService.php
@@ -14,8 +14,9 @@ public function getScope(): array
     public function getMembers(): array
     {
         $request = new ServiceRequest(
-            LtiServiceConnector::METHOD_GET,
-            $this->getServiceData()['context_memberships_url']
+            ServiceRequest::METHOD_GET,
+            $this->getServiceData()['context_memberships_url'],
+            ServiceRequest::TYPE_GET_MEMBERSHIPS
         );
         $request->setAccept(static::CONTENTTYPE_MEMBERSHIPCONTAINER);
 

diff --git a/src/LtiServiceConnector.php b/src/LtiServiceConnector.php
@@ -15,38 +15,16 @@ class LtiServiceConnector implements ILtiServiceConnector
 {
     public const NEXT_PAGE_REGEX = '/<([^>]*)>; ?rel="next"/i';
 
-    public const METHOD_GET = 'GET';
-    public const METHOD_POST = 'POST';
-    public const METHOD_PUT = 'PUT';
-
-    // Supported request types which map to an error log message
-    public const UNSUPPORTED_REQUEST = 0;
-    public const SYNC_GRADE_REQUEST = 1;
-    public const CREATE_LINEITEM_REQUEST = 2;
-    public const GET_LINEITEMS_REQUEST = 3;
-    public const UPDATE_LINEITEM_REQUEST = 4;
-    public const AUTH_REQUEST = 5;
-
     private $cache;
     private $client;
     private $debuggingMode = false;
-    private $errorMessages;
 
     public function __construct(
         ICache $cache,
         Client $client
     ) {
         $this->cache = $cache;
         $this->client = $client;
-
-        $this->errorMessages = [
-            static::UNSUPPORTED_REQUEST => 'Logging request data: ',
-            static::SYNC_GRADE_REQUEST => 'Syncing grade for this lti_user_id: ',
-            static::CREATE_LINEITEM_REQUEST => 'Creating lineitem: ',
-            static::GET_LINEITEMS_REQUEST => 'Getting lineitems: ',
-            static::UPDATE_LINEITEM_REQUEST => 'Updating lineitem: ',
-            static::AUTH_REQUEST => 'Authenticating: ',
-        ];
     }
 
     public function setDebuggingMode(bool $enable): void
@@ -86,10 +64,12 @@ public function getAccessToken(ILtiRegistration $registration, array $scopes)
             'scope' => implode(' ', $scopes),
         ];
 
-        $url = $registration->getAuthTokenUrl();
-
         // Get Access
-        $request = new ServiceRequest(static::METHOD_POST, $url);
+        $request = new ServiceRequest(
+            ServiceRequest::METHOD_POST,
+            $registration->getAuthTokenUrl(),
+            ServiceRequest::TYPE_AUTH
+        );
         $request->setPayload(['form_params' => $authRequest]);
         $response = $this->makeRequest($request);
 
@@ -111,7 +91,6 @@ public function makeRequest(IServiceRequest $request)
 
         if ($this->debuggingMode) {
             $this->logRequest(
-                static::AUTH_REQUEST,
                 $request,
                 $this->getResponseHeaders($response),
                 $this->getResponseBody($response)
@@ -142,15 +121,8 @@ public function makeServiceRequest(
         ILtiRegistration $registration,
         array $scopes,
         IServiceRequest $request,
-        ?int $requestType = null,
         bool $shouldRetry = true
     ): array {
-        // Set $requestType here, since static properties cannot be evaluated
-        // as parameters
-        if (!isset($requestType)) {
-            $requestType = self::UNSUPPORTED_REQUEST;
-        }
-
         $request->setAccessToken($this->getAccessToken($registration, $scopes));
 
         try {
@@ -164,7 +136,7 @@ public function makeServiceRequest(
                 $key = $this->getAccessTokenCacheKey($registration, $scopes);
                 $this->cache->clearAccessToken($key);
 
-                return $this->makeServiceRequest($registration, $scopes, $request, $requestType, false);
+                return $this->makeServiceRequest($registration, $scopes, $request, false);
             }
 
             throw $e;
@@ -182,17 +154,16 @@ public function getAll(
         array $scopes,
         IServiceRequest $request,
         string $key = null,
-        ?int $requestType = null
     ): array {
-        if ($request->getMethod() !== static::METHOD_GET) {
+        if ($request->getMethod() !== ServiceRequest::METHOD_GET) {
             throw new \Exception('An invalid method was specified by an LTI service requesting all items.');
         }
 
         $results = [];
         $nextUrl = $request->getUrl();
 
         while ($nextUrl) {
-            $response = $this->makeServiceRequest($registration, $scopes, $request, $requestType);
+            $response = $this->makeServiceRequest($registration, $scopes, $request);
 
             $page_results = $key === null ? ($response['body'] ?? []) : ($response['body'][$key] ?? []);
             $results = array_merge($results, $page_results);
@@ -207,7 +178,6 @@ public function getAll(
     }
 
     private function logRequest(
-        int $requestType,
         IServiceRequest $request,
         array $responseHeaders,
         ?array $responseBody
@@ -219,17 +189,17 @@ private function logRequest(
             'response_body' => json_encode($responseBody),
         ];
 
-        $requestBody = $request->getPayload()['body'] ?? '';
+        $requestBody = $request->getPayload()['body'] ?? null;
 
         if (!empty($requestBody)) {
             $contextArray['request_body'] = $requestBody;
         }
 
-        $userId = json_decode($requestBody)->userId ?? '';
-
-        $logMsg = $this->errorMessages[$requestType];
-
-        error_log($logMsg.$userId.' '.print_r($contextArray, true));
+        error_log(implode(' ', array_filter([
+            $request->getErrorPrefix(),
+            json_decode($requestBody)->userId ?? null,
+            print_r($contextArray, true),
+        ])));
     }
 
     private function getAccessTokenCacheKey(ILtiRegistration $registration, array $scopes)
@@ -243,7 +213,7 @@ private function getAccessTokenCacheKey(ILtiRegistration $registration, array $s
     private function getNextUrl(array $headers)
     {
         $subject = $headers['Link'] ?? '';
-        preg_match(LtiServiceConnector::NEXT_PAGE_REGEX, $subject, $matches);
+        preg_match(static::NEXT_PAGE_REGEX, $subject, $matches);
 
         return $matches[1] ?? null;
     }