martin-magakian / Amazing-Cloud-Search

Allow you to search, faceted search, add, update, remove objects from your Amazon Cloud Search Index in C#.
30 stars 17 forks source link

Group Condition Bug - when using StringListBooleanCondition and StringIntBooleanCondition #7

Closed martin-magakian closed 6 years ago

martin-magakian commented 11 years ago

Group Condition don't build the query correctly when it's used with StringListBooleanCondition or StringIntBooleanCondition

A failing test has been added in QueryBuilderTester.cs:

    [Test]
    public void ComplexOrWithList()
    {
        var genres = new List<string> { "Sci-Fi", "Fantasy" };
        var condition1A = new StringListBooleanCondition("genre", genres, ConditionType.OR);
        var years = new List<int> { 1987, 1990, 2010 };
        var condition1B = new IntListBooleanCondition("year", years, ConditionType.OR);
        var groupCondition1 = new GroupedCondition(condition1A, ConditionType.AND, condition1B);

        var condition2 = new StringBooleanCondition("director", "doduck");

        var groupConditionAll = new GroupedCondition(groupCondition1, ConditionType.AND, condition2);

        var bQuery = new BooleanQuery();
        bQuery.Conditions.Add(groupConditionAll);

        _searchQuery = new SearchQuery<Movie> { BooleanQuery = bQuery };
        string query = _queryBuilder.BuildSearchQuery(_searchQuery);
        query.ShouldNotContain("(and+(and+(and+genre%3A'Sci-Fi'+genre%3A'Fantasy'+year%3A1987+year%3A1990+year%3A2010)+director%3A'doduck'))");
    }
martin-magakian commented 11 years ago

IBooleanCondition implement GetConditionType(). It can be a AND / OR condition.

I guess GroupedCondition need to look for it IBooleanCondition "ConditionType" when building the query.

radavies commented 11 years ago

Hi guys,

I've been giving this a go this afternoon as I need it working for my project :)

Can I first check with you that the tests for OrCondition and ComplexOr are correct?

The reason I ask is that the first thing I noticed about the GroupedCondition class is that it's GetConditionType() is hard coded to return AND regardless of what value it is given, correct my if I'm wrong but this doesn't seem correct? If you change this class to report the CondtionType it was given in the constructor it causes the above tests to fail which seems weird.

For OrCondition you want (and+(or+genre%3A'Sci-Fi'+year%3A2013..)) and making that change results in (or+(or+genre%3A'Sci-Fi'+year%3A2013..))

For ComplexOr you want (and+(or+(and+genre%3A'Sci-Fi'+year%3A1990..)+(and+genre%3A'Fantasy'+year%3A2013..))) and with that change you get (or+(or+(and+genre%3A'Sci-Fi'+year%3A1990..)+(and+genre%3A'Fantasy'+year%3A2013..)))

I had a further change that fixed the ComplexOrWithList test but it also broke the above tests so I wanted to check with you guys as you know this change better that I do.

Thanks,

Raymond

martin-magakian commented 11 years ago

Hi Raymond, Good to hear you're working on this bug.

In all you describe below I agree. GetConditionType() shouldn't be hard coded.

Meaning the UnitTest should start with (or+... when GroupedCondition is create with OR condition.

Be careful

I'm not 100% so sure. I guess it's true. But you will probably need to dig a bit more in the API. Also test your result with real data.


Good luck !

martin-magakian commented 11 years ago

As an idea of improvement. It might be nice to change the method signature to

public GroupedCondition(ConditionType conditionType, IBooleanCondition conditionA, IBooleanCondition conditionB, params IBooleanCondition[] conditionN)

So it accept an unlimited number of of condition to group from.