Bug in movies.py - paging

I recently finished the course, and while going through it I happened upon a bug in the API side that I don’t think anyone has noticed. I’ll first describe the ‘bug’ that led me to movies.py: during the course, I set up a mongodb Docker container for local testing, and used the atlas cluster for prod. This container was mongodb v4.4, while the atlas cluster it has you set up is v4.2. Where I ran into an issue was the unit test for paging and the assert statement about the title of the movie on the last page of the paged results. From test_paging.py:

@pytest.mark.paging
def test_supports_paging_by_text(client):
    filter = {'text': 'bank robbery'}
    (movies0, results0) = get_movies(filter, 0, 20)
    assert len(list(movies0)) == 20
    assert results0 == 475
    assert movies0[0].get('title') == 'The Bank'
    last_page = int(475 / 20) # last_page = 23
    (movies2, results2) = get_movies(filter, last_page, 20)
    assert len(list(movies2)) == 15
    assert movies2[0].get('title') == "Ugetsu"

That last assert was failing against the test DB (4.4), but succeeding if I ran it against the cluster (4.2). After seeing that it was giving ‘Skippy’ as the first title on the last page, I tried to figure out why and came to the conclusion it must be a difference in the way the cursor.sort() method behaves between the 2 versions. Since Ugetsu and Skippy had the same meta textscore, I hypothesized it must be using natural order when subsorting on equal fields in 4.2, and implicitly subsorting on additional fields (title? Since ‘S’ comes before ‘U’) in 4.4. Again this only happens if you have DB version 4.4, so if you follow the instructions and do everything against the cluster you won’t have any issues. So that’s fine, a ‘bug’ of my own creation. But while trying to figure this out, I noticed a bug in movies.py in how the paging is implemented when doing a text search.

If you do exactly what the unit test is doing, but using the actual website and scrolling, you’ll start to see something wrong. Here are lines 61-64 of movies.py:

search = request.args.get('text')
if search:
    filters["text"] = search
    return_filters["search"] = search

Notice it returns a filter key of ‘search’. This leads to the following behavior: opening the search panel, selecting text, then typing in ‘bank robbery’ renders the first 20 movies as expected, and as you can see from the debug output from flask:
"GET /api/v1/movies/search?text=bank%20robbery HTTP/1.1" 200 -
Now what happens if we scroll to the bottom? It should simply add &page=1 to the end of the GET, but since the return filter is ‘search’ this happens:
"GET /api/v1/movies/search?search=bank%20robbery&page=1 HTTP/1.1" 200 -
Now ‘search’ is not a valid key/type of search, so this part of the request is ignored, and what we see is the unfiltered collection of all movies but with page=1. This becomes obvious for all subsequent pages:
"GET /api/v1/movies/search?page=2 HTTP/1.1" 200 -
"GET /api/v1/movies/search?page=3 HTTP/1.1" 200 -
"GET /api/v1/movies/search?page=4 HTTP/1.1" 200 -
and so on…

The simple fix for this is to change line 64 in movies.py from
return_filters["search"] = search
to
return_filters["text"] = search
and now the filter is kept as expected for all pages > 0.

Since this is a bug in the actual front-end I doubt many would have noticed unless they tried as I did to replicate what the unit test was doing within the UI.

Just wanted to let the instructors know about 2 potential issues. Let me know if this helps anyone.

1 Like

Thanks @Jonathan_Strasburg. We will take a look.

Kanika

Thanks @Jonathan_Strasburg, I have same issue here using docker with last mongo image, after change to mongo:4.2, my pytest -m paging was ok.
Thanks a lot

I have set my cluster up for this course using Mongo DB 4.2 but still seem to be having an issue with this last assertion. Has anyone else encountered the same or similar issue with this unit test and resolved it?

Edit
It seems to have started passing randomly. Not sure what the issue was…

Failing for me as well.

Hi @David_Baker

Did you setup MongoDB 4.2 or which version did you use? Is the test still failing for you?

Kindest regards,
Eoin

Here’s the info.

def get_movies(filters, page, movies_per_page):

“”"

Returns a cursor to a list of movie documents.

Based on the page number and the number of movies per page, the result may

be skipped and limited.

The filters from the API are passed to the build_query_sort_project

method, which constructs a query, sort, and projection, and then that query

is executed by this method (get_movies).

Returns 2 elements in a tuple: (movies, total_num_movies)

“”"

query, sort, project = build_query_sort_project(filters)

if project:

cursor = db.movies.find(query, project).sort(sort)

else:

cursor = db.movies.find(query).sort(sort)

total_num_movies = 0

if page == 0:

total_num_movies = db.movies.count_documents(query)

“”"

Ticket: Paging

Before this method returns back to the API, use the “movies_per_page”

argument to decide how many movies get displayed per page. The “page”

argument will decide which page

Paging can be implemented by using the skip() and limit() methods against

the Pymongo cursor.

“”"

TODO: Paging

Use the cursor to only return the movies that belong on the current page.

total_num_movies = cursor.count()

movies = cursor.skip(movies_per_page * page).limit(movies_per_page)

return (list(movies), total_num_movies)

================================== FAILURES ===================================

________________________ test_supports_paging_by_text _________________________

client = <FlaskClient <Flask ‘mflix.factory’>>

@pytest.mark.paging

def test_supports_paging_by_text(client):

filter = {‘text’: ‘bank robbery’}

(movies0, results0) = get_movies(filter, 0, 20)

assert len(list(movies0)) == 20

assert results0 == 475

assert movies0[0].get(‘title’) == ‘The Bank’

last_page = int(475 / 20)

(movies2, results2) = get_movies(filter, last_page, 20)

assert len(list(movies2)) == 15

assert movies2[0].get(‘title’) == “Ugetsu”

E assert “Operation 'Y…'s Adventures” == ‘Ugetsu’

E - Operation ‘Y’ & Other Shurik’s Adventures

E + Ugetsu

tests\test_paging.py:46: AssertionError

---------------------------- Captured stdout call -----------------------------

Thanks,

David

Hi @David_Baker

Thanks for flagging this, can you re-try the query with a fresh load of the Atlas Dataset if you are using Atlas or re-import the mflix data if you are using this locally. I’d like to rule out that it’s an accidental change to your local copy of the data.

Thanks!
Eoin

Something is really off with Paging unit test. UI (status page) gives PASS, while CLI pytest -m paging fails on 3rd test (search by text) doing the very last assert:

E assert “Operation 'Y…'s Adventures” == ‘Ugetsu’
E - Operation ‘Y’ & Other Shurik’s Adventures
E + Ugetsu

Yes I am having the same issue.

client = <FlaskClient <Flask ‘mflix.factory’>>

@pytest.mark.paging
def test_supports_paging_by_text(client):
    filter = {'text': 'bank robbery'}
    (movies0, results0) = get_movies(filter, 0, 20)
    assert len(list(movies0)) == 20
    assert results0 == 475
    assert movies0[0].get('title') == 'The Bank'
    last_page = int(475 / 20)
    (movies2, results2) = get_movies(filter, last_page, 20)
    assert len(list(movies2)) == 15
  assert movies2[0].get('title') == "Ugetsu"

E assert “Operation 'Y…'s Adventures” == ‘Ugetsu’
E - Operation ‘Y’ & Other Shurik’s Adventures
E + Ugetsu

tests\test_paging.py:46: AssertionError
============================= 40 tests deselected =============================
============== 1 failed, 2 passed, 40 deselected in 1.50 seconds ==============

It looks like a sorting issue in your result set.

It is better to share your code when a test case is failing as most of the time the issue is in the code rather than the test case. The test case is there to verify if your code meets all the lab requirements. When it fails, you must likely missed one of the requirements.

I suspect a sort issue because both result sets have to correct size. But a bad skip might bring the wrong list of movies.

1 Like

hi I am getting the same issue
And only that assertation is failiing
E AssertionError: assert ‘Skippy’ == ‘Ugetsu’
E - Skippy
E + Ugetsu

I managed to solve my issue. It works now

yes it helped me to get the validation key.

Thanks,

Ogabek