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

eq object (JSON columns) doesn't work #477

Open
mgabeler-lee-6rs opened this issue Apr 22, 2021 · 6 comments
Open

eq object (JSON columns) doesn't work #477

mgabeler-lee-6rs opened this issue Apr 22, 2021 · 6 comments

Comments

@mgabeler-lee-6rs
Copy link
Contributor

mgabeler-lee-6rs commented Apr 22, 2021

Steps to reproduce

  1. Create a model that has an object column, mapped as a JSON type in PostgreSQL
  2. Try to do a find searching for a full value in that property/column, e.g. const objectValue = {a: 1, b: 2}; and then repo.find({where: {objectProperty: objectValue}}) or repo.find({where: {objectProperty: {eq: objectValue}}})

Current Behavior

  1. The {objectProperty: {eq: value}} is translated down into {objectProperty: value}
  2. This line assumes that, if the value is an object, it must contain exactly one field that must be an operator: https://github.com/strongloop/loopback-connector-postgresql/blob/master/lib/postgresql.js#L654
  3. It tries to map e.g. a as an operator name
  4. The buildExpression operator switch hits its default clause which delegates to the base class in loopback-connector: https://github.com/strongloop/loopback-connector-postgresql/blob/master/lib/postgresql.js#L540-L543
  5. That base class method has a switch with no default clause, so it doesn't throw any errors and just concatenates the column name with the placeholder for the value: https://github.com/strongloop/loopback-connector/blob/master/lib/sql.js#L969
  6. And so it generates invalid SQL that looks like "columName"$1

Expected Behavior

  • I should be able to use object values in where clauses if the property contains object values

Link to reproduction sandbox

WIP -- NB: encountering this in an LB4 app

Additional information

  • Running on linux x64 12.22.1
  • npm ls doesn't work with rush, but using loopback-connector-postgresql v5.0.1, with loopback-connector v4.11.1, and the following LB4 components:
  • "@loopback/boot": "2.2.0"
  • "@loopback/context": "3.9.3"
  • "@loopback/core": "2.5.0"
  • "@loopback/metadata": "2.2.6"
  • "@loopback/openapi-v3": "3.3.1"
  • "@loopback/openapi-v3-types": "1.2.1"
  • "@loopback/repository": "2.4.0"
  • "@loopback/rest": "4.0.0"
  • "@loopback/rest-explorer": "2.2.0"

Related Issues

Haven't found any yet

Workaround

Create a custom class to represent the value, and then have the equality comparison value use that, e.g. something like this, but without the prototype pollution vulnerabilities:

class JSONWrapper {
  [k: string]: any
  constructor(value: any) {
    Object.assign(this, value)
  }
}

// elsewhere:
repo.find({where: {objectProperty: new JSONWrapper(objectValue)}});

This causes the expression.constructor === Object check to fail, and so it doesn't try to unwrap the value

@stale
Copy link

stale bot commented Jul 12, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 12, 2021
@mgabeler-lee-6rs
Copy link
Contributor Author

I'm not gonna fight with stale-bot ... having something like that activate without the maintainers even visibly acknowledging the issue just encourages switching to a better supported framework :(

@stale stale bot removed the stale label Jul 12, 2021
@stale
Copy link

stale bot commented Sep 10, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Sep 10, 2021
@mgabeler-lee-6rs
Copy link
Contributor Author

@dhmlau @bajtos any chance someone can look at this? having stale-bot auto-close things without the maintainers even seeming to look at them doesn't give your users good feels.

@stale stale bot removed the stale label Sep 10, 2021
@achrinza
Copy link
Member

Hi @mgabeler-lee-6rs, apologies for not attending to the issue earlier. I've added this to my pipeline for review and will try to see if I can timebox this.

Currently, there's quite a bit of backlog at my end so unfortunately I may not be able to work on a fix asap; Though I'm open to merging community contributions.

@stale
Copy link

stale bot commented Nov 10, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

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

No branches or pull requests

2 participants