Mutators appear to be broken. Works on initial insert/save but not on updates

[vesper8]

Environment

No response

Reproduction

Please see description below

Describe the bug

I've created a very simple mutator that encrypts my string. The method of encryption itself does not matter in the context of this bug report.

  static mutators() {
    return {
      content: {
        set: (value) => {
          const encryptedValue = encryptData(value);
          console.log('content.set', value, encryptedValue);
          return encryptedValue;
        },
      },
    };
  }

When I create a new record:

          await useRepo(Requirement).save({
            id: 1,
            content: this.form.content,
          });

Then everything works fine. I can trace that my set mutator is being visited, the plainText is encrypted, and if I view my content with

        const requirement = useRepo(Requirement).find(1);

        console.log('requirement.content', requirement.content);

Then the value is indeed encrypted and life is good.

If I then update this existing requirement

Whether like this:

          await useRepo(Requirement)
            .find(1)
            .update({
              content: this.form.content,
            });

Or like this

          await useRepo(Requirement).save({
            id: 1,
            content: this.form.content,
          });

Then again I can trace that my set mutator is being visited. The set mutator does the encryption, however unlike when I created a new record, this time if I inspect the value with

        const requirement = useRepo(Requirement).find(1);

        console.log('requirement.content', requirement.content);

This time the value is plainText. It is not the encrypted value. So the mutator is "used" but somehow not applied on updates, only on inserts.

Additional context

No response

Logs

No response

[vesper8]

The same behaviour occurs if using a custom cast. It works on the initial insert, but fails on any subsequent updates

EncryptedCast.js

import { CastAttribute } from 'pinia-orm';

import { encryptData, decryptData } from '@/services/encryptionService';

export class EncryptedCast extends CastAttribute {
  constructor(attributes) {
    super(attributes);
  }

  get(value) {
    const decryptedValue = decryptData(value);
    console.log('content.get', value, decryptedValue);
    return decryptedValue;
  }

  set(value) {
    const encryptedValue = encryptData(value);
    console.log('content.set', value, encryptedValue);
    return encryptedValue;
  }
}

And then inside my model file:

Requirement.js

import { Model } from 'pinia-orm';

import { EncryptedCast } from '@/pinia/orm/casts/EncryptedCast';

export default class extends Model {
  static entity = 'requirement';

  static fields() {
    return {
      id: this.attr(null),
      content: this.string(''),
    };
  }

  static casts() {
    return {
      content: EncryptedCast,
    };
  }
}

[vesper8]

Ok I think I'm losing my mind here...

I tried to get this working another way by using the hooks:

  static creating(model, record) {
    model.content = encryptData(model.content);
  }
  static updating(model, record) {
    model.content = encryptData(model.content);
  }

And guess what... same behaviour! Correctly encrypts my string when I'm saving/inserting a new record. But when I'm updating an existing record (immediately after saving) then in spite of going inside the hook, it doesn't work and it saves the plaintext value instead.

What the heck am I missing? @CodeDredd please? Any idea?

[CodeDredd]

You are right. The hydration cache is not correctly updating for updates. That's the reason why all this is not working. As a workaround you could have done this before every update -> useRepo(Model).hydratedDataCache.clear().

Fix is on the way. You can test it out with the new edge release if you like. 😉(Posting it here after merge) Release will be later

[CodeDredd]

:rocket: Release triggered! You can now install pinia-orm@npm:pinia-orm-edge@1.9.0-28580088.76eb4a5

[vesper8]

@CodeDredd Thanks! Really so happy that you're back at work on this. Looking forward to 1.9.0 stable!

For now I tried installing the edge releases:

"@pinia-orm/axios-edge": "latest",

"pinia-orm-edge": "latest",

But now I'm getting inundated with these errors when I try to start my project

1:43:17 PM [vite] Pre-transform error: Failed to resolve import "pinia-orm" from "src/pinia/orm/models/User.js". Does the file exist?

Is that because I would have to replace all my imports from pinia-orm to pinia-orm-edge ? That's hundreds of replacements.. my Git history wouldn't love that..

[CodeDredd]

@vesper8 Don't take the edge directly for testing. Here is way how to do it....i added it also to the docs. Just need to rebuild the docs again.

Update pinia-orm dependency inside package.json:

{
  "devDependencies": {
-   "pinia-orm": "^1.0.0"
+   "pinia-orm": "npm:pinia-orm-edge@latest"
  }
}

Remove lockfile (package-lock.json, yarn.lock, or pnpm-lock.yaml) and reinstall dependencies.

Opting out from the edge channel

Update pinia-orm dependency inside package.json:

{
  "devDependencies": {
-   "pinia-orm": "npm:pinia-orm-edge@latest"
+   "pinia-orm": "^1.0.0"
  }
}

[vesper8]

Thank you! It works

"@pinia-orm/axios": "npm:@pinia-orm/axios-edge@latest",
"pinia-orm": "npm:pinia-orm-edge@latest",

Going to do some testing now see if this introduced any regressions. I can confirm the problem raised in this issue is now working though so that's great.

[vesper8]

@CodeDredd it seems this issue may not be entirely solved yet.

In trying to decide how to approach my specific problem (which I'd appreciate your advice on, see below) I may have uncovered another issue.

I'm trying to make pinia-orm handle my encryption/decryption of specific fields on specific models so that the value inside pinia-orm is always stored encrypted, but is automatically decrypted when displayed on the web app.

Another very important point is that whenever this data is persisted to the backend, it should always transfer the encrypted values.

I'm using a custom cast to achieve this:

import { CastAttribute } from 'pinia-orm';

import { encryptData, decryptData } from '@/services/encryptionService';

export class EncryptedCast extends CastAttribute {
  constructor(attributes) {
    super(attributes);
  }

  get(value) {
    return decryptData(value);
  }

  set(value) {
    return encryptData(value);
  }
}

This works on create, but it makes it very hard for me to A) confirm that the data is stored encrypted and B) access the encrypted value. Is there a way to circumvent the getter here if I want to access the raw value, for example when it's time to persist to the backend using the pinia-orm axios plugin?

This also exposes the remaining issue.. it seems to be the same issue as you fixed with the setter.. but it's also happening with the getter. The getter will return the decrypted value correctly on create, but on update it just seems to ignore the getter and returns the encrypted value instead.

This led me to experiment with a different approach:

import { CastAttribute } from 'pinia-orm';

import { encryptData, decryptData } from '@/services/encryptionService';

export class EncryptedCast extends CastAttribute {
  constructor(attributes) {
    super(attributes);
  }

  get(value) {
    return {
      encrypted: value,
      decrypted: decryptData(value),
    };
  }

  set(value) {
    return encryptData(value);
  }
}

But in this case the same issue occurs and the getter only returns the object format { "encrypted": "3Xelj+wXrRlrDjaRjf0ewNoOFYtETHK46gPjEnOTs8E=", "decrypted": "test" } on the initial create and not after an update.

[CodeDredd]

@vesper8 You are again right. There was a design flaw in the code. Finding here a solution was really hard. I had to make a design change to mutators/casts to get it working. The downside that the performance got a bit slower if the props have mutators/casts. But it should be still good.

You can test it for me with that version: pinia-orm-edge@1.9.0-28580767.64b3b57

[vesper8]

Hey @CodeDredd. Thanks for staying on top of this!

I can confirm that 1.9.0-28580767.64b3b57 introduces a new problem. It appears to be going through the getter twice now.

So in this example (Custom Cast)

  get(value) {
    return value + ' (decrypted)';
  }

  set(value) {
    return value + ' (encrypted)';
  }

After doing an initial insert with the string "test", you would expect the value to be test (encrypted) (decrypted)

But instead you get test (encrypted) (decrypted) (decrypted)

In my previous example, with a encrypting/decrypting method

  get(value) {
    return decryptData(value);
  }

  set(value) {
    return encryptData(value);
  }

Well I'm just getting an empty value every time because it correctly encrypts on set, and then it tries to decrypt twice, the 2nd time fails and returns empty.

I guess you're not catching this because you're using toLowerCase/toUpperCase in your tests.

[CodeDredd]

@vesper8 Thanks for the testing....you are getting what i was hoping you to get 😆 ... i know it sounds strange. But i hoped that my unit test was wrong because i had added some small hack which in my mind hadn't made sense.

Thanks for your great feedback 🎉

[CodeDredd]

@vesper8 Ok next try. Hoefully it works now as intended. From my side it looks fine.

[vesper8]

we're real close but this one is persistent!

for me the setter doesn't work at all on updates right now.

With this really simplified example:

  set(value) {
    return value + ' (modified)';
  }

no getter needed here.. on create the content is affected and returns 'test (modified)', but on update the setter doesn't apply at all and the value is just 'test'

I checked it many times I'm pretty sure I'm not misusing it

[CodeDredd]

@vesper8 this bug is annoying....but well now i have great hope that it's fixed. You can try out the new patch.

And again thanks for testing.

[vesper8]

It appears to be working correctly now! Thanks for addressing this quickly!

Now that this is working. I was wondering if you could share your thoughts on what I'm trying to achieve and what features of pinia-orm would be used for my use case.

I'm trying to make pinia-orm handle my encryption/decryption of specific fields on specific models so that the value inside pinia-orm is always stored encrypted, but is automatically decrypted when displayed on the web app.

Another very important point is that whenever this data is persisted to the backend, it should always transfer the encrypted values.

Currently, using a CustomCast achieves the automatic encryption/decryption, but it makes it hard for me to A) confirm that the data is really stored encrypted and B) access the encrypted values themselves for when it's time to pass them to the Axios plugin.

Is there a way to access the "raw" value of a field by skipping any getter? Am I maybe missing something that would make my use-case work well with pinia-orm?

Many thanks!!

[CodeDredd]

I see. Well you can always check the values by accessing the store. So you can always check if it is encrypted by using useRepo(MODEL).piniaStore.$state.data. Here the data should be encrypted. And if you now the id you can easly search for the record and the data.

Once the model is hydrated where is no way to check the original data passed to it.

For transfering it to the backend i would use some param in your Decryption cast with that you could skip decryption. This param can be then some global State param which is defined in the model.

Then in a custom Repository i would write an function for the transfer to the backend, where i can use a normall pinia-orm query and if needed clearing the hydration cache before querying.

[vesper8]

@CodeDredd Thank you for your suggestions.. I'm trying this out and first there was a small typo in useRepo(MODEL).piniaStore.$state.data, it needs to be useRepo(MODEL).piniaStore().$state.data

This does in fact show the encrypted data so that's good. Is it possible to query this raw data with any of the usual methods? I guess not?

I would find it very useful if there was a helper that returned the raw values without the getters. I thought $toJson() might work but it doesn't return the raw values. Maybe a $toRawValues or something could be added for this purpose?

I think I may have uncovered another issue. Let me know if you think this is wrong or not.

Right now if I return an object in my getter:

  get(value) {
    return {
      encrypted: value,
      decrypted: decryptData(value),
    };
  }

It will throw this warning Requirement.js:23 [Pinia ORM] Field requirement:content - [object Object] is not a string

My content field is set like so content: this.string(''),

And then in my casts

  static casts() {
    return {
      content: EncryptedCast,
    };
  }

I kind of feel like this warning is not correct? Since the actual value is a string.. the setter sets just the encrypted string

  set(value) {
    return encryptData(value);
  }

So this warning is wrong? Or is it? What do you think?

Thanks!!

[CodeDredd]

Well adding a $toRawJson() would mean that i need to save the data in the model twice. Once with casts and mutators and another time without. That's to much overhead. The warning might be wrong. Normally it isn't. But maybe you found an edge case with casts/mutators. before a value is set in a model it checks the type for specific type fields like this.string().

[vesper8]

@CodeDredd Yea I understand how saving it twice wouldn't make sense. Thanks for clarifying.

And yes it looks like I did find an edge case. Do you think this is worth "fixing" or I should just set my content to this.attr(null) instead of this.string('') ?

Lastly.. it seems that I've come across a new problem (not a bug from pinia-orm) with my approach.

You see when I'm creating or updating my record on the frontend, it's implied that plainText is being passed in, and through the setter it's being encrypted.

The problem is.. when I load data from my backend, the API returns already-encrypted data, and now the setter from my custom cast is trying to encrypt already encrypted data.

Which makes me doubt that my approach of using getters and setters may not be the best one. As you explained with a global flag inside the custom cast I could turn off automatic encryption whenever I'm loading data from the backend, and then turn it on again. But this seems messy and unintuitive.

Whenever I'm loading data from the backend, I always use the pinia-orm axios plugin. Would it be possible to somehow instruct the custom cast not to run the setter whenever the source of the data is the axios plugin?

[CodeDredd]

And yes it looks like I did find an edge case. Do you think this is worth "fixing" or I should just set my content to this.attr(null) instead of this.string('') ?

I checked my tests with casts and mutators and no warning. Maybe the warning is right and your not saving a string but an object, which is later serlized to an string. You should check that.

Whenever I'm loading data from the backend, I always use the pinia-orm axios plugin. Would it be possible to somehow instruct the custom cast not to run the setter whenever the source of the data is the axios plugin?

Well thats the approch with an veriable...i think its intutive if your casts knows if the data what it gets is already encrtypted. Either by passing a meta prop to the data or your decryption functions returns an error and you can use it to check with it if it's encrypted.

[vesper8]

I'm definitely saving a string. I save a string, then the setter encrypts that string but it's still a string, and the getter is the one that returns an object. If I comment out the getter I don't get the warning. With the getter returning an object I get the warning.

For the rest, I'll think about what you said.. I think checking if the incoming data is already encrypted by trying to decrypt it might lead to bad performance.. but maybe it won't really matter. Thanks for chiming in.

[vesper8]

Hey @CodeDredd : )

Sorry to bump this issue again. I just found something else. Maybe it's intended.. you tell me.

I just noticed that when I load related models, the getter is not being used for those.

Remember I have a getter that auto-decrypts my values. This works well if I query the model repository directly.

But if I retrieve that model through a parent model then the getter is not used.

So here it works well

const requirements = useRepo(Requirement).all()

console.log(requirements); // here the cast is applied and the getter is used

but here it doesn't

const architectureView = useRepo(ArchitectureView) //
  .with('requirements')
  .where('id', this.currentArchitectureView.id)
  .first();

console.log(architectureView.requirements); // here the cast is not applied and the getter is not used

Is this intended? Is there a way to circumvent that so the getter is always used?

Thanks!

[vesper8]

@CodeDredd Hey again. Sorry if I pinged you on a Friday evening above. If you have a chance to look at my latest comment I'd appreciate a reply to understand if this is an issue that will be fixed or if I should develop a workaround to deal with it. Thanks!

[CodeDredd]

Hey @vesper8, no problem ;-) .... i think its an issue. Sound to me like that. Making a unit test to look more into it. Thanks for the post 👍