Issue #3073942 by jsacksick, jorgik: Commerce entities implementing...

Issue #3073942 by jsacksick, jorgik: Commerce entities implementing EntityOwnerInterface should always return a user.

modules/payment/src/Entity/PaymentMethod.php

@@ -2,13 +2,13 @@
 
 namespace Drupal\commerce_payment\Entity;
 
+use Drupal\commerce\EntityOwnerTrait;
 use Drupal\Core\Entity\ContentEntityBase;
 use Drupal\Core\Entity\EntityChangedTrait;
 use Drupal\Core\Entity\EntityMalformedException;
 use Drupal\Core\Entity\EntityStorageInterface;
 use Drupal\Core\Entity\EntityTypeInterface;
 use Drupal\Core\Field\BaseFieldDefinition;
-use Drupal\user\UserInterface;
 use Drupal\profile\Entity\ProfileInterface;
 
 /**
@@ -58,6 +58,7 @@ use Drupal\profile\Entity\ProfileInterface;
 class PaymentMethod extends ContentEntityBase implements PaymentMethodInterface {
 
   use EntityChangedTrait;
+  use EntityOwnerTrait;
 
   /**
    * {@inheritdoc}
@@ -104,36 +105,6 @@ class PaymentMethod extends ContentEntityBase implements PaymentMethodInterface
     return $this->get('payment_gateway_mode')->value;
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function getOwner() {
-    return $this->get('uid')->entity;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function getOwnerId() {
-    return $this->getEntityKey('owner');
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function setOwnerId($uid) {
-    $this->set('uid', $uid);
-    return $this;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function setOwner(UserInterface $account) {
-    $this->set('uid', $account->id());
-    return $this;
-  }
-
   /**
    * {@inheritdoc}
    */
@@ -259,6 +230,7 @@ class PaymentMethod extends ContentEntityBase implements PaymentMethodInterface
    */
   public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
     $fields = parent::baseFieldDefinitions($entity_type);
+    $fields += static::ownerBaseFieldDefinitions($entity_type);
 
     $fields['payment_gateway'] = BaseFieldDefinition::create('entity_reference')
       ->setLabel(t('Payment gateway'))
@@ -271,12 +243,9 @@ class PaymentMethod extends ContentEntityBase implements PaymentMethodInterface
       ->setDescription(t('The payment gateway mode.'))
       ->setRequired(TRUE);
 
-    $fields['uid'] = BaseFieldDefinition::create('entity_reference')
+    $fields['uid']
       ->setLabel(t('Owner'))
       ->setDescription(t('The payment method owner.'))
-      ->setSetting('target_type', 'user')
-      ->setSetting('handler', 'default')
-      ->setDefaultValueCallback('Drupal\commerce_payment\Entity\PaymentMethod::getCurrentUserId')
       ->setDisplayOptions('view', [
         'label' => 'above',
         'type' => 'author',
@@ -346,16 +315,4 @@ class PaymentMethod extends ContentEntityBase implements PaymentMethodInterface
     return $fields;
   }
 
-  /**
-   * Default value callback for 'uid' base field definition.
-   *
-   * @see ::baseFieldDefinitions()
-   *
-   * @return array
-   *   An array of default values.
-   */
-  public static function getCurrentUserId() {
-    return [\Drupal::currentUser()->id()];
-  }
-
 }

modules/payment/tests/src/Kernel/Entity/PaymentMethodTest.php

@@ -7,6 +7,7 @@ use Drupal\commerce_payment\Entity\PaymentMethod;
 use Drupal\commerce_payment\Plugin\Commerce\PaymentMethodType\CreditCard;
 use Drupal\profile\Entity\Profile;
 use Drupal\Tests\commerce_order\Kernel\OrderKernelTestBase;
