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

Virtual Getter not working without returning the dependent column #7237

Open
xLarry opened this issue Feb 11, 2017 · 17 comments
Open

Virtual Getter not working without returning the dependent column #7237

xLarry opened this issue Feb 11, 2017 · 17 comments
Labels
topic: virtual fields For issues and PRs. Things that involve virtual fields. type: feature For issues and PRs. For new features. Never breaking changes.

Comments

@xLarry
Copy link

xLarry commented Feb 11, 2017

What you are doing?

I want to retrieve a computed column ("cipher") which is the encrypted value of another column ("secret"). Unfortunately, I am not able to exclude the "secret" column from the result set without breaking the computation of "cipher".

var Sequelize = require('sequelize')

var sequelize = new Sequelize('sqlite://', {storage: 'test.sqlite'})

var User = sequelize.define('User', 
  { 
    name: Sequelize.TEXT,
    secret: Sequelize.TEXT

    // Alternative: define cipher2 as a VIRTUAL column including 'secret' as dependent field
    // --> exclude: ['secret'] doesn't have any effect at all

    // , cipher2: {
    //   type: new Sequelize.VIRTUAL(Sequelize.TEXT, ['secret']),
    //   get: function() {
    //     return this.getDataValue('cipher');
    //   }
    // }

  },
  {
    getterMethods: {
      cipher: function(){ 
        var secret = this.getDataValue('secret');
        var encrypted = "Encrypted value of " + secret;

        return encrypted; 
      }
    }
});

sequelize.sync({ force: true }).then(function(){

    User.bulkCreate([{ name: 'Hans', secret: 'Uschi123' },{ name: 'Franz', secret: 'Gudrun456' }]).then(function(){

        User.find({ 
          attributes: {
            exclude: ['secret'] 
          }
        })
        .then(function(user){
            console.log(JSON.stringify(user,null,'  '));
        });

    })

})

What do you expect to happen?

Expected output is as follows:

{
  "cipher": "Encrypted value of Uschi123",
  "id": 1,
  "name": "Hans",
  "createdAt": "2017-02-11T16:52:39.687Z",
  "updatedAt": "2017-02-11T16:52:39.687Z"
}

What is actually happening?

{
  "cipher": "Encrypted value of undefined",
  "id": 1,
  "name": "Hans",
  "createdAt": "2017-02-11T16:52:39.687Z",
  "updatedAt": "2017-02-11T16:52:39.687Z"
}

If you comment out line 39 (exclude: ['secret']) the output is as follows:

{
  "cipher": "Encrypted value of Uschi123",
  "id": 1,
  "name": "Hans",
  "secret": "Uschi123",
  "createdAt": "2017-02-11T16:56:51.213Z",
  "updatedAt": "2017-02-11T16:56:51.213Z"
}

Dialect: sqlite
Database version: 3.1.8
Sequelize version: 3.30.2

@stale stale bot added the stale label Jun 29, 2017
@stale stale bot closed this as completed Jul 7, 2017
@1tayH
Copy link

1tayH commented Feb 12, 2018

Did you find any solution for this?

@xLarry
Copy link
Author

xLarry commented Feb 12, 2018

Don't have much time to answer but I guess the best solution was to define dependency fields when defining the virtual column (createdAt in the example below):

{
  active: {
    type: new DataTypes.VIRTUAL(DataTypes.BOOLEAN, ['createdAt']),
    get: function() {
      return this.get('createdAt') > Date.now() - (7 * 24 * 60 * 60 * 1000)
    }
  }
}

http://docs.sequelizejs.com/variable/index.html#static-variable-DataTypes

@1tayH
Copy link

1tayH commented Feb 13, 2018

@xLarry Thanks for the reply, I tried using it, however it pulls the "secret" column (email in my case) in addition, which is not something I'm interesting in exposing to my customers.

For example, here's my model definition:

module.exports = function(sequelize, DataTypes) {
  const User = sequelize.define(
    'User',
    {
      email: {
        type: DataTypes.STRING,
        allowNull: false,
        unique: true
      },
      avatar: {
        type: new DataTypes.VIRTUAL(DataTypes.STRING, ['email']),
        get() {
          return this.getAvatarUrl()
        }
      }
    },
    {
      underscored: true
    }
  )

  return User
}

When I try to get the avatar attribute, it also adds the "email" attribute, which is something that should stay private:

    let query = {
      where: {
        user_id: 1
      },
      attributes: ['id', 'body', 'subject', 'created_at'],
      include: [
        {
          model: User,
          attributes: ['avatar']
        }
      ]
    }
    return Ticket.findAndCountAll(query)
{
    "count": 2,
    "rows": [
        {
            "id": 2,
            "body": "body",
            "subject": "subject",
            "created_at": "2018-02-13T08:21:35.574Z",
            "User": {
                "avatar": "https://s.gravatar.com/avatar/5e65c423b791c5437a54b2567eff09f9?s=100&r=x&d=identicon",
                "email": "..."
            }
        },
        {
            "id": 1,
            "body": "test",
            "subject": "test",
            "created_at": "2018-02-12T19:52:26.362Z",
            "User": {
                "avatar": "https://s.gravatar.com/avatar/d2891204815cd41f8dc1be389ba23ea9?s=100&r=x&d=identicon",
                "email": "..."
            }
        }
    ]
}

I could map through the rows and delete the email key, before sending the response to the user, however it feels very nasty doing it every time.

@xLarry
Copy link
Author

xLarry commented Feb 13, 2018

Yeah, right. That's what I had to do as well and it didn't feel correct too.

As I am writing this I am thinking about creating a custom getter method that sets the secret column to null. Not nice as well but just wanted to mention another approach that might do the trick as a workaround.

@Danita
Copy link

Danita commented Aug 7, 2020

Can we reopen this? Is there a workaround? I have the same question.

@barelyAWesome
Copy link

@Danita agreed - having this issue as well. Would really not like to expose the dependent column if not needed.

@ErtugrulBEKTIK
Copy link

I have this issue too and I found a tricky way to solve it.

I have a virtual fullName field and it has two dependency as firstName and lastName

fullName: {
  type: new DataTypes.VIRTUAL(DataTypes.STRING, ['firstName', 'lastName']),
  get() {
    return `${this.firstName} ${this.lastName}`;
  }
}

I can delete firstName and lastName fields from returned object as below:

fullName: {
  type: new DataTypes.VIRTUAL(DataTypes.STRING, ['firstName', 'lastName']), 
  get() {
    const fullName = `${this.firstName} ${this.lastName}`;

    delete this.dataValues.firstName;
    delete this.dataValues.lastName;

    return fullName;
  }
}

But there is a problem in this solution. Whenever you pull fullName field, firstName and lastName fields will be deleted.

To detect when we exclude this fields, we create extra alias fields in query like below.

Staff.findAll({
  attributes: ['id', 'fullName', ['firstName', 'excludeFirstName'], ['lastName', 'excludeLastName'] ]
});

And add extra condition in get method of fullName field. We check if there exist a field like 'excludeFirstName', then delete both firstName field and excludeFirstName field that we created after. Same as lastName field.

fullName: {
  type: new DataTypes.VIRTUAL(DataTypes.STRING, ['firstName', 'lastName']),
  get() {
    const fullName = `${this.firstName} ${this.lastName}`;

    if('excludeFirstName' in this.dataValues){
      delete this.dataValues.firstName;
      delete this.dataValues.excludeFirstName;
    }

    if('excludeLastName' in this.dataValues){
      delete this.dataValues.lastName;
      delete this.dataValues.excludeLastName;
    }

    return fullName;
  }
}

Now we can filter dependency fields if we want.

I hope it helps :)

@codoffer
Copy link

codoffer commented Dec 2, 2021

Hello, I am still facing an issue with this. Is there any way to hide the base column? I have tried to exclude column, but its not working.

@WikiRik WikiRik reopened this Dec 2, 2021
@WikiRik WikiRik added type: feature For issues and PRs. For new features. Never breaking changes. topic: virtual fields For issues and PRs. Things that involve virtual fields. and removed stale labels Dec 2, 2021
@SheaOHagan
Copy link

Hi, I also have an issue with this. I have also tried to exclude the column but it's not working. Interested if there's a solution.

@adilsoncarvalho
Copy link

In my case, I have virtuals that generate other URL addresses for a photo, and because of that, the field photo was returned as well. The only way I found to be able to remove the photo was to convert the model to plain and then delete the attribute.

res.json(
  people.map(person => {
    const p = person.get({ plain: true });
    delete p.photo;
    return p;
  })
);

Here is the sequelize doc I used to understand the differences between raw and plain.

@ephys
Copy link
Member

ephys commented May 28, 2022

A way to solve this is to define your own toJSON() method on your model which only returns the public data:

class User extends Model {
  toJSON() {
    const data = { ...super.toJSON() };

    delete data.secret;

    return data;
  }
}

Of course there are other ways to filter which data will be sent to the client, but it's not really the ORM's responsibility.


Excluding an attribute excludes it from being loaded from the database, if you have a method that depends on it, that will not work.

If you want to exclude an attribute that is being depended upon by a virtual attribute, you need to exclude the virtual attribute too.


I'll close as it doesn't look like there is a bug

@ephys ephys closed this as not planned Won't fix, can't repro, duplicate, stale May 28, 2022
@scottbeacher
Copy link

Overriding toJSON to delete the field only works if you never want that attribute ever. Using the attributes or include/exclude fields when you make the call is where you would naturally want to declare what you want returned. Maybe there needs to be a "returnedFields" or some parameter that can be included that can be used which fields should be returned in the call, but that just feels excessive since that's what the attributes are. Sequelize really should just know if a field is required because a virtual field needs it but is specifically excluded from the attributes then it shouldn't be included in the returned JSON.

This issue was labeled as a feature, not a bug.

@ephys
Copy link
Member

ephys commented May 28, 2022

This issue was labeled as a feature, not a bug.

Ah yes, I'll reopen then.

However I don't think this is in scope for an ORM.
I think it belongs to a serialization layer, unrelated to where the data comes from.

Not strictly related to this issue but I also don't think it's normal for virtual attributes to override the exclude parameter and load attributes that were explicitly excluded. I think this scenario should emit an error.
I don't think adding extra interactions between these two options is something we want to do. There is also a huge question mark regarding how it interacts with the other methods, such as reload, update, etc…

@ephys ephys reopened this May 28, 2022
@elizatlawy
Copy link

Is there any progress to implement this feature?

@ephys
Copy link
Member

ephys commented Mar 14, 2023

None, but as I indicated in May of last year I still don't believe this feature belongs in an ORM.

@alexgilbertDG
Copy link

alexgilbertDG commented Apr 15, 2023

For my project I use this fix, which in my opinion is not that bad and don't conflict with the sequelize instance:

The goal: Have "groupHandles" field but not the "groups" field in the JSON.

In User model file, the virtual field:

  groupHandles: {
            type: Sequelize.DataTypes.VIRTUAL,
            get(): string[] {
                //@ts-ignore
                const value = this.getDataValue("groups");
                if (!value) return [];
                return value.map((g: any) => g.handle);
            },
        },

In User model file:

User.prototype.excludeProperties = ["groups"]

In Meeting model file:

  Meeting.addScope("defaultScope", {
        include: [
            {
                model: models.user.scope("simple"),//use simple scope
                as: "users",
                through: { attributes: [] },//remove the pivot table
            },
        ],
    });

Meeting.prototype.excludeProperties = [
    ...User.prototype.excludeProperties.map((value: string) => `users.*.${value}`), //make sure to remove all users excludeProperties, since include with scope is a big issue
];
//remove groups in users
Meeting.prototype.toJSON = function () {
    const values = Object.assign({}, this.get());
    return _.omit(values, Meeting.prototype.excludeProperties);//remove properties we dont want it in response.
};

You will need to repeat that code each time you have a include associations.

@nbkhope
Copy link

nbkhope commented May 16, 2024

"sequelize": "^6.31.0",
@1tayH I have the exact same problem. Which leads me to stop using virtual fields since it's pointless if they expose dependent fields necessary for the computation of the field, but I dont want to be public.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: virtual fields For issues and PRs. Things that involve virtual fields. type: feature For issues and PRs. For new features. Never breaking changes.
Projects
None yet
Development

No branches or pull requests