fix(appinfo): properly type info parser and app manager for appinfo.xml types #60066
There are no files selected for viewing
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,11 @@ | |
| use OCP\ICache; | ||
| use function simplexml_load_string; | ||
|
|
||
| /** | ||
| * @psalm-import-type AppInfoLocalizedEntry from \OCP\App\AppInfoDefinition | ||
| * @psalm-import-type AppInfoXmlDefinition from \OCP\App\AppInfoDefinition | ||
| * @psalm-import-type AppInfoDefinition from \OCP\App\AppInfoDefinition | ||
| */ | ||
| class InfoParser { | ||
| /** | ||
| * @param ICache|null $cache | ||
|
|
@@ -21,15 +26,15 @@ public function __construct( | |
|
|
||
| /** | ||
| * @param string $file the xml file to be loaded | ||
| * @return AppInfoXmlDefinition|null - The parsed app info or null if an error occurred | ||
| */ | ||
| public function parse(string $file): ?array { | ||
| if (!file_exists($file)) { | ||
| return null; | ||
| } | ||
|
|
||
| $fileCacheKey = $file . filemtime($file); | ||
| if ($this->cache !== null) { | ||
| if ($cachedValue = $this->cache->get($fileCacheKey)) { | ||
| return json_decode($cachedValue, true); | ||
| } | ||
|
|
@@ -42,14 +47,14 @@ public function parse(string $file): ?array { | |
| libxml_clear_errors(); | ||
| return null; | ||
| } | ||
|
|
||
| $array = $this->xmlToArray($xml); | ||
| if (is_string($array)) { | ||
| return null; | ||
| } | ||
|
|
||
| if (!array_key_exists('description', $array)) { | ||
| $array['description'] = ''; | ||
|
Comment on lines
+56
to
+57
Contributor
Author
There was a problem hiding this comment. @provokateurin there is no info inside the info, the The new code is just moved from |
||
| } | ||
| if (!array_key_exists('remote', $array)) { | ||
| $array['remote'] = []; | ||
|
|
@@ -166,11 +171,8 @@ public function parse(string $file): ?array { | |
| if (isset($array['activity']['providers']['provider']) && is_array($array['activity']['providers']['provider'])) { | ||
| $array['activity']['providers'] = $array['activity']['providers']['provider']; | ||
| } | ||
| if (isset($array['collaboration']['plugins']['plugin']) && is_array($array['collaboration']['plugins']['plugin'])) { | ||
| $array['collaboration']['plugins'] = $array['collaboration']['plugins']['plugin']; | ||
| } | ||
|
Comment on lines
+174
to
176
Contributor
Author
There was a problem hiding this comment. @provokateurin here the old code does nothing there are no such elements. The new code fixes a problem where wrong elements were parsed. This is similar to all other parsing of such elements in this function, e.g. see the if block before this comment. |
||
| if (isset($array['settings']['admin']) && !is_array($array['settings']['admin'])) { | ||
| $array['settings']['admin'] = [$array['settings']['admin']]; | ||
|
|
@@ -211,6 +213,9 @@ public function parse(string $file): ?array { | |
| $array['category'] = [$array['category']]; | ||
| } | ||
|
|
||
| /** | ||
| * @var AppInfoXmlDefinition $array | ||
| */ | ||
|
Comment on lines
+216
to
+218
Member
There was a problem hiding this comment. This While I would like to see this, it's probably a bit overkill for this PR 🙈
Contributor
Author
There was a problem hiding this comment. Lets do this in a follow up, I think it makes sense but not in the scope of this PR |
||
| if ($this->cache !== null) { | ||
| $this->cache->set($fileCacheKey, json_encode($array)); | ||
| } | ||
|
|
@@ -283,27 +288,42 @@ public function xmlToArray(\SimpleXMLElement $xml): array|string { | |
|
|
||
| /** | ||
| * Select the appropriate l10n version for fields name, summary and description | ||
| * | ||
| * @param AppInfoXmlDefinition $data | ||
| * @return AppInfoDefinition | ||
| */ | ||
| public function applyL10N(array $data, string $lang): array { | ||
| // Ensure name is set and convert arrays to strings | ||
| if (!isset($data['name'])) { | ||
| $data['name'] = ''; | ||
| } elseif (is_array($data['name'])) { | ||
| $data['name'] = $this->findBestL10NOption($data['name'], $lang); | ||
| } | ||
| $data['name'] = trim((string)$data['name']); | ||
|
|
||
| if (!isset($data['summary'])) { | ||
| $data['summary'] = ''; | ||
| } elseif (is_array($data['summary'])) { | ||
| $data['summary'] = $this->findBestL10NOption($data['summary'], $lang); | ||
| } | ||
| $data['summary'] = trim((string)$data['summary']); | ||
|
|
||
| // Ensure description is set and convert arrays to strings | ||
| if (!isset($data['description'])) { | ||
| $data['description'] = ''; | ||
| } elseif (is_array($data['description'])) { | ||
| $data['description'] = trim($this->findBestL10NOption($data['description'], $lang)); | ||
| } | ||
| $data['description'] = trim((string)$data['description']); | ||
|
|
||
| return $data; | ||
| } | ||
|
|
||
| /** | ||
| * @param AppInfoLocalizedEntry|list<string|AppInfoLocalizedEntry> $options - The available l10n options for a field | ||
|
susnux marked this conversation as resolved.
|
||
| * @param string $lang - The desired language code | ||
| * @return string - The best matching l10n option for the given language | ||
| */ | ||
| protected function findBestL10NOption(array $options, string $lang): string { | ||
| // only a single option | ||
| if (isset($options['@value'])) { | ||
|
|
||
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.