Array Filters Support

[amreo]

Fixes #1

[amreo]

@256dpi this PR is becoming a very mess :( ...

[256dpi]

I didn't read all of the changes, but I guess it would make sense to discuss the strategy a bit before continuing. Everything up and including the changes based on my feedback made sense to me. To make things easier I added array-filters branch with cleaned up commits so far to be used as a base for this and additional PR's.

[256dpi]

It would be nice if you could rebase your PR and squash all obsolete commits.

[256dpi]

Now to the strategy part: It makes most sense to me to improve the processing framework to be able to handle the various positional operators. This functionality can then be leveraged by the apply functions to handle the update operators and the one projection operator.

Now, this would need some research on how this interface may look like. I don't have an idea at the moment but will also take some time to tinker and come up with some solutions. In the meantime we may also work on writing test to then be able to verify whatever solution we come up with.

I propose that we work independently on this and merge stuff that we agree in this branch until all things are together. What do you think?

[amreo]

I think it's a good idea. I will write tests and think about the interface.

[amreo]

Added resolver! @256dpi

[256dpi]

Could you move just the resolver part to a new branch based on the array-filters branch and create a PR against array-filters.

[amreo]

Isn't this PR against array-filters??? "amreo wants to merge 3 commits into 256dpi:array-filters from amreo:master"

[256dpi]

Sorry, didn't see that. I saw stuff from the previous PR floating around and assumed you didn't rebase yet. Anyhow, It would be nice If the PR would just be about the resolve function. The other changes add too much noise IMHO.

[amreo]

@256dpi Why do you have decided that bsokit.Get should take a bsonkit.Doc as param instead of a interface{}?

[amreo]

@256dpi Why do you have decided that mongokit.Match should take a bsonkit.Doc as param instead of a interface{}?

[256dpi]

I decided that bsonkit and mongokit use the Doc and List as the default types where possible to emphasize on the idea of working with full documents by default.

Since Doc is just an alias to *bson.D and a document is made of a bunch of bson.D and bson.A, you can easily convert parts of a document to a Doc temporarily. This is for example used here: https://github.com/256dpi/lungo/blob/master/mongokit/match.go#L381

[amreo]

What do you think about the commits?

[256dpi]

It looks like we are going in the right direction! However, wrapping all operators with Resolve looks quite ugly. Also, the whole resolving should be transparent to the operator implementation. Therefore, we should move it to the ProcessExpression function.

I just updated the master and array-filters branch with a change that embeds the applyAll wrapper in the ProcessExpression method (bb621e1). We should handle the resolving of positional operators in the same way.

We could add the following parameters to the Context struct:

// The query used to resolve positional operators in top level operator
// invocation paths.
TopLevelQuery bsonkit.Doc

// The array filters used to resolve positional operators in top level
// operator invocation paths.
TopLevelArrayFilters bsonkit.List

And then use them to resolve the paths before calling the operator:

// call operator for each pair
for _, cond := range update {
    err := Resolve(cond.Key, ctx.TopLevelQuery, doc, ctx.TopLevelArrayFilters, func(path string) error {
        return operator(ctx, doc, pair.Key, path, cond.Value)
    })
    if err != nil {
        return err
    }
}

Edit: Fixed type mistake.

[amreo]

Why the TopLevelArrayFilters is a []bsonkit.List instead of bsonkit.List?

// The array filters used to resolve positional operators in top level
// operator invocation paths.
TopLevelArrayFilters []bsonkit.List

[256dpi]

That's a mistake. I mean bsonkit.List of course.

[256dpi]

The PR has become a lot less noisy now. Thanks!

Since the resolving has now become transparent to operators, I have the feeling it could be enough to just add a `TestApplyPositionalOperatorX" test rather than testing each operator with positional operators.

[amreo]

I should have finished to implement the feature

[256dpi]

Thanks. I'll take a look in the upcoming days.

[256dpi]

I started with some refactoring, but I need to take another attempt at it later.

[256dpi]

I finished my edits and would be ready to merge this into array-filters. Then we can look at the whole thing and discuss the final steps. What do you think?