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

[FE-2892] fix single ref argument #667

Open
wants to merge 3 commits into
base: v4
Choose a base branch
from

Conversation

ptpaterson
Copy link
Contributor

Jira Ticket

Problem

query.Ref accepts a single argument for backwards compatibility with the Ref("collections/widget/123") format, which is serialized as { "@ref": "collections/widget/123" }.

The driver does this by just stuffing whatever is given to it as value => { "@ref": value }. This is a problem when the value provided is anything other than a string. The database expects FQL values to be... values! E.g. is illegal to even provide an expression that resolves to a string

client.query(q.Ref(q.Concat(["collections", "widget", "123"], "/"))

// wire protocol
// {"@ref":{"concat":["collections","widget","123"],"separator":"/"}}

{
  "errors": [
    {
	"position": [],
	"code": "invalid expression",
	"description": "Ref expected, Object provided."
    }
  ]
}

This error is correct behavior.

examples of other problematic queries

It is illegal wire protocol for FQL expressions to be in literal values, but is currently allowed

const funky_ref = q.Ref(q.Ref(q.Collection("things"), "307286571965481024"))
const funky_wire = JSON.stringify(funky_ref, null, 2)
console.log(funky_wire)

{
  "@ref": {
    "ref": {
      "collection": "things"
    },
    "id": "307286571965481024"
  }
}

It there is also potential for folks to wrap a value.Ref into another layer of @ref, which is problematic.

const result = await client.query(q.Ref(q.Collection("things"), "307286571965481024"))
const funky_ref = q.Ref(result)
const funky_wire = JSON.stringify(funky_ref, null, 2)

console.log(funky_wire)
{
  "@ref": {
    "@ref": {
      "id": "307286571965481024",
      "collection": {
        "@ref": {
          "id": "things",
          "collection": {
            "@ref": {
              "id": "collections"
            }
          }
        }
      }
    }
  }
}

Expected behavior

Using query.Ref with two arguments, it is expected that the arguments can be any expression 👍🏻

But if only one argument is provided, we know it must be a string (again, not even an expression that could resolve to a string).

Solution

Assert that in the single-argument case the argument is a string, i.e. typeof arguments[0] === 'string' || arguments[0] instanceof String

Testing

additional tests added

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

Successfully merging this pull request may close these issues.

None yet

1 participant