+use Drupal\user\UserInterface;
 
 /**
  * Tests the payment method entity.
@@ -107,7 +108,13 @@ class PaymentMethodTest extends OrderKernelTestBase {
     $this->assertEquals($this->user, $payment_method->getOwner());
     $this->assertEquals($this->user->id(), $payment_method->getOwnerId());
     $payment_method->setOwnerId(0);
-    $this->assertEquals(NULL, $payment_method->getOwner());
+    $this->assertInstanceOf(UserInterface::class, $payment_method->getOwner());
+    $this->assertTrue($payment_method->getOwner()->isAnonymous());
+    // Non-existent/deleted user ID.
+    $payment_method->setOwnerId(890);
+    $this->assertInstanceOf(UserInterface::class, $payment_method->getOwner());
+    $this->assertTrue($payment_method->getOwner()->isAnonymous());
+    $this->assertEquals(890, $payment_method->getOwnerId());
     $payment_method->setOwnerId($this->user->id());
     $this->assertEquals($this->user, $payment_method->getOwner());
     $this->assertEquals($this->user->id(), $payment_method->getOwnerId());

modules/product/src/Entity/Product.php

@@ -3,13 +3,13 @@
 namespace Drupal\commerce_product\Entity;
 
 use Drupal\commerce\Entity\CommerceContentEntityBase;
+use Drupal\commerce\EntityOwnerTrait;
 use Drupal\Core\Cache\Cache;
 use Drupal\Core\Entity\EntityPublishedTrait;
 use Drupal\Core\Entity\EntityChangedTrait;
 use Drupal\Core\Entity\EntityStorageInterface;
 use Drupal\Core\Entity\EntityTypeInterface;
 use Drupal\Core\Field\BaseFieldDefinition;
-use Drupal\user\UserInterface;
 
 /**
  * Defines the product entity class.
@@ -80,6 +80,7 @@ use Drupal\user\UserInterface;
 class Product extends CommerceContentEntityBase implements ProductInterface {
 
   use EntityChangedTrait;
+  use EntityOwnerTrait;
   use EntityPublishedTrait;
 
   /**
@@ -146,36 +147,6 @@ class Product extends CommerceContentEntityBase implements ProductInterface {
     return $this;
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function getOwner() {
-    return $this->get('uid')->entity;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function setOwner(UserInterface $account) {
-    $this->set('uid', $account->id());
-    return $this;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function getOwnerId() {
-    return $this->getEntityKey('owner');
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function setOwnerId($uid) {
-    $this->set('uid', $uid);
-    return $this;
-  }
-
   /**
    * {@inheritdoc}
    */
@@ -271,8 +242,10 @@ class Product extends CommerceContentEntityBase implements ProductInterface {
     foreach (array_keys($this->getTranslationLanguages()) as $langcode) {
       $translation = $this->getTranslation($langcode);
 
-      // If no owner has been set explicitly, make the anonymous user the owner.
-      if (!$translation->getOwner()) {
+      // Explicitly set the owner ID to 0 if the translation owner is anonymous
+      // (This will ensure we don't store a broken reference in case the user
+      // no longer exists).
+      if ($translation->getOwner()->isAnonymous()) {
         $translation->setOwnerId(0);
       }
     }
@@ -324,6 +297,7 @@ class Product extends CommerceContentEntityBase implements ProductInterface {
    */
   public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
     $fields = parent::baseFieldDefinitions($entity_type);
+    $fields += static::ownerBaseFieldDefinitions($entity_type);
     $fields += static::publishedBaseFieldDefinitions($entity_type);
 
     $fields['stores'] = BaseFieldDefinition::create('entity_reference')
@@ -340,13 +314,9 @@ class Product extends CommerceContentEntityBase implements ProductInterface {
       ->setDisplayConfigurable('form', TRUE)
       ->setDisplayConfigurable('view', TRUE);
 
-    $fields['uid'] = BaseFieldDefinition::create('entity_reference')
+    $fields['uid']
       ->setLabel(t('Author'))
       ->setDescription(t('The product author.'))
-      ->setSetting('target_type', 'user')
-      ->setSetting('handler', 'default')
-      ->setDefaultValueCallback('Drupal\commerce_product\Entity\Product::getCurrentUserId')
-      ->setTranslatable(TRUE)
       ->setDisplayConfigurable('view', TRUE)
       ->setDisplayOptions('form', [
         'type' => 'entity_reference_autocomplete',
@@ -450,16 +420,4 @@ class Product extends CommerceContentEntityBase implements ProductInterface {
     return $fields;
   }
 
-  /**
-   * Default value callback for 'uid' base field definition.
-   *
-   * @see ::baseFieldDefinitions()
-   *
-   * @return array
-   *   An array of default values.
-   */
-  public static function getCurrentUserId() {
-    return [\Drupal::currentUser()->id()];
-  }
-
 }

modules/product/src/Entity/ProductVariation.php

@@ -4,6 +4,7 @@ namespace Drupal\commerce_product\Entity;
 
 use Drupal\commerce\Entity\CommerceContentEntityBase;
 use Drupal\commerce\EntityHelper;
+use Drupal\commerce\EntityOwnerTrait;
 use Drupal\commerce_price\Price;
 use Drupal\Core\Cache\Cache;
 use Drupal\Core\Entity\EntityChangedTrait;
@@ -12,7 +13,6 @@ use Drupal\Core\Entity\EntityStorageInterface;
 use Drupal\Core\Entity\EntityTypeInterface;
 use Drupal\Core\Field\BaseFieldDefinition;
 use Drupal\Core\Url;
-use Drupal\user\UserInterface;
 use Symfony\Component\Routing\Exception\RouteNotFoundException;
 
 /**
@@ -89,6 +89,7 @@ use Symfony\Component\Routing\Exception\RouteNotFoundException;
 class ProductVariation extends CommerceContentEntityBase implements ProductVariationInterface {
 
   use EntityChangedTrait;
+  use EntityOwnerTrait;
   use EntityPublishedTrait;
 
   /**
@@ -240,36 +241,6 @@ class ProductVariation extends CommerceContentEntityBase implements ProductVaria
     return $this;
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function getOwner() {
-    return $this->get('uid')->entity;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function setOwner(UserInterface $account) {
-    $this->set('uid', $account->id());
-    return $this;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function getOwnerId() {
-    return $this->getEntityKey('owner');
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function setOwnerId($uid) {
-    $this->set('uid', $uid);
-    return $this;
-  }
-
   /**
    * {@inheritdoc}
    */
@@ -466,15 +437,12 @@ class ProductVariation extends CommerceContentEntityBase implements ProductVaria
    */
   public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
     $fields = parent::baseFieldDefinitions($entity_type);
+    $fields += static::ownerBaseFieldDefinitions($entity_type);
     $fields += static::publishedBaseFieldDefinitions($entity_type);
 
-    $fields['uid'] = BaseFieldDefinition::create('entity_reference')
+    $fields['uid']
       ->setLabel(t('Author'))
       ->setDescription(t('The variation author.'))
-      ->setSetting('target_type', 'user')
-      ->setSetting('handler', 'default')
-      ->setDefaultValueCallback('Drupal\commerce_product\Entity\ProductVariation::getCurrentUserId')
-      ->setTranslatable(TRUE)
       ->setDisplayConfigurable('form', TRUE);
 
     // The product backreference, populated by Product::postSave().
@@ -591,16 +559,4 @@ class ProductVariation extends CommerceContentEntityBase implements ProductVaria
     return $fields;
   }
 
-  /**
-   * Default value callback for 'uid' base field definition.
-   *
-   * @see ::baseFieldDefinitions()
-   *
-   * @return array
-   *   An array of default values.
-   */
-  public static function getCurrentUserId() {
-    return [\Drupal::currentUser()->id()];
-  }
-
 }

modules/product/tests/src/Kernel/Entity/ProductTest.php

@@ -5,6 +5,7 @@ namespace Drupal\Tests\commerce_product\Kernel\Entity;
 use Drupal\commerce_product\Entity\ProductVariation;
 use Drupal\commerce_product\Entity\Product;
 use Drupal\Tests\commerce\Kernel\CommerceKernelTestBase;
+use Drupal\user\UserInterface;
 
 /**
  * Tests the Product entity.
@@ -86,7 +87,13 @@ class ProductTest extends CommerceKernelTestBase {
     $this->assertEquals($this->user, $product->getOwner());
     $this->assertEquals($this->user->id(), $product->getOwnerId());
     $product->setOwnerId(0);
-    $this->assertEquals(NULL, $product->getOwner());
+    $this->assertInstanceOf(UserInterface::class, $product->getOwner());
+    $this->assertTrue($product->getOwner()->isAnonymous());
+    // Non-existent/deleted user ID.
+    $product->setOwnerId(891);
+    $this->assertInstanceOf(UserInterface::class, $product->getOwner());
+    $this->assertTrue($product->getOwner()->isAnonymous());
+    $this->assertEquals(891, $product->getOwnerId());
     $product->setOwnerId($this->user->id());
     $this->assertEquals($this->user, $product->getOwner());
     $this->assertEquals($this->user->id(), $product->getOwnerId());
@@ -95,6 +102,12 @@ class ProductTest extends CommerceKernelTestBase {
       'store',
       'url.query_args:v',
     ], $product->getCacheContexts());
+
+    // Ensure that we don't store a broken reference to the product owner.
+    $product->setOwnerId(900);
+    $this->assertEqual($product->getOwnerId(), 900);
+    $product->save();
+    $this->assertEqual($product->getOwnerId(), 0);
   }
 
   /**

modules/product/tests/src/Kernel/Entity/ProductVariationTest.php

@@ -8,6 +8,7 @@ use Drupal\commerce_product\Entity\ProductAttributeValue;
 use Drupal\commerce_product\Entity\ProductVariation;
 use Drupal\commerce_product\Entity\Product;
 use Drupal\Tests\commerce\Kernel\CommerceKernelTestBase;
+use Drupal\user\UserInterface;
 
 /**
  * Tests the Product variation entity.
@@ -125,7 +126,13 @@ class ProductVariationTest extends CommerceKernelTestBase {
     $this->assertEquals($this->user, $variation->getOwner());
     $this->assertEquals($this->user->id(), $variation->getOwnerId());
     $variation->setOwnerId(0);
-    $this->assertEquals(NULL, $variation->getOwner());
+    $this->assertInstanceOf(UserInterface::class, $variation->getOwner());
+    $this->assertTrue($variation->getOwner()->isAnonymous());
+    // Non-existent/deleted user ID.
+    $variation->setOwnerId(892);
+    $this->assertInstanceOf(UserInterface::class, $variation->getOwner());
+    $this->assertTrue($variation->getOwner()->isAnonymous());
+    $this->assertEquals(892, $variation->getOwnerId());
     $variation->setOwnerId($this->user->id());
     $this->assertEquals($this->user, $variation->getOwner());
     $this->assertEquals($this->user->id(), $variation->getOwnerId());

modules/store/src/Entity/Store.php

@@ -5,8 +5,8 @@ namespace Drupal\commerce_store\Entity;
 use CommerceGuys\Addressing\AddressFormat\AddressField;
 use CommerceGuys\Addressing\AddressFormat\FieldOverride;
 use Drupal\address\AddressInterface;
+use Drupal\commerce\EntityOwnerTrait;
 use Drupal\commerce_price\Entity\CurrencyInterface;
-use Drupal\user\UserInterface;
 use Drupal\Core\Entity\ContentEntityBase;
 use Drupal\Core\Entity\EntityStorageInterface;
 use Drupal\Core\Entity\EntityTypeInterface;
@@ -82,6 +82,8 @@ use Drupal\Core\Field\BaseFieldDefinition;
  */
 class Store extends ContentEntityBase implements StoreInterface {
 
+  use EntityOwnerTrait;
+
   /**
    * {@inheritdoc}
    */
@@ -97,36 +99,6 @@ class Store extends ContentEntityBase implements StoreInterface {
     return $this;
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public function getOwner() {
-    return $this->get('uid')->entity;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function setOwner(UserInterface $account) {
-    $this->set('uid', $account->id());
-    return $this;
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function getOwnerId() {
-    return $this->getEntityKey('owner');
-  }
-
-  /**
-   * {@inheritdoc}
-   */
-  public function setOwnerId($uid) {
-    $this->set('uid', $uid);
-    return $this;
-  }
-
   /**
    * {@inheritdoc}
    */
@@ -247,8 +219,10 @@ class Store extends ContentEntityBase implements StoreInterface {
     foreach (array_keys($this->getTranslationLanguages()) as $langcode) {
       $translation = $this->getTranslation($langcode);
 
-      // If no owner has been set explicitly, make the anonymous user the owner.
-      if (!$translation->getOwner()) {
+      // Explicitly set the owner ID to 0 if the translation owner is anonymous
+      // (This will ensure we don't store a broken reference in case the user
+      // no longer exists).
+      if ($translation->getOwner()->isAnonymous()) {
         $translation->setOwnerId(0);
       }
     }
@@ -284,6 +258,7 @@ class Store extends ContentEntityBase implements StoreInterface {
    */
   public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
     $fields = parent::baseFieldDefinitions($entity_type);
+    $fields += static::ownerBaseFieldDefinitions($entity_type);
 
     $fields['type'] = BaseFieldDefinition::create('entity_reference')
       ->setLabel(t('Type'))
@@ -291,11 +266,9 @@ class Store extends ContentEntityBase implements StoreInterface {
       ->setSetting('target_type', 'commerce_store_type')
       ->setReadOnly(TRUE);
 
-    $fields['uid'] = BaseFieldDefinition::create('entity_reference')
+    $fields['uid']
       ->setLabel(t('Owner'))
       ->setDescription(t('The store owner.'))
-      ->setDefaultValueCallback('Drupal\commerce_store\Entity\Store::getCurrentUserId')
-      ->setSetting('target_type', 'user')
       ->setDisplayConfigurable('view', TRUE)
       ->setDisplayConfigurable('form', TRUE);
 
@@ -415,18 +388,6 @@ class Store extends ContentEntityBase implements StoreInterface {
     return $fields;
   }
 
-  /**
-   * Default value callback for the 'uid' base field definition.
-   *
-   * @see ::baseFieldDefinitions()
-   *
-   * @return array
-   *   An array of default values.
-   */
-  public static function getCurrentUserId() {
-    return [\Drupal::currentUser()->id()];
-  }
-
   /**
    * Default value callback for the 'timezone' base field definition.
    *

modules/store/tests/src/Kernel/Entity/StoreTest.php

@@ -5,6 +5,7 @@ namespace Drupal\Tests\commerce_store\Kernel\Entity;
 use Drupal\commerce_price\Entity\Currency;
 use Drupal\commerce_store\Entity\Store;
 use Drupal\Tests\commerce\Kernel\CommerceKernelTestBase;
+use Drupal\user\UserInterface;
 
 /**
  * Tests the Store entity.
@@ -95,10 +96,18 @@ class StoreTest extends CommerceKernelTestBase {
     $this->assertEquals($this->user, $store->getOwner());
     $this->assertEquals($this->user->id(), $store->getOwnerId());
     $store->setOwnerId(0);
-    $this->assertEquals(NULL, $store->getOwner());
+    $this->assertInstanceOf(UserInterface::class, $store->getOwner());
+    $this->assertTrue($store->getOwner()->isAnonymous());
     $store->setOwnerId($this->user->id());
     $this->assertEquals($this->user, $store->getOwner());
     $this->assertEquals($this->user->id(), $store->getOwnerId());
+
+    // Ensure that we don't store a broken reference to the store owner.
+    $store->setOwnerId(900);
+    $this->assertTrue($store->getOwner()->isAnonymous());
+    $this->assertEqual($store->getOwnerId(), 900);
+    $store->save();
+    $this->assertEqual($store->getOwnerId(), 0);
   }
 
   /**

src/EntityOwnerTrait.php

+<?php
+
+namespace Drupal\commerce;
+
+use Drupal\user\Entity\User;
+use Drupal\user\EntityOwnerTrait as CoreEntityOwnerTrait;
+
+/**
+ * Provides a trait for Commerce entities that have an owner.
+ */
+trait EntityOwnerTrait {
+
+  use CoreEntityOwnerTrait;
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getOwner() {
+    $key = $this->getEntityType()->getKey('owner');
+    $owner = $this->get($key)->entity;
+    // Handle deleted customers.
+    if (!$owner) {
+      $owner = User::getAnonymousUser();
+    }
+    return $owner;
+  }
+
+}