Wikibase (30 usages found) lib (30 usages found) packages (30 usages found) wikibase (30 usages found) changes (8 usages found) src (8 usages found) EntityDiffChangedAspects.php (4 usages found) 6 use Serializable; 18 class EntityDiffChangedAspects implements Serializable { 139 * @see Serializable::serialize 148 * @see Serializable::unserialize RepoRevisionIdentifier.php (4 usages found) 7 use Serializable; 17 class RepoRevisionIdentifier implements Serializable { 79 * @see Serializable::serialize 88 * @see Serializable::unserialize data-model (22 usages found) src (20 usages found) Entity (8 usages found) EntityId.php (2 usages found) 5 use Serializable; 10 interface EntityId extends Serializable { EntityIdValue.php (2 usages found) 27 * @see Serializable::serialize 40 * @see Serializable::unserialize ItemId.php (2 usages found) 63 * @see Serializable::serialize 74 * @see Serializable::unserialize NumericPropertyId.php (2 usages found) 59 * @see Serializable::serialize 68 * @see Serializable::unserialize Snak (8 usages found) PropertyValueSnak.php (2 usages found) 50 * @see Serializable::serialize 61 * @see Serializable::unserialize Snak.php (2 usages found) 5 use Serializable; 17 interface Snak extends Serializable, PropertyIdProvider { SnakList.php (2 usages found) 268 * @see Serializable::serialize 277 * @see Serializable::unserialize SnakObject.php (2 usages found) 96 * @see Serializable::serialize 107 * @see Serializable::unserialize ReferenceList.php (4 usages found) 9 use Serializable; 27 class ReferenceList implements Countable, IteratorAggregate, Serializable { 225 * @see Serializable::serialize 256 * @see Serializable::unserialize tests (2 usages found) fixtures (2 usages found) CustomEntityId.php (2 usages found) 16 * @see Serializable::serialize 25 * @see Serializable::unserialize
Description
Details
Event Timeline
I’d like to think we don’t rely on serialize() of Wikibase entity IDs anywhere, but that’s probably a fool’s hope. (E.g., I believe the caching layer uses PHP serialization, and we probably put entity ID objects into and out of caches somewhere, even if strings would be cleaner.)
The good news that this should be reasonably straightforward, the serialization is just a string and the deserialization just needs to reconstruct a small bit of internal state afterwards. (I believe this is true for all the entity ID types we have, so including T301248 and T301223.)
…And I should learn to read the full task description, this one isn’t only about entity IDs. 🤦
But from a quick look through them, it seems like the other affected classes also don’t have super complicated serialization methods. In fact the new magic method interface should be more easy to implement, since we can directly return arrays in some cases instead of wrapping them in JSON.
Sounds pretty much like what I did for the couple of patches to numerous classes in MW Core.
Make the new __ methods handle the array version, keep the old non __ prefixed versions with any special handling (turning to strings, or whatever else) and make them call the __ versions
https://gerrit.wikimedia.org/r/c/756964
https://gerrit.wikimedia.org/r/c/758800
And if the tests are about right, you should know whether your changes are correct ;D
And thankfully we already run php74 in gate-and-submit, so at least one job should cover the new methods 😌
Change 762094 had a related patch set uploaded (by Hoo man; author: Hoo man):
[mediawiki/extensions/Wikibase@master] Implement __serialize/__unserialize for PHP 8.1 support
Change 762094 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Implement __serialize/__unserialize for PHP 8.1 support
Change 773521 had a related patch set uploaded (by Hoo man; author: Hoo man):
[mediawiki/extensions/Wikibase@master] DataModel\Entity: Add __serialize/__unserialize for EntityIds
I think the Diff package, used by Wikibase, could also be included in this task; somebody already uploaded a pull request for it: https://github.com/wmde/Diff/pull/128
(Edit: See also the more narrowly scoped PR https://github.com/wmde/Diff/pull/129)
As far as I can see, all remaining issues can only be addressed once T305785 is fixed.
Change 773521 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] DataModel\Entity: Add __serialize/__unserialize for EntityIds
Moving back to Doing, AFAICT there’s nothing to review here at the moment. I think the remaining open part is the DataValues library.
Change 842427 had a related patch set uploaded (by Hoo man; author: Hoo man):
[mediawiki/extensions/Wikibase@master] DM: Add abstract __(un)serialize to SerializableEntityId
Change 842428 had a related patch set uploaded (by Hoo man; author: Hoo man):
[mediawiki/extensions/Wikibase@master] DM: Add __(un)serialize to Snak (and implementations)
Change 842434 had a related patch set uploaded (by Hoo man; author: Hoo man):
[mediawiki/extensions/WikibaseMediaInfo@master] MediaInfoId: Add (return) type hints to __(un)serialize
Change 842436 had a related patch set uploaded (by Hoo man; author: Hoo man):
[mediawiki/extensions/WikibaseQualityConstraints@master] Use MockBuilder::getMockForAbstractClass for SerializableEntityId
Change 842436 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Use MockBuilder::getMockForAbstractClass for SerializableEntityId
Change 842434 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] MediaInfoId: Add (return) type hints to __(un)serialize
Change 842427 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] DM: Add abstract __(un)serialize to SerializableEntityId
Change 842428 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] DM: Add __(un)serialize to Snak (and implementations)
Change 845589 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Declare compatibility with data-values/number ^0.12.0
Change 845590 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Declare compatibility with data-values/number ^0.12.0
Change 848265 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] WIP: Drop compatibility with older data-values versions
Change 848346 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[mediawiki/vendor@master] Update data-values/*
Change 845589 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Update data-values versions in composer.json
Change 845590 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Declare compatibility with data-values/number ^0.12.3
Change 850189 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] FederatedPropertyId: Add __serialize()/__unserialize()
Change 850189 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] FederatedPropertyId: Add __serialize()/__unserialize()
Change 848265 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Drop compatibility with older data-values versions
Moving back to doing; this PHP 8.1 CI log has two Serializable-related deprecation warnings left:
24) Wikibase\Repo\Tests\Normalization\CommonsMediaValueNormalizerTest::testNormalize_badType Mock_DataValue_5fa3bfea implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) Deprecated: Mock_EntityId_c95bc3e8 implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in /workspace/src/vendor/phpunit/phpunit/src/Framework/MockObject/MockClass.php(51) : eval()'d code on line 3
That second one isn’t reported as belonging to any particular test case, so I guess we just need to grep for calls like $this->createMock( EntityId::class ). Shouldn’t require any more library updates, though – I think all the DataValues stuff is done at last.
Correction: there might be more Serializable-related warnings, because the above job failed before it ran the DB tests. (But it at least finished running the non-DB tests and reported all the errors in there, instead of crashing partway through as it used to.)
git grep 'createMock.*EntityId:' finds 16 occurrences. I think a convenient way to make all of them work would be to add __serialize() / __unserialize() methods to the EntityId interface, so that PHPUnit will mock them automatically. (This would be a breaking change, but the data model RELEASE-NOTES.md already list the in-progress version as a major version update (10.0.0) with breaking changes.) I wonder if this would be acceptable?
Change 851651 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Use BooleanValue in CommonsMediaValueNormalizerTest
Change 851651 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Use BooleanValue in CommonsMediaValueNormalizerTest
Change 852162 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Wikibase@master] Make a few tests more compatible with PHP 8.1
Change 852162 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Make a few tests more compatible with PHP 8.1
Change 853286 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Add __serialize() and __unserialize() to EntityId
git grep 'createMock.*EntityId:' finds 16 occurrences. I think a convenient way to make all of them work would be to add __serialize() / __unserialize() methods to the EntityId interface, so that PHPUnit will mock them automatically. (This would be a breaking change, but the data model RELEASE-NOTES.md already list the in-progress version as a major version update (10.0.0) with breaking changes.) I wonder if this would be acceptable?
Did that in the above change, since I haven’t heard any complaints about it yet. Let’s see if CI is happy with it.
Change 853286 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Add __serialize() and __unserialize() to EntityId
PHP 8.1 is voting in gate-and-submit now, so I think we can be confident that this is done.
Change 880548 had a related patch set uploaded (by Addshore; author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@REL1_39] FederatedPropertyId: Add __serialize()/__unserialize()
Change 880549 had a related patch set uploaded (by Addshore; author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@REL1_39] Add __serialize() and __unserialize() to EntityId
Change 880550 had a related patch set uploaded (by Addshore; author: Hoo man):
[mediawiki/extensions/Wikibase@REL1_39] DataModel\Entity: Add __serialize/__unserialize for EntityIds
Change 880551 had a related patch set uploaded (by Addshore; author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@REL1_39] Drop compatibility with older data-values versions
Change 880923 had a related patch set uploaded (by Addshore; author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@REL1_39] Declare compatibility with data-values/number ^0.12.3
Change 880550 abandoned by Addshore:
[mediawiki/extensions/Wikibase@REL1_39] DataModel\Entity: Add __serialize/__unserialize for EntityIds
Reason:
Change 880551 abandoned by Addshore:
[mediawiki/extensions/Wikibase@REL1_39] Drop compatibility with older data-values versions
Reason:
Change 880549 abandoned by Addshore:
[mediawiki/extensions/Wikibase@REL1_39] Add __serialize() and __unserialize() to EntityId
Reason:
Change 880548 abandoned by Addshore:
[mediawiki/extensions/Wikibase@REL1_39] FederatedPropertyId: Add __serialize()/__unserialize()
Reason:
Change 880923 abandoned by Addshore:
[mediawiki/extensions/WikibaseQualityConstraints@REL1_39] Declare compatibility with data-values/number ^0.12.3
Reason: