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

Fix parse of filter rules for boolean fields into SQL query #16982

Merged
merged 1 commit into from Dec 5, 2019

Conversation

DaveTBlake
Copy link
Member

Fix how filter rules for boolean fields, as given in smart playlist files (.xsp), custom node files(.xml) and JSON API data requests, are converted into SQL queries. In particular correctly handle "is", "isnot", "contains" and "doesnotcontain" operators with value true or false.


It is far from intuitive how filter rules for boolean fields should be entered. Of course operators "true" and "false" naturally work with boolean fields e.g.
a) in xml (for smartplaylists and custom nodes)
<rule field="compilation" operator="true">
<rule field="compilation" operator="false">

b) in JSON
"filter": { "field": "compilation", "operator": "true", "value": "any dummy value here"}
"filter": { "field": "compilation", "operator": "false", "value": "any dummy value here"}

Notice that the JSON schema forces all filter rules to have a "value" property regardless of operator hence "filter": { "field": "compilation", "operator": "true"} is invalid.

However it is reasonable to expect to be able to use
<rule field="compilation" operator="is"> <value>true</value> </rule>
or
"filter": { "field": "compilation", "operator": "is", "value": "true"}

These are valid rule input formats, but an incorrect SQL query is formed as a consequence and the data is not filtered in the way the user expects.

This PR ensures that for boolean fields the SQL created with operators "is" or "contains" and value "true", or operators "isnot" or "doesnotcontain" and value "false" will result in the same SQL as operator="true". Likewise that created with operators "is" or "contains" and value "false", or operators "isnot" or "doesnotcontain" and value "true" will be the same as with operator="false"

An example test smart playlist is follows (you need a music library with some albums flagged as compilations

<?xml version="1.0" encoding="UTF-8" standalone="yes" ?>
<smartplaylist type="albums">
    <name></name>
    <match>all</match>
    <rule field="compilation" operator="is">
        <value>true</value>
    </rule>
</smartplaylist>

Equilvalent JSON request

{"jsonrpc": "2.0", "id": 1, "method": "AudioLibrary.GetAlbums", "params": { 
"limits": { "start" : 0, "end": 60 }, 
"filter": {"field": "compilation", "operator": "is", "value": "true"} } }

Bump patch version of JSON API as change to internal implementation but not to the API definition.

@DaveTBlake
Copy link
Member Author

jenkins has been "pending" for 2 days now?
Anyway BuildMulti PR-Manualy gives greens https://jenkins.kodi.tv/view/Helpers/job/BuildMulti-PR-Manually/308/

@Rechi
Copy link
Member

Rechi commented Dec 2, 2019

The build you linked hasn't build all platforms a PR build would build.

@DaveTBlake
Copy link
Member Author

DaveTBlake commented Dec 2, 2019

Yes, I know that @Rechi but what would be the point in adding UWP or Win32 which fail for everything.

Any idea why jenkins is pending fo 2 days?

@Rechi
Copy link
Member

Rechi commented Dec 2, 2019

Those aren't the only ones missing. Android-X86, FreeBSD, IOS, LINUX-64, LINUX-64-GBM, LINUX-64-Wayland, LINUX-AML, LINUX-RBPI and TVOS are missing too.

Also building the known failing platforms still has a point. You can see if a change causes an earlier build failure.

@DaveTBlake
Copy link
Member Author

Yes I know that there are other platforms too @Rechi
What would be really useful is if you could press the magic button and get this jenkin PR build unstuck

@DaveTBlake
Copy link
Member Author

jenkins build this please

@DaveTBlake
Copy link
Member Author

Build errors are UWP/Python3 related, nothing to do with this PR

@DaveTBlake DaveTBlake merged commit 07070b8 into xbmc:master Dec 5, 2019
@DaveTBlake DaveTBlake deleted the boolrulefix branch December 5, 2019 14:32
Maven85 pushed a commit to Maven85/kodi that referenced this pull request Jan 21, 2020
Fix parse of filter rules for boolean fields  into SQL query
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants