Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consistent entry ID type (32-bit compatibility) #5213

Merged
merged 10 commits into from Mar 22, 2023

Conversation

Alkarex
Copy link
Member

@Alkarex Alkarex commented Mar 19, 2023

Entry IDs (which are 64-bit integers) must be processed as string to be compatible with 32-bit platforms.
In some previous type hints PR, this was a bit messed with.
(NB: Category / Feed / Tag IDs are 32-bit integers, so they can be passed as int without problem)

#5212 must be merged first.

P.S. The last commits add PHPStan level 6 for affected files

vendor/bin/phpstan analyse --level 6 app/Models/EntryDAO.php app/Models/EntryDAOPGSQL.php app/Models/EntryDAOSQLite.php

The interface was not used, and it was preventing more precise types for the different `searchById()` methods, as they each have different input and output types.
Entry IDs (which are 64-bit integers) must be processed as string to be compatible with 32-bit platforms
@Alkarex Alkarex added this to the 1.22.0 milestone Mar 19, 2023
@Alkarex Alkarex added API 🤝 API for other clients System care Everything related to the system care labels Mar 19, 2023
@Alkarex Alkarex changed the title Consistent entry ID (32-bit compatibility) Consistent entry ID type (32-bit compatibility) Mar 19, 2023
return $this->entriesToIdList($entries);
}

/**
* @return integer|false
*/
private function setItemAsRead(int $id) {
private function setItemAsRead(string $id) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example, which was breaking 32-bit compatibility

@@ -303,7 +307,7 @@ public function updateEntry(array $valuesTmp) {
* @todo simplify the query by removing the str_repeat. I am pretty sure
* there is an other way to do that.
*
* @param integer|array $ids
* @param string|array<string> $ids
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example of wrong type hint, breaking 32-bit compatibility

@Alkarex Alkarex merged commit e750448 into FreshRSS:edge Mar 22, 2023
1 check passed
@Alkarex Alkarex deleted the consistent-entry-id branch March 22, 2023 08:57
Alkarex added a commit to Alkarex/FreshRSS that referenced this pull request Jan 18, 2024
Alkarex added a commit that referenced this pull request Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API 🤝 API for other clients System care Everything related to the system care
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants