Skip to content

NestedSet::isNode($node) returns false negative #538

Open
@nagmat84

Description

@nagmat84

I have implemented a Eloquent class which uses the trait NodeTrait. Deleting objects of such a class fails with an InvalidArgumentException exception "Model must be node." in BaseRelation::__construct() due to NestedSet::isNode returning false.

The call stack in reverse order is

BaseRelation.php:40, Kalnoy\Nestedset\DescendantsRelation->__construct()
NodeTrait.php:252, App\Models\Album->descendants()
NodeTrait.php:631, App\Models\Album->deleteDescendants()
NodeTrait.php:56, App\Models\Album::Kalnoy\Nestedset\{closure:/var/www/localhost/lychee/vendor/kalnoy/nestedset/src/NodeTrait.php:55-57}()
Dispatcher.php:392, Illuminate\Events\Dispatcher->Illuminate\Events\{closure:/var/www/localhost/lychee/vendor/laravel/framework/src/Illuminate/Events/Dispatcher.php:387-393}()
Dispatcher.php:237, Illuminate\Events\Dispatcher->dispatch()
HasEvents.php:189, App\Models\Album->fireModelEvent()
Model.php:1158, App\Models\Album->delete()

(Note: App\Models\Album in the last line is my own model).

The Album is defined as follows

class Album extends BaseAlbum {
  use NodeTrait;
  ...
}

and BaseAlbum is

abstract class BaseAlbum extends Model implements AbstractAlbum {
  ...
}

NestedSet::isNode coverts the object to an array in order to check whether the object uses the trait NodeTrait. The array conversion returns

array (
  'attributes' => array (
    'id' => '16356901022810',
    'parent_id' => NULL,
    'license' => 'reserved',
    'cover_id' => NULL,
    '_lft' => '21',
    '_rgt' => '24',
    'min_taken_at' => NULL,
    'max_taken_at' => NULL,
  ),
  'casts' => array (
    'min_taken_at' => 'datetime',
    'max_taken_at' => 'datetime',
    'cover_id' => 'integer',
    'parent_id' => 'integer',
    '_lft' => 'integer',
    '_rgt' => 'integer',
  ),
  'hidden' => array (
    0 => 'base_class',
    1 => 'cover',
    2 => '_lft',
    3 => '_rgt',
    4 => 'parent',
    5 => 'all_photos',
    6 => 'base_class',
  ),
  'with' => array (
    0 => 'cover',
    1 => 'thumb',
    2 => 'base_class',
  ),
  'incrementing' => false,
  'connection' => 'sqlite',
  'table' => 'albums',
  'primaryKey' => 'id',
  'keyType' => 'int',
  'withCount' => array (),
  'perPage' => 15,
  'exists' => true,
  'wasRecentlyCreated' => false,
  'original' => 
  array (
    'id' => '16356901022810',
    'parent_id' => NULL,
    'license' => 'reserved',
    'cover_id' => NULL,
    '_lft' => '21',
    '_rgt' => '24',
    'min_taken_at' => NULL,
    'max_taken_at' => NULL,
  ),
  'changes' => array (),
  'classCastCache' => array (),
  'dates' => array (),
  'dateFormat' => NULL,
  'appends' => array (),
  'dispatchesEvents' => array (),
  'observables' => array (),
  'relations' => array (
    'cover' => NULL,
    'thumb' => NULL,
    'base_class' => App\Models\BaseAlbumImpl::__set_state(array(
      'table' => 'base_albums',
      'incrementing' => false,
      'attributes' => array (
        'id' => '16356901022810',
        'created_at' => '2021-10-31 14:21:42',
        'updated_at' => '2021-10-31 14:21:44',
        'title' => 'NEW_TEST',
        'description' => 'new description',
        'owner_id' => '0',
        'is_public' => '0',
        'grants_full_photo' => '1',
        'requires_link' => '0',
        'is_downloadable' => '0',
        'is_share_button_visible' => '0',
        'is_nsfw' => '0',
        'password' => NULL,
        'sorting_col' => NULL,
        'sorting_order' => 'ASC',
      ),
      'casts' => array (
        'created_at' => 'datetime',
        'updated_at' => 'datetime',
        'is_public' => 'boolean',
        'requires_link' => 'boolean',
        'is_nsfw' => 'boolean',
        'owner_id' => 'integer',
        'id' => 'integer',
      ),
      'hidden' => array (
        0 => 'owner',
        1 => 'password',
      ),
      'appends' => array ( 0 => 'has_password' ),
      'with' => array ( 0 => 'owner' ),
      'connection' => 'sqlite',
      'primaryKey' => 'id',
      'keyType' => 'int',
      'withCount' => array (),
      'perPage' => 15,
      'exists' => true,
      'wasRecentlyCreated' => false,
      'original' => array (
        'id' => '16356901022810',
        'created_at' => '2021-10-31 14:21:42',
        'updated_at' => '2021-10-31 14:21:44',
        'title' => 'NEW_TEST',
        'description' => 'new description',
        'owner_id' => '0',
        'is_public' => '0',
        'grants_full_photo' => '1',
        'requires_link' => '0',
        'is_downloadable' => '0',
        'is_share_button_visible' => '0',
        'is_nsfw' => '0',
        'password' => NULL,
        'sorting_col' => NULL,
        'sorting_order' => 'ASC',
      ),
      'changes' => array (),
      'classCastCache' => array (),
      'dates' => array (),
      'dateFormat' => NULL,
      'dispatchesEvents' => array (),
      'observables' => array (),
      'relations' => array (
        'owner' => App\Models\User::__set_state(array(
          'fillable' => array (
            0 => 'username',
            1 => 'password',
            2 => 'email',
          ),
          'hidden' => array (
            0 => 'password',
            1 => 'remember_token',
            2 => 'created_at',
            3 => 'updated_at',
          ),
          'casts' => array (
            'upload' => 'int',
            'lock' => 'int',
          ),
          'connection' => 'sqlite',
          'table' => 'users',
          'primaryKey' => 'id',
          'keyType' => 'int',
          'incrementing' => true,
          'with' => array (),
          'withCount' => array (),
          'perPage' => 15,
          'exists' => true,
          'wasRecentlyCreated' => false,
          'attributes' => array (
            'id' => '0',
            'username' => '',
            'password' => '',
            'upload' => '0',
            'lock' => '0',
            'remember_token' => NULL,
            'created_at' => '2021-10-31 13:05:22',
            'updated_at' => '2021-10-31 13:05:22',
            'email' => NULL,
          ),
          'original' => array (
            'id' => '0',
            'username' => '',
            'password' => '',
            'upload' => '0',
            'lock' => '0',
            'remember_token' => NULL,
            'created_at' => '2021-10-31 13:05:22',
            'updated_at' => '2021-10-31 13:05:22',
            'email' => NULL,
          ),
          'changes' => array (),
          'classCastCache' => array (),
          'dates' => array (),
          'dateFormat' => NULL,
          'appends' => array (),
          'dispatchesEvents' => array (),
          'observables' => array (),
          'relations' => array (),
          'touches' => array (),
          'timestamps' => true,
          'visible' => array (),
          'guarded' => array ( 0 => '*' ),
          'rememberTokenName' => 'remember_token',
        )),
      ),
      'touches' => array (),
      'timestamps' => true,
      'visible' => array (),
      'fillable' => array (),
      'guarded' => array ( 0 => '*' ),
    )),
  ),
  'touches' => array (),
  'timestamps' => false,
  'visible' => array (),
  'fillable' => array (),
  'guarded' => array ( 0 => '*' ),
  'pending' => NULL,
  'moved' => false,
)

(As you can see, the array does indeed not report the trait NodeTrait).

As I know for sure that Album is a node everything just works fine if I patch NestedSet::isNode to skip the buggy check. I haven't yet figured out under what conditions the construction (array)some_object returns or does not return the used traits. However, using a conversion to an array is not a reliable method.

Half a solution

There is the SPL method class_uses which perfectly works fine for me in all cases, but it is not a general solution, because it does not return traits of parent classes. This means, for another model hierarchy which may use NodeTrait on a common base class for several models, class_uses will also fail.

Proposed solution

Create an interface Node which declares all the necessary methods and let NodeTrait provide the implementation of these methods. Then check with some_object instanceof Node if the model is a node. This is the only reliable method.

Drawback: All users of the library are enforced to make their models implement the said interface. But this is the purpose of interfaces (aka "contracts").

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions