Removing regex pattern doesn't remove by insertion order

[StarpTech]

@mcollina Is it intentionaly? I'm a little bit afraid but it's been a while since I worked on the project.

test('removing regex pattern without other keys deletes all matches', function (t) {
  t.plan(4)

  var instance = bloomrun()
  var pattern = { to: /^[a-zA-Z0-9-.]+$/i }
  var pattern2 = { to: 'foo' }

  instance.add(pattern)
  instance.add(pattern2)

  t.deepEqual(instance.lookup({ to: 'you' }), pattern)
  t.deepEqual(instance.lookup({ to: 'foo' }), pattern2)

  // since we use "insertion mode" I expect that pattern2 will be removed as well because the regex match both pattern
  instance.remove(pattern)

  t.equal(instance.lookup({ to: 'you' }), null)
  t.equal(instance.lookup({ to: 'foo' }), null)
})

test('removing regex pattern with other keys deletes all matches', function (t) {
  t.plan(4)

  var instance = bloomrun()
  var pattern = { to: /^[a-zA-Z0-9-.]+$/i, a: 'foo' }
  var pattern2 = { to: 'foo', a: 'foo' }

  instance.add(pattern)
  instance.add(pattern2)

  t.deepEqual(instance.lookup({ to: 'you', a: 'foo' }), pattern)
  t.deepEqual(instance.lookup({ to: 'foo', a: 'foo' }), pattern)

  instance.remove(pattern)

  t.equal(instance.lookup({ to: 'you', a: 'foo' }), null)
  // here I expect that both lookups returns an empty result because both matched with the first pattern
  t.equal(instance.lookup({ to: 'foo', a: 'foo' }), null)
})

[mcollina]

Maybe, I don't remember either :/. I'm not using this one at all. Have you tried using the github history/blame feature?

[StarpTech]

Have you tried using the github history/blame feature?

There was no significant changes regarding to regex.

Could you explain the edge cases when using regex?

This case is also a bit odd.

  var instance = bloomrun()
  var pattern = { to: /.*/, foo: /.*/ }
  var pattern2 = { to: /.*/, foo: 'bar' }

  instance.add(pattern)
  instance.add(pattern2)

// this will fail because pattern2 is expected
// but according to the docs the default behaviour is insertion mode?
  t.deepEqual(instance.lookup({ to: 'you', foo: 'bar' }), pattern)

and in that case the second registered pattern is still there

  var instance = bloomrun()
  var pattern = { to: /.*/, foo: /.*/ }
  var pattern2 = { to: /.*/, foo: 'bar' }

  instance.add(pattern)
  instance.add(pattern2)

  instance.remove(pattern)

  t.equal(instance.lookup({ to: 'you' }), null)
  // will fail because pattern2 is still registered
  t.equal(instance.lookup({ to: 'you', foo: 'bar' }), null)

so remove is not able to remove more than one registered pattern? When yes then I dont know why this is possible

 var instance = bloomrun()
  var pattern = { to: 'you' }
  var pattern2 = { to: 'you' }

  instance.add(pattern)
  instance.add(pattern2)

  instance.remove(pattern)

// correct both are removed
  t.equal(instance.lookup({ to: 'you' }), null)
  t.equal(instance.lookup({ to: 'you' }), null)

is there an edge case with regex?

[mcollina]

I don't know :/.

[StarpTech]

@mcollina could you have a detail look at it and tell me your expertise how it should behave correctly? After that, I could work on it.

[mcollina]

IMHO I don't use this module directly anymore, and I totally forgot about it. I recommend that a) you fix the best way you can think of and we release a new major or b) we leave it as it is.