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

Add Insensitive Like Operator to WHERE Filter #273

Merged
merged 9 commits into from
Nov 2, 2021

Conversation

MRichards99
Copy link
Collaborator

@MRichards99 MRichards99 commented Oct 18, 2021

This PR will close #243

Description

This adds two new operators for the WHERE filter, ilike and nilike. The first one will be used for ral-facilities/datagateway#731, and the not like alternative was simple to implement while I was there so I might as well ready for the Search API. This makes use of JPQL functions, specifically UPPER() will is applied to both sides of the query, the values in icatdb as well as the value given by the user. The support for JPQL functions is being added in 0.20.0 of Python ICAT.

For ral-facilities/datagateway#731 to be fixed, there will need to be a search and replace task on like to ilike in DataGateway. Equally, I'm happy to quickly change this PR so the like is actually case insensitive, but I believe having a specific ilike operator is the last thing we agreed on verbally. Something for @louise-davies and I to discuss.

I've had to edit some of the existing unit tests because there are some small changes in how conditions are stored in Python ICAT. The main change is that the conditions (stored in dictionary keys) are always stored as a list of strings, even if there is only a single condition per attribute. This makes no difference to functionality (nor am I concerned about this for anything in the future) but did require editing the tests to suit the new behaviour.

This PR will remain in a draft state until Python ICAT 0.20.0 has been released and I update the API's dependencies to use the new version.

I've also been trying to add a couple of tests that create a test DataGateway API (using Flask's test app) but pointing to ISIS dev ICAT instead of localhost. I've got it working independently of the rest of the tests, but it seems to get confused with the other test apps that run on localhost when running all tests together. I've got this saved in a stash and will try to get it working - don't want to spend excessive amounts of time on it.

Testing Instructions

ICATs using mariadb are case insensitive by nature so this feature must be tested on an ICAT which uses an Oracle database. I have been using ISIS production and dev for my own manual testing, with the following requests:

# Should show two results
http://{{datagateway-api}}/investigations?where={"title": {"like": "tomato"}}

# Should also two results, even though the request sends toMATo
http://{{datagateway-api}}/investigations?where={"title": {"ilike": "toMATo"}}

# Should return two results as it's looking for things not like tomato
http://{{datagateway-api}}/investigations?where={"id": {"in": [12578343, 12578344]}}&where={"title": {"nlike": "toMATo"}}

# Should return an empty array as it matches the title with what's given in the request and NOTs that
http://{{datagateway-api}}/investigations?where={"id": {"in": [12578343, 12578344]}}&where={"title": {"nilike": "toMATo"}}

Until Python ICAT 0.20.0 is released, you'll need to build and install the fixes in Python ICAT manually.

  • Review code
  • Check GitHub Actions build
  • If icatdb Generator Script Consistency Test CI job fails, is this because of a deliberate change made to the script to change generated data (which isn't actually a problem) or is there an underlying issue with the changes made?
  • Review changes to test coverage

Agile Board Tracking

Connect to #243

@codecov
Copy link

codecov bot commented Nov 1, 2021

Codecov Report

Merging #273 (550de36) into master (a426ec7) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #273      +/-   ##
==========================================
+ Coverage   91.23%   91.25%   +0.02%     
==========================================
  Files          32       32              
  Lines        2395     2401       +6     
  Branches      205      207       +2     
==========================================
+ Hits         2185     2191       +6     
  Misses        186      186              
  Partials       24       24              
Impacted Files Coverage Δ
datagateway_api/common/icat/filters.py 99.36% <100.00%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a426ec7...550de36. Read the comment docs.

@MRichards99
Copy link
Collaborator Author

Ignore the failing check from 'Semantic Pull Request'. This tool will be used for the versioning i.e. once #242 is closed.

@MRichards99
Copy link
Collaborator Author

The safety part of the CI workflow will be fixed once #278 is merged.

@MRichards99 MRichards99 marked this pull request as ready for review November 1, 2021 13:19
Copy link
Contributor

@VKTB VKTB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and manual tests pass as expected!

@MRichards99 MRichards99 merged commit 822408b into master Nov 2, 2021
@MRichards99 MRichards99 deleted the feature/ilike-operator-#243 branch November 2, 2021 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Insensitive Like Operator
2 participants