Daniel Kinzler has uploaded a new change for review. https://gerrit.wikimedia.org/r/49647
Change subject: (bug 44636/hot fix) added EntitySerializer option setIncludeValuesWithMissingReferences ...................................................................... (bug 44636/hot fix) added EntitySerializer option setIncludeValuesWithMissingReferences This is basically I7d0a971b from Master. Added EntitySerializationOptions::setIncludeValuesWithMissingReferences() which can be set to false to change serializer behavior to not include Snaks using missed properties (e.g. missing because of deletion). With the option set to false, this can also end in a whole Reference or Claim not being included in the following cases: - Claim: If the main Snak is affected - References: If all Snaks of the Reference are affected or if the Reference only consists of one Snak which is affected. * made all related serializers aware of the option * if a serializer runs into the option and one of above cases, an empty array will be returned as serialization. Upstream serializers will check for an empty array being returned. * Introduced a new SerializerObject, renamed the old one to GenericSerializerObject and added a todo to move it into core or out of the Wikibase namespace. * Changed EntityView to use the newly introduced option so frontend wouldn't run into the situation where it had to get the missing entities from the local entity store. * minor fix in claimview.js to handle removed properties used in references snaks. Change-Id: Ife7e2d087748974f3e0dfb8fd80916baefb75b5a --- M lib/includes/serializers/ByPropertyListSerializer.php M lib/includes/serializers/ClaimSerializer.php M lib/includes/serializers/ReferenceSerializer.php M lib/includes/serializers/SerializationOptions.php M lib/includes/serializers/Serializer.php M lib/includes/serializers/SerializerObject.php M lib/includes/serializers/SnakSerializer.php M lib/resources/jquery.wikibase/jquery.wikibase.claimview.js M lib/tests/phpunit/serializers/ClaimSerializerTest.php M repo/includes/EntityView.php 10 files changed, 120 insertions(+), 12 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase refs/changes/47/49647/1 diff --git a/lib/includes/serializers/ByPropertyListSerializer.php b/lib/includes/serializers/ByPropertyListSerializer.php index 08b61eb..8afbeba 100644 --- a/lib/includes/serializers/ByPropertyListSerializer.php +++ b/lib/includes/serializers/ByPropertyListSerializer.php @@ -29,6 +29,7 @@ * * @licence GNU GPL v2+ * @author Jeroen De Dauw < [email protected] > + * @author Daniel Werner < [email protected] > */ class ByPropertyListSerializer extends SerializerObject { @@ -87,13 +88,26 @@ $serializedObjects = array(); foreach ( $objects->getByPropertyId( $propertyId ) as $object ) { - $serializedObjects[] = $this->elementSerializer->getSerialized( $object ); + $serializedElement = $this->elementSerializer->getSerialized( $object ); + + if( !$this->options->getIncludeValuesWithMissingReferences() + && empty( $serializedElement ) + ) { + continue; + } + $serializedObjects[] = $serializedElement; } $this->setIndexedTagName( $serializedObjects, $this->elementName ); $propertyId = new \Wikibase\EntityId( \Wikibase\Property::ENTITY_TYPE, $propertyId ); + if( !$this->options->getIncludeValuesWithMissingReferences() + && EntityContentFactory::singleton()->getFromId( $propertyId ) === null + ) { + continue; + } + if ( $this->options->shouldIndexTags() ) { $serializedObjects['id'] = $propertyId->getPrefixedId(); $serialization[] = $serializedObjects; diff --git a/lib/includes/serializers/ClaimSerializer.php b/lib/includes/serializers/ClaimSerializer.php index bc0dc54..9619687 100644 --- a/lib/includes/serializers/ClaimSerializer.php +++ b/lib/includes/serializers/ClaimSerializer.php @@ -121,7 +121,11 @@ $serialization['references'] = array(); foreach ( $claim->getReferences() as $reference ) { - $serialization['references'][] = $referenceSerializer->getSerialized( $reference ); + // reference can be empty if options->getIncludeValuesWithMissingReferences is false + $serializedRef = $referenceSerializer->getSerialized( $reference ); + if( !empty( $serializedRef ) ) { + $serialization['references'][] = $serializedRef; + } } if ( $serialization['references'] === array() ) { diff --git a/lib/includes/serializers/ReferenceSerializer.php b/lib/includes/serializers/ReferenceSerializer.php index 3709b91..720e7db 100644 --- a/lib/includes/serializers/ReferenceSerializer.php +++ b/lib/includes/serializers/ReferenceSerializer.php @@ -29,6 +29,7 @@ * * @licence GNU GPL v2+ * @author Jeroen De Dauw < [email protected] > + * @author Daniel Werner < [email protected] > */ class ReferenceSerializer extends SerializerObject implements Unserializer { @@ -54,7 +55,13 @@ $snakSerializer = new SnakSerializer( $this->options ); $snaksSerializer = new ByPropertyListSerializer( 'snak', $snakSerializer, $this->options ); - $serialization['snaks'] = $snaksSerializer->getSerialized( $reference->getSnaks() ); + $snakList = $snaksSerializer->getSerialized( $reference->getSnaks() ); + + if( !$this->options->getIncludeValuesWithMissingReferences() && empty( $snakList ) ) { + // don't want a serialized reference without any snaks if the option is set! + return array(); + } + $serialization['snaks'] = $snakList; return $serialization; } diff --git a/lib/includes/serializers/SerializationOptions.php b/lib/includes/serializers/SerializationOptions.php index 6823157..8b7f060 100644 --- a/lib/includes/serializers/SerializationOptions.php +++ b/lib/includes/serializers/SerializationOptions.php @@ -30,6 +30,7 @@ * * @licence GNU GPL v2+ * @author Jeroen De Dauw < [email protected] > + * @author Daniel Werner < [email protected] > */ class SerializationOptions { @@ -95,6 +96,7 @@ * * @licence GNU GPL v2+ * @author Jeroen De Dauw < [email protected] > + * @author Daniel Werner < [email protected] > */ class EntitySerializationOptions extends SerializationOptions { @@ -146,6 +148,16 @@ * @var string Element of the EntitySerializationOptions::SORT_ enum */ protected $sortDirection = self::SORT_NONE; + + /** + * Whether or not to include values into the serialization if Snaks use properties which do not + * exist for some reason. + * + * @since 0.4 + * + * @var bool + */ + protected $includeValuesWithMissingReferences = true; /** * Sets if keys should be used in the serialization. @@ -279,4 +291,29 @@ return $this->sortDirection; } + /** + * Sets whether the serialized output should include Snaks using missed properties (e.g. because + * of deletion). This can also end in a whole Reference or Claim not being included in the + * following cases: + * - Claim: If the main Snak is affected + * - References: If all Snaks of the Reference are affected or if the Reference only consists of + * one Snak which is affected. + * + * @since 0.4 + * + * @return bool + */ + public function setIncludeValuesWithMissingReferences( $value ) { + return $this->includeValuesWithMissingReferences = $value; + } + + /** + * + * @since 0.4 + * + * @return bool + */ + public function getIncludeValuesWithMissingReferences() { + return $this->includeValuesWithMissingReferences; + } } diff --git a/lib/includes/serializers/Serializer.php b/lib/includes/serializers/Serializer.php index da69de7..3ec153b 100644 --- a/lib/includes/serializers/Serializer.php +++ b/lib/includes/serializers/Serializer.php @@ -44,7 +44,8 @@ * * @param mixed $object * - * @return array + * @return array Can be an empty array in case the serializer is dealing with an invalid object + * of some and/or options are set accordingly. */ public function getSerialized( $object ); diff --git a/lib/includes/serializers/SerializerObject.php b/lib/includes/serializers/SerializerObject.php index a607835..aebf4e8 100644 --- a/lib/includes/serializers/SerializerObject.php +++ b/lib/includes/serializers/SerializerObject.php @@ -30,8 +30,11 @@ * * @licence GNU GPL v2+ * @author Jeroen De Dauw < [email protected] > + * + * @todo Move this into core or somewhere else but out of the Wikibase namespace since it is not + * Wikibase specific */ -abstract class SerializerObject implements Serializer { +abstract class GenericSerializerObject implements Serializer { /** * The ApiResult to use during serialization. @@ -97,3 +100,32 @@ } } + +/** + * Base class for Wikibase related ApiSerializers. + * + * @since 0.4 + * + * @file + * @ingroup WikibaseLib + * + * @licence GNU GPL v2+ + * @author Daniel Werner < [email protected] > + */ +abstract class SerializerObject extends GenericSerializerObject { + /** + * Constructor. + * + * @since 0.4 + * + * @param ApiResult $apiResult + * @param EntitySerializationOptions|null $options + */ + public function __construct( SerializationOptions $options = null ) { + if ( $options === null ) { + $options = new EntitySerializationOptions(); + } + + $this->options = $options; + } +} diff --git a/lib/includes/serializers/SnakSerializer.php b/lib/includes/serializers/SnakSerializer.php index bd86221..b4e1edc 100644 --- a/lib/includes/serializers/SnakSerializer.php +++ b/lib/includes/serializers/SnakSerializer.php @@ -48,11 +48,20 @@ throw new MWException( 'SnakSerializer can only serialize Snak objects' ); } + $propertyId = $snak->getPropertyId(); + + if( !$this->options->getIncludeValuesWithMissingReferences() ) { + $entityContent = EntityContentFactory::singleton()->getFromId( $propertyId ); + if( $entityContent === null ) { + return array(); + } + } + $serialization = array(); $serialization['snaktype'] = $snak->getType(); - $serialization['property'] = $snak->getPropertyId()->getPrefixedId(); + $serialization['property'] = $propertyId->getPrefixedId(); // TODO: we might want to include the data type of the property here as well diff --git a/lib/resources/jquery.wikibase/jquery.wikibase.claimview.js b/lib/resources/jquery.wikibase/jquery.wikibase.claimview.js index d3f2912..885f63a 100644 --- a/lib/resources/jquery.wikibase/jquery.wikibase.claimview.js +++ b/lib/resources/jquery.wikibase/jquery.wikibase.claimview.js @@ -235,9 +235,9 @@ } ); } ); - if ( this._claim || this.options.predefined.mainSnak ) { + if ( this.value() || this.options.predefined.mainSnak ) { var propertyName; - if ( this._claim ) { + if ( this.value() ) { propertyName = wb.entities[this.mainSnak().getPropertyId()].label; } else { propertyName = wb.entities[this.options.predefined.mainSnak.property].label; diff --git a/lib/tests/phpunit/serializers/ClaimSerializerTest.php b/lib/tests/phpunit/serializers/ClaimSerializerTest.php index e7fe6fa..2490a43 100644 --- a/lib/tests/phpunit/serializers/ClaimSerializerTest.php +++ b/lib/tests/phpunit/serializers/ClaimSerializerTest.php @@ -2,7 +2,8 @@ namespace Wikibase\Test; use Wikibase\Statement; -use Wikibase\Lib\Serializers\ClaimSerializer; +use Wikibase\Claim; +use Wikibase\ClaimSerializer; /** * Tests for the Wikibase\ClaimSerializer class. @@ -59,13 +60,13 @@ $id = new \Wikibase\EntityId( \Wikibase\Property::ENTITY_TYPE, 42 ); - $validArgs[] = new \Wikibase\Claim( new \Wikibase\PropertyNoValueSnak( $id ) ); + $validArgs[] = new Claim( new \Wikibase\PropertyNoValueSnak( $id ) ); - $validArgs[] = new \Wikibase\Claim( new \Wikibase\PropertySomeValueSnak( $id ) ); + $validArgs[] = new Claim( new \Wikibase\PropertySomeValueSnak( $id ) ); $validArgs = $this->arrayWrap( $validArgs ); - $claim = new \Wikibase\Claim( new \Wikibase\PropertyNoValueSnak( $id ) ); + $claim = new Claim( new \Wikibase\PropertyNoValueSnak( $id ) ); $validArgs[] = array( $claim, diff --git a/repo/includes/EntityView.php b/repo/includes/EntityView.php index 54049fa..1eb0530 100644 --- a/repo/includes/EntityView.php +++ b/repo/includes/EntityView.php @@ -725,6 +725,9 @@ $serializationOptions = new \Wikibase\Lib\Serializers\EntitySerializationOptions(); $serializationOptions->addProp( 'sitelinks' ); + // TODO (bug 44639): add handling for Claims/References with deleted properties in their + // Snaks and set this option to true again + $serializationOptions->setIncludeValuesWithMissingReferences( false ); $serializer = \Wikibase\Lib\Serializers\EntitySerializer::newForEntity( $entity, $serializationOptions ); -- To view, visit https://gerrit.wikimedia.org/r/49647 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ife7e2d087748974f3e0dfb8fd80916baefb75b5a Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: mw1.21-wmf10 Gerrit-Owner: Daniel Kinzler <[email protected]> Gerrit-Reviewer: Daniel Werner <[email protected]> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
