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

Caching side effects #82

Closed
IDragonfire opened this issue May 16, 2017 · 11 comments
Closed

Caching side effects #82

IDragonfire opened this issue May 16, 2017 · 11 comments

Comments

@IDragonfire
Copy link
Contributor

The caching from @nicooga 44701c4 has some unexpected side effects ...

Example

You first request is Entity A
Your second request is Entity A again with some included relationships, e.g. SubEntityA
The cache is used for Entity A and SubEntityA is null, but it was send to the Client ...

@nicooga
Copy link

nicooga commented May 16, 2017

Im sorry to hear that. Can you expand?

The cache is used for Entity A and SubEntityA is null, but it was send to the Client ...

What do you mean by entity is null?
I suggest flushing the cache just before returning deserialized model to se wether that solves the issue

@IDragonfire
Copy link
Contributor Author

Flushing is a good solution...

The returned model of the second request don't contain sub entity a, because it was missing on the first request and in the cache

IDragonfire added a commit to IDragonfire/devour that referenced this issue May 16, 2017
@toadle
Copy link

toadle commented May 18, 2017

👍

I also got heavy side-effects and just had to downgrade to 1.4.1 to avoid them.
In my case I got a call to an endpoint that serves and included object like this:

{
  "data": {
    "id": "428",
    "type": "patients",
    "attributes": {
       ...
    },
    "relationships": {
      "leads": {
        "data": [
          {
            "id": "9",
            "type": "leads"
          }
        ]
      }
    }
  },
  "included": [
    {
      "id": "9",
      "type": "leads",
      "attributes": {
         ...
        },
      },
  ],
  "jsonapi": {
    "version": "1.0"
  }
}

In my case the lead of ID 9 has been loaded with devour before.
devour 1.4.2 will serve the lead with it's OLD attributes, not with the ones where lead would be included in a subsequent call.

@IDragonfire
Copy link
Contributor Author

IDragonfire commented May 18, 2017

@toadle does the pr #83 solves your problem?

@toadle
Copy link

toadle commented May 19, 2017

@IDragonfire Is there any easy way to test your branch in my project?
I tried just replacing the files in node_modules/devour-client but that doesn't seem to work.

@IDragonfire
Copy link
Contributor Author

@toadle you first need to run npm run compile and then copy the folder, lib should be enough ;)

@toadle
Copy link

toadle commented May 19, 2017

@IDragonfire OK, thanks for the advice. Your PR actually solves the cache problem, but introduces another that JSONAPI originally aimed to solve: It causes JSON-object-recursions.

Like this, which is a practical example from my app:

  "data": {
    "id": "8",
    "type": "authentications",
    "attributes": {
       ...
    },
    "relationships": {
      "user": {
        "data": {
          "id": "10",
          "type": "users"
        }
      }
    }
  },
  "included": [
    {
      "id": "10",
      "type": "users",
      "attributes": {
          ...
      },
      "relationships": {
        "current_authentication": {
          "data": {
            "id": "8",
            "type": "authentications"
          }
        }
      }
    }
  ],
  "jsonapi": {
    "version": "1.0"
  }
}

Will result in an endless: authentication.user.current_authentication.user.current_authentication.... object.

I suppose it should stop at the point, where in my example the current_authentication of the user is not included anymore. Therefore should be empty.

@IDragonfire
Copy link
Contributor Author

@toadle Oh, you are correct, I created an issue for this #84 but it depend on the include parameter ...
If you pull only 614e62f does this solve the endless object?

@IDragonfire IDragonfire mentioned this issue May 19, 2017
@toadle
Copy link

toadle commented May 19, 2017

@IDragonfire Do you still need me to test the above commit or was #85 enough?

@IDragonfire
Copy link
Contributor Author

IDragonfire commented May 19, 2017

The commit above is #85 ;)
I decided after my comment above to close the PR and create an new one ^^

Issue here

image

Pr #85

image

@IDragonfire
Copy link
Contributor Author

Fixed by #85

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants