From 911100b9b9bd4934d60efb14e3ba3481b3266dd0 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Tue, 23 Jul 2024 13:24:54 +0300 Subject: [PATCH] Refactor/Simplify Primary Link Calculation --- app/src/Controller/AppController.php | 179 +++++++++++++-------------- 1 file changed, 87 insertions(+), 92 deletions(-) diff --git a/app/src/Controller/AppController.php b/app/src/Controller/AppController.php index ade7ce791..51ff92c50 100644 --- a/app/src/Controller/AppController.php +++ b/app/src/Controller/AppController.php @@ -224,8 +224,7 @@ protected function primaryLinkOnGet(string $potentialPrimaryLink): Object|bool if($allowsUnkeyed) { $query = $this->request->getQuery(); if($query) { - $this->cur_pl->value = $this->request->getQuery($potentialPrimaryLink); - return false; + return $this->populatedPrimaryLink($potentialPrimaryLink); } } @@ -252,52 +251,26 @@ protected function primaryLinkOnPost(string $potentialPrimaryLink): Object|bool // Post = add, where we can have a list of objects and nothing in /objects/{id} // We don't support different primary links across objects, so we throw an error // if different parent keys are provided. - - $linkValue = null; - - // Data in API format - $reqData = $this->request->getData($modelsName); - - if(!$reqData - // Don't create $reqData if the POST data is also empty - && !empty($this->request->getData())) { - // Data in POST format - $reqData[] = $this->request->getData(); + // Data in API format | Data in POST format + $reqData = $this->request->getData($modelsName) ?? $this->request->getData() ?? []; + $potentialPrimaryLinkRecords = collection($reqData)->filter(fn($value, $key) => $key === $potentialPrimaryLink); + if($potentialPrimaryLinkRecords->count() > 1) { + // We don't support multiple records with different parents + throw new \InvalidArgumentException(__d('error', 'primary_link.mismatch')); } - - if(!empty($reqData)) { - foreach($reqData as $rec) { - if(!empty($rec[$potentialPrimaryLink])) { - if(!$linkValue) { - // This is the first record we've seen, use this primary link value - $linkValue = $rec[$potentialPrimaryLink]; - } elseif($linkValue != $rec[$potentialPrimaryLink]) { - // We don't support multiple records with different parents - throw new \InvalidArgumentException(__d('error', 'primary_link.mismatch')); - } - } - - $this->cur_pl->value = $linkValue; - } + if($potentialPrimaryLinkRecords->count() === 1) { + return $this->populatedPrimaryLink($potentialPrimaryLink); } // If we didn't find the primary link in the submitted form or API // request, it might be available via the URL. - if(!$linkValue - && $this->$modelsName->allowLookupPrimaryLink($this->request->getParam('action'))) { - // Try to map the requested object ID (this is probably a delete, so no attribute in post body) - $param = (int)$this->request->getParam('pass.0'); - - if(!empty($param)) { - return $this->$modelsName->findPrimaryLink($param); - } - } - - return false; + return $this->primaryLinkOnPut(); } /** + * Primary link available via the URL. + * * @return Object|bool * @since COmanage Registry v5.0.0 * @@ -322,7 +295,45 @@ protected function primaryLinkOnPut(): Object|bool } /** - * @return void + * @param string $potentialPrimaryLink + * + * @return Object|\stdClass + */ + protected function populatedPrimaryLink(string $potentialPrimaryLink): Object + { + // $this->name = Models + $modelsName = $this->name; + // $potentialPrimaryLink will be something like 'attribute_collector_id' + // $potentialPrimaryLinkTable will be something like 'CoreEnroller.AttributeCollectors' + $potentialPrimaryLinkTable = $this->$modelsName->getPrimaryLinkTableName($potentialPrimaryLink); + + // For looking up values in records here, we want only the attribute + // itself and not the plugin name (used for hacky notation by + // PrimaryLinkTrait::setPrimaryLink(). Note this is a field and not + // a model, but pluginModel() gets us the bit we need. + + // Store the plugin for possible later reference. + $potentialPlugin = str_contains($potentialPrimaryLinkTable, '.') + ? StringUtilities::pluginPlugin($potentialPrimaryLinkTable) + : null; + + $cur = new \stdClass(); + $cur->value = $this->request->getQuery($potentialPrimaryLink); + // We found a populated primary link. Store the attribute and break the loop. + $cur->attr = $potentialPrimaryLink; + if($potentialPlugin) { + $cur->plugin = $potentialPlugin; + } + + return $cur; + } + + + /** + * Perform primary link lookup. + * + * @throws \RuntimeException Exception thrown if a primary link is empty and it is not allowed + * @since Symfony v7.0.3 */ protected function primaryLinkLookup(): void { @@ -330,11 +341,8 @@ protected function primaryLinkLookup(): void $modelsName = $this->name; $availablePrimaryLinks = $this->$modelsName->getPrimaryLinks(); + // Iterate over all the potential primary links and pick the appropriate one foreach($availablePrimaryLinks as $potentialPrimaryLink) { - // $potentialPrimaryLink will be something like 'attribute_collector_id' - // $potentialPrimaryLinkTable will be something like 'CoreEnroller.AttributeCollectors' - $potentialPrimaryLinkTable = $this->$modelsName->getPrimaryLinkTableName($potentialPrimaryLink); - $cur = match(true) { $this->request->is('get') => $this->primaryLinkOnGet($potentialPrimaryLink), ($this->request->is('post') && $this->request->getParam('action') != 'delete') => $this->primaryLinkOnPost($potentialPrimaryLink), @@ -342,40 +350,23 @@ protected function primaryLinkLookup(): void default => false, }; - if($cur !== false) { + if($cur !== false && $cur->value !== null) { $this->cur_pl = $cur; $this->set('vv_primary_link', $this->cur_pl->attr); + // Exit the for loop break; } + } // foreach - if(!empty($this->cur_pl->value)) { - // For looking up values in records here, we want only the attribute - // itself and not the plugin name (used for hacky notation by - // PrimaryLinkTrait::setPrimaryLink(). Note this is a field and not - // a model, but pluginModel() gets us the bit we need. - - // Store the plugin for possible later reference. - $potentialPlugin = str_contains($potentialPrimaryLinkTable, '.') - ? StringUtilities::pluginPlugin($potentialPrimaryLinkTable) - : null; - - // We found a populated primary link. Store the attribute and break the loop. - $this->cur_pl->attr = $potentialPrimaryLink; - if($potentialPlugin) { - $this->cur_pl->plugin = $potentialPlugin; - } - $this->set('vv_primary_link', $this->cur_pl->attr); - break; - } - } - - if(empty($this->cur_pl->value) && !$this->$modelsName->allowEmptyPrimaryLink($this->request->getParam('action'))) { + // At the end we need to have a Primary Link + if(empty($this->cur_pl->value) + && !$this->$modelsName->allowEmptyPrimaryLink($this->request->getParam('action'))) { throw new \RuntimeException(__d('error', 'primary_link')); } } /** - * Obtain information about the Standard Object's Primary Link, if set. + * Collect information about the Standard Object's Primary Link, if set. * The $vv_primary_link view variable is also set. * * @since COmanage Registry v5.0.0 @@ -384,7 +375,8 @@ protected function primaryLinkLookup(): void * @throws \RuntimeException */ - public function getPrimaryLink(bool $lookup=false) { + public function getPrimaryLink(bool $lookup=false): Object + { // Did we already figure this out? (But only if $lookup) if($lookup && isset($this->cur_pl->value)) { return $this->cur_pl; @@ -410,42 +402,45 @@ public function getPrimaryLink(bool $lookup=false) { $this->primaryLinkLookup(); } - if(!empty($this->cur_pl->value)) { - // Look up the link value to find the related entity + if(empty($this->cur_pl->value)) { + return $this->cur_pl; + } - $linkTableName = $this->$modelsName->getPrimaryLinkTableName($this->cur_pl->attr); - $linkTable = $this->getTableLocator()->get($linkTableName); + // Look up the link value to find the related entity - $this->set('vv_primary_link_model', $linkTableName); + $linkTableName = $this->$modelsName->getPrimaryLinkTableName($this->cur_pl->attr); + $linkTable = $this->getTableLocator()->get($linkTableName); - try { - $plObj = $linkTable->findById($this->cur_pl->value)->firstOrFail(); + $this->set('vv_primary_link_model', $linkTableName); - $this->set('vv_primary_link_obj', $plObj); + try { + $plObj = $linkTable->findById($this->cur_pl->value)->firstOrFail(); - // While we're here, note the CO since we'll probably need it soon - if(!empty($plObj->co_id)) { - $this->cur_pl->co_id = $plObj->co_id; - } elseif(method_exists($linkTable, 'findCoForRecord')) { - $this->cur_pl->co_id = $linkTable->findCoForRecord((int)$this->cur_pl->value); - } - } - catch(RecordNotFoundException $e) { - $this->llog('error', "Could not find value '" . $this->cur_pl->value . "' for primary link object " . $linkTableName); - // Mask this with a generic UnauthorizedException - throw new UnauthorizedException(__d('error', 'perm')); + $this->set('vv_primary_link_obj', $plObj); + + // While we're here, note the CO since we'll probably need it soon + if(!empty($plObj->co_id)) { + $this->cur_pl->co_id = $plObj->co_id; + } elseif(method_exists($linkTable, 'findCoForRecord')) { + $this->cur_pl->co_id = $linkTable->findCoForRecord((int)$this->cur_pl->value); } } - + catch(RecordNotFoundException $e) { + $this->llog('error', "Could not find value '" . $this->cur_pl->value . "' for primary link object " . $linkTableName); + // Mask this with a generic UnauthorizedException + throw new UnauthorizedException(__d('error', 'perm')); + } + return $this->cur_pl; } - + /** * Get the redirect goal for this table. * + * @param string $action Action + * + * @return string|null Redirect goal * @since COmanage Registry v5.0.0 - * @param string $action Action - * @return string Redirect goal */ protected function getRedirectGoal(string $action): ?string {