Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assertion Failed exceptions: Cannot remove a resource that is not present #8990

Open
eodb opened this issue Oct 10, 2023 · 3 comments
Open
Labels
🏷️ bug This PR primarily fixes a reported issue Needs Bug Verification

Comments

@eodb
Copy link

eodb commented Oct 10, 2023

Reproduction

It took some time to find a reproducible qunit test but here it is:

import { run } from '@ember/runloop';
import Model, { attr, belongsTo, hasMany } from '@ember-data/model';
import { setupTest } from 'ember-qunit';
import { module, test } from 'qunit';

let store;

module('integration/relationship/clear hasMany relation twice', function (hooks) {
  setupTest(hooks);

  hooks.beforeEach(function () {
    class DocPage extends Model {
      @hasMany('entry', { inverse: 'docPageId', async: false }) entries;
    }

    class Entry extends Model {
      @attr state;
      @belongsTo('doc-page', { async: true, inverse: 'entries' }) docPageId;
      @belongsTo('page-assembly', { async: false, inverse: 'entries' }) pageAssembly;
    }

    class PageAssembly extends Model {
      @hasMany('entry', { async: false, inverse: 'pageAssembly' }) entries;
    }

    store = this.owner.lookup('service:store');

    this.owner.register('model:doc-page', DocPage);
    this.owner.register('model:entry', Entry);
    this.owner.register('model:page-assembly', PageAssembly);
  });

  const hashDocPage = {
    data: [
      {
        id: '0',
        type: 'doc-page',
      },
    ],
  };

  const hashEntries = {
    data: [
      {
        id: '0',
        type: 'page-assembly',
        relationships: { entries: { data: [{ id: '0', type: 'entry' }] } },
      },
    ],
    included: [
      {
        id: '0',
        type: 'entry',
        attributes: { state: 'CREATED' },
        relationships: {
          docPageId: { data: { id: '0', type: 'doc-page' } },
        },
      },
    ],
  };

  const hashClearDocPageEntries = {
    data: [
      {
        id: '0',
        type: 'page-assembly',
        relationships: { entries: { data: [{ id: '0', type: 'entry' }] } },
      },
    ],
    included: [
      {
        id: '0',
        type: 'entry',
        attributes: { state: 'CLEARED' },
        relationships: {
          docPageId: { data: null },
        },
      },
    ],
  };

  async function pushDocPage() {
    let docPages;
    await run(() => {
      docPages = store.push(hashDocPage);
    });
    return docPages;
  }

  async function pushEntries() {
    let pageAssemblies;
    await run(() => {
      pageAssemblies = store.push(JSON.parse(JSON.stringify(hashEntries)));
    });
    return pageAssemblies;
  }

  async function pushClearDocPageEntries() {
    let pageAssemblies;
    await run(() => {
      pageAssemblies = store.push(JSON.parse(JSON.stringify(hashClearDocPageEntries)));
    });
    return pageAssemblies;
  }

  test('pushEntries', async function (assert) {
    let docPages = await pushDocPage();

    // 1st iteration to create relationship entry->docPage with 'pushEntries()' and to clear it with 'pushClearDocPageEntries()'
    let pageAssemblies = await pushEntries();

    assert.equal(store.peekAll('page-assembly').length, 1);
    assert.equal(store.peekAll('entry').length, 1);
    assert.equal(store.peekAll('doc-page').length, 1);
    assert.equal(pageAssemblies[0].entries.length, 1);

    let docPage = await pageAssemblies[0].entries[0].docPageId;
    assert.equal(docPage.entries.length, 1, 'docPage linked with entry');

    pageAssemblies = await pushClearDocPageEntries();
    assert.equal(docPage.entries.length, 0, 'docPage cleared');

    // 2nd iteration to create relationship entry->docPage with 'pushEntries()' and to clear it with 'pushClearDocPageEntries()'
    pageAssemblies = await pushEntries();
    assert.equal(pageAssemblies[0].entries.length, 1);
    docPage = await pageAssemblies[0].entries[0].docPageId;

    // NOTE: UNCOMMENT THE LINE BELOW: i.e. simply accessing the "docPage.entries" relationship once more and THE TEST SUCCEEDS!
    // assert.equal(docPage.entries.length, 1, 'docPage linked with entry');

    pageAssemblies = await pushClearDocPageEntries();
    assert.equal(docPage.entries.length, 0, 'docPage cleared 2nd time');
  });
});

Description

Populate a "belongsTo" relationship with an inverse "hasMany", and clear it afterwards. This works fine the first time. (see 1st iteration in comment)
The second time however, running the same scenario by submitting the same payload(s), an assertion error occurs: "Cannot remove a resource that is not present". (see 2nd iteration in comment)

Expected: the same result the 2nd time the payloads are submitted.

Unexpected is also that when the "docPage.entries" relationship is simply accessed by reading the property, the test succeeds while this should have no effect. (to reproduce, uncomment the line after "NOTE: UNCOMMENT THE LINE BELOW..." in the test above)

Debugging the issue learned the following:

In the 2nd iteration of the scenario above, the docPage.entries relationship.localState = []. This causes the assert in the 2nd scenario when clearing the belongsTo relationship of the entry.docPage by updating the reverse relationship of docPage.entries in '_removeRemote', called from 'removeFromInverse' because localState is not null, but an empty array:

function _removeRemote(relationship, value) {
  ....
  // if we have existing localState
  // and we have an index
  // apply the change, as this is more efficient
  // than recomputing localState and
  // it allows us to preserve local ordering
  // to a small extend. Local ordering should not
  // be relied upon as any remote change will blow it away
  if (relationship.localState) {
    index = relationship.localState.indexOf(value);
    assert(`Cannot remove a resource that is not present`, index !== -1);
    relationship.localState.splice(index, 1);
  }
....
}

By reading the "docPage.entries" relationship, before the belongsTo relationship of the entry.docPage is cleared (UNCOMMENT THE LINE - in the test above), the docPage.entries relationship.localState is not empty and does contain the identifier, which can then be successfully removed when clearing the entry.docPage belongsTo relationship, passing the test.

Versions

Works fine in v5.2.0.
Fails from v5.3.0

@runspired
Copy link
Contributor

separate from whether there is a bug here, the payload setup here isn't describing the doc-page entries relationship in a valid way. The invalid setup likely contributes to the observed issue by putting the relationship into an invalid state, from which point the later behaviors become less well defined.

@runspired runspired moved this from needs triage to Needs Planning in EmberData Oct 10, 2023
@runspired runspired moved this from Needs Planning to needs champion in EmberData Oct 10, 2023
@runspired runspired added 🏷️ bug This PR primarily fixes a reported issue Needs Bug Verification labels Oct 10, 2023
@eodb
Copy link
Author

eodb commented Apr 5, 2024

Hi @runspired,
Finally found the time to look into this again (our major release is out..). I couldn't figure our immediately what was wrong in the setup, so instead I tried to extend one of the exiting tests from the ember-data testsuite since the setup was almost similar.
I've made a PR (#9326) that will make it clear I guess. Repeating the scenario twice (i.e. setting the belongsTo relation to null) generates the same error.
Hope this helps to figure this one out.

@hoIIer
Copy link

hoIIer commented Sep 7, 2024

Getting same error in an acceptance test after upgrading.

Error: Cannot remove a resource that is not present
    at eval (webpack://__ember_auto_import__/./node_modules/ember-data/node_modules/@ember-data/graph/dist/-private.js?:374:633)
    at _removeRemote (webpack://__ember_auto_import__/./node_modules/ember-data/node_modules/@ember-data/graph/dist/-private.js?:374:694)
    at removeFromInverse (webpack://__ember_auto_import__/./node_modules/ember-data/node_modules/@ember-data/graph/dist/-private.js?:298:1355)
    at addToInverse (webpack://__ember_auto_import__/./node_modules/ember-data/node_modules/@ember-data/graph/dist/-private.js?:294:1667)
    at eval (webpack://__ember_auto_import__/./node_modules/ember-data/node_modules/@ember-data/graph/dist/-private.js?:275:120)
    at _compare (webpack://__ember_auto_import__/./node_modules/ember-data/node_modules/@ember-data/graph/dist/-private.js?:358:96)
    at diffCollection (webpack://__ember_auto_import__/./node_modules/ember-data/node_modules/@ember-data/graph/dist/-private.js?:361:47)
    at replaceRelatedRecordsRemote (webpack://__ember_auto_import__/./node_modules/ember-data/node_modules/@ember-data/graph/dist/-private.js?:272:79)
    at replaceRelatedRecords (webpack://__ember_auto_import__/./node_modules/ember-data/node_modules/@ember-data/graph/dist/-private.js?:260:68)
    at Graph.update (webpack://__ember_auto_import__/./node_modules/ember-data/node_modules/@ember-data/graph/dist/-private.js?:498:235)

ember-data: 5.3.8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bug This PR primarily fixes a reported issue Needs Bug Verification
Projects
Status: needs champion
Development

No branches or pull requests

3 participants