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

and filter with exists doesn't work against arrays with duplicate values #24

Closed
leomchan opened this issue May 26, 2019 · 4 comments
Closed
Assignees
Labels

Comments

@leomchan
Copy link

If you have an and filter, and one of the sub filters is an exists against a array with duplicate values, the other filters are ignored.

Expected Behavior

The other filters should still be respected.

Current Behavior

It appears the other filters are ignored.

Steps to Reproduce

The following code demonstrates the problem

const Koncorde = require('koncorde');

const engine = new Koncorde();

const filter = {
  and: [
    {
      equals: {
        name: 'Leo',
      }
    },
    {
      exists: 'skills.languages["javascript"]'
    }
  ]
};

engine.register('index', 'collection', filter)
.then((result) => {
  const matches1 = engine.test('index', 'collection', {
    name: 'Leo',
    skills: {
      languages: ['c', 'java', 'javascript']
    }
  });

  if (matches1.indexOf(result.id) < 0) {
    throw new Error('Document 1 should have matched.');
  }

  const matches2 = engine.test('index', 'collection', {
    name: 'Bob',
    skills: {
      languages: ['pascal', 'javascript', 'python', 'javascript']
    }
  });

  if (matches2.indexOf(result.id) >= 0) {
    throw new Error('Document 2 should not have matched.');
  }
})
.then(() => {
  console.log('Finished.');
  process.exit(0);
})
.catch((err) => {
  console.error(err);
  process.exit(1);
})

Note that in the second document javascript is duplicated in skills.languages. If you remove the extra javascript item, the test passes as expected.

Context (Environment)

I'm using lts/carbon, with koncorde 1.2.1
Problem came across in our internal testing.

@scottinet scottinet added the bug label Jun 21, 2019
@scottinet scottinet self-assigned this Jun 21, 2019
@scottinet
Copy link
Contributor

Thanks for the heads up. I'll take a look at it.

@scottinet
Copy link
Contributor

@leomchan,

Operands (and, or, bool) work by maintaining a counter of different conditions that must all be met before a subfilter can match.

That counter should only be decremented once per tested condition on any given subfilter.
But when testing an array of values with exists, the exists matcher decrements the counter without verifying if the condition has already been verified.

For instance, the following condition: exists: foo["bar"] matches 2 times if the following object is tested: {"foo": ["bar", "bar"]}

This bug is solved by #26, by making the exists matcher test only a set of unique values extracted from the array.

I checked the other matchers and they seem unaffected by this bug.

And... I also activated notifications on this repository. Sorry for not having see that issue earlier.

@leomchan
Copy link
Author

@scottinet ,

Thanks for looking into this issue and replying to me!

@scottinet
Copy link
Contributor

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

No branches or pull requests

2 participants