Page MenuHomePhabricator

Remove SerializerFactory::newEntitySerializer
Closed, InvalidPublic

Description

SerializerFactory::newEntitySerializer only works for Items and Properties. The code that currently uses it should however use a serializer that supports all configured entity types, as returned by WikibaseRepo::getEntityDeserializer. The same applies for get DeserializerFactory::newEntityDeserializer.

SerializerFactory could still have newItemSerializer() and newPropertySerializer() methods. That would be consistent with the idea that this class provides serializers for the basic entity types.

One specific issue caused by code using SerializerFactory::newEntitySerializer instead of instance of WikibaseRepo::getEntityDeserializer is T160426.

Initially, we planned to add support for custom entity types to SerializerFactory, see T157596.

Note that DeserializerFactory::newEntityDeserializer has one usage which could be inlined, and SerializerFactory::newEntitySerializer is only used in tests, where it could be replaced by newItemSerializer, or inlined.

Related Objects

StatusSubtypeAssignedTask
OpenFeatureNone
OpenFeatureNone
OpenFeatureNone
OpenFeatureNone
OpenNone
OpenNone
OpenNone
DuplicateNone
OpenFeatureNone
OpenFeatureNone
DuplicateNone
ResolvedNone
ResolvedNone
ResolvedNone
DuplicateNone
InvalidLydia_Pintscher
ResolvedLydia_Pintscher
ResolvedLydia_Pintscher
OpenNone
ResolvedLydia_Pintscher
ResolvedLydia_Pintscher
ResolvedLydia_Pintscher
ResolvedAddshore
ResolvedAddshore
ResolvedAddshore
ResolvedLydia_Pintscher
ResolvedAddshore
ResolvedAddshore
Resolvedjcrespo
ResolvedAddshore
ResolvedAddshore
ResolvedBawolff
ResolvedAddshore
ResolvedAddshore
ResolvedAddshore
ResolvedAddshore
ResolvedAddshore
DuplicateWMDE-leszek
ResolvedWMDE-leszek
ResolvedAddshore
ResolvedAddshore
ResolvedAddshore
ResolvedAddshore
ResolvedAddshore
ResolvedAddshore
ResolvedAddshore
Resolved Marostegui
ResolvedAddshore
ResolvedAddshore
DeclinedNone
ResolvedLydia_Pintscher
ResolvedLydia_Pintscher
OpenNone
StalledNone
OpenNone
ResolvedAddshore
Resolvedthiemowmde
ResolvedAddshore
InvalidNone
Resolvedthiemowmde

Event Timeline

daniel updated the task description. (Show Details)
thiemowmde lowered the priority of this task from Low to Lowest.EditedMar 15 2017, 2:31 PM

Even if I agree we should continue to work on this code, the most trivial answer is "just do not use SerializerFactory::newEntitySerializer". See https://gerrit.wikimedia.org/r/342816 for an actual commit in that direction.

I totally agree that many things in this component are badly named, because all these terms ("entity", "data model") do have different meanings, depending on the context. This tends to become very confusing, and already cause us to waste a lot of time failing for wrong assumptions.

Change 342816 had a related patch set uploaded (by Thiemo Mättig (WMDE)):
[mediawiki/extensions/Wikibase] Do not use SerializerFactory::newEntitySerializer in tests

https://gerrit.wikimedia.org/r/342816

Change 342816 merged by jenkins-bot:
[mediawiki/extensions/Wikibase] Do not use SerializerFactory::newEntitySerializer in tests

https://gerrit.wikimedia.org/r/342816