Mercurial > p > roundup > code
comparison test/rest_common.py @ 7853:03c1b7ae3a68
issue2551328/issue2551264 unneeded next link and total_count incorrect
Fix: issue2551328 - REST results show next link if number of
results is a multiple of page size. (Found by members of
team 3 in the UMass-Boston CS682 Spring 2024 class.)
issue2551264 - REST X-Total-Count header and @total_size
count incorrect when paginated
These issues arose because we retrieved the exact number of rows
from the database as requested by the user using the @page_size
parameter. With this changeset, we retrieve up to 10 million + 1
rows from the database. If the total number of rows exceeds 10
million, we set the total_count indicators to -1 as an invalid
size. (The max number of requested rows (default 10 million +1)
can be modified by the admin through interfaces.py.)
By retrieving more data than necessary, we can calculate the
total count by adding @page_index*@page_size to the number of
rows returned by the query.
Furthermore, since we return more than @page_size rows, we can
determine the existence of a row at @page_size+1 and use that
information to determine if a next link should be
provided. Previously, a next link was returned if @page_size rows
were retrieved.
This change does not guarantee that the user will get @page_size
rows returned. Access policy filtering occurs after the rows are
returned, and discards rows inaccessible by the user.
Using the current @page_index/@page_size it would be difficult to
have the roundup code refetch data and make sure that a full
@page_size set of rows is returned. E.G. @page_size=100 and 5 of
them are dropped due to access restrictions. We then fetch 10
items and add items 1-4 and 6 (5 is inaccessible). There is no
way to calculate the new database offset at:
@page_index*@page_size + 6 from the URL. We would need to add an
@page_offset=6 or something.
This could work since the client isn't adding 1 to @page_index to
get the next page. Thanks to HATEOAS, the client just uses the
'next' url. But I am not going to cross that bridge without a
concrete use case.
This can also be handled client side by merging a short response
with the next response and re-paginating client side.
Also added extra index markers to the docs to highlight use of
interfaces.py.
| author | John Rouillard <rouilj@ieee.org> |
|---|---|
| date | Mon, 01 Apr 2024 09:57:16 -0400 |
| parents | be6cb2e0d471 |
| children | 171ff2e487df |
comparison
equal
deleted
inserted
replaced
| 7852:9e686e3f39c1 | 7853:03c1b7ae3a68 |
|---|---|
| 5 | 5 |
| 6 from time import sleep | 6 from time import sleep |
| 7 from datetime import datetime, timedelta | 7 from datetime import datetime, timedelta |
| 8 from roundup.anypy.cgi_ import cgi | 8 from roundup.anypy.cgi_ import cgi |
| 9 from roundup.anypy.datetime_ import utcnow | 9 from roundup.anypy.datetime_ import utcnow |
| 10 from roundup.exceptions import UsageError | |
| 10 from roundup.test.tx_Source_detector import init as tx_Source_init | 11 from roundup.test.tx_Source_detector import init as tx_Source_init |
| 11 | 12 |
| 12 | 13 |
| 13 try: | 14 try: |
| 14 from datetime import timezone | 15 from datetime import timezone |
| 334 issue_open_crit = self.db.issue.create( | 335 issue_open_crit = self.db.issue.create( |
| 335 title='foo5', | 336 title='foo5', |
| 336 status=self.db.status.lookup('open'), | 337 status=self.db.status.lookup('open'), |
| 337 priority=self.db.priority.lookup('critical') | 338 priority=self.db.priority.lookup('critical') |
| 338 ) | 339 ) |
| 340 | |
| 341 def test_no_next_link_on_full_last_page(self): | |
| 342 """Make sure that there is no next link | |
| 343 on the last page where the total number of entries | |
| 344 is a multiple of the page size. | |
| 345 """ | |
| 346 | |
| 347 self.server.client.env.update({'REQUEST_METHOD': 'GET'}) | |
| 348 | |
| 349 # Retrieve third user of the total of 3. | |
| 350 form = cgi.FieldStorage() | |
| 351 form.list = [ | |
| 352 cgi.MiniFieldStorage('@page_index', '3'), | |
| 353 cgi.MiniFieldStorage('@page_size', '1'), | |
| 354 ] | |
| 355 results = self.server.get_collection('user', form) | |
| 356 self.assertEqual(self.dummy_client.response_code, 200) | |
| 357 self.assertEqual(len(results['data']['collection']), 1) | |
| 358 self.assertEqual(results['data']['collection'][0]['id'], "3") | |
| 359 self.assertEqual(results['data']['@total_size'], 3) | |
| 360 print(self.dummy_client.additional_headers["X-Count-Total"]) | |
| 361 self.assertEqual( | |
| 362 self.dummy_client.additional_headers["X-Count-Total"], | |
| 363 "3" | |
| 364 ) | |
| 365 self.assertNotIn('next', results['data']['@links']) | |
| 366 self.dummy_client.additional_headers.clear() | |
| 367 | |
| 368 # Retrieve first user of the total of 3. | |
| 369 form = cgi.FieldStorage() | |
| 370 form.list = [ | |
| 371 cgi.MiniFieldStorage('@page_index', '1'), | |
| 372 cgi.MiniFieldStorage('@page_size', '1'), | |
| 373 ] | |
| 374 results = self.server.get_collection('user', form) | |
| 375 self.assertEqual(self.dummy_client.response_code, 200) | |
| 376 self.assertEqual(len(results['data']['collection']), 1) | |
| 377 self.assertEqual(results['data']['collection'][0]['id'], "1") | |
| 378 self.assertEqual(results['data']['@total_size'], 3) | |
| 379 print(self.dummy_client.additional_headers["X-Count-Total"]) | |
| 380 self.assertEqual( | |
| 381 self.dummy_client.additional_headers["X-Count-Total"], | |
| 382 "3" | |
| 383 ) | |
| 384 self.assertIn('next', results['data']['@links']) | |
| 385 self.dummy_client.additional_headers.clear() | |
| 386 | |
| 387 def testTotal_size(self): | |
| 388 """Make sure that total_size is properly set if @page_size | |
| 389 is specified. | |
| 390 | |
| 391 Also test for the cases: | |
| 392 | |
| 393 @page_size >= the max number of retreivable rows. | |
| 394 raises UsageError (and error code 400) | |
| 395 @page_size < max retreivable rows, but | |
| 396 the amount of matching rows is > max retreivable rows. | |
| 397 total_size/X-Count-Total should be -1 | |
| 398 | |
| 399 no @page_size and limit < total results returns | |
| 400 limit size and -1 for total. | |
| 401 | |
| 402 Check: | |
| 403 http response code | |
| 404 length of collection | |
| 405 An expected id at end of collection | |
| 406 @total_size in payload | |
| 407 X-Count-Total in http headers | |
| 408 | |
| 409 """ | |
| 410 from roundup.rest import RestfulInstance | |
| 411 | |
| 412 self.server.client.env.update({'REQUEST_METHOD': 'GET'}) | |
| 413 | |
| 414 # Retrieve one user of the total of 3. limit 10M+1 | |
| 415 form = cgi.FieldStorage() | |
| 416 form.list = [ | |
| 417 cgi.MiniFieldStorage('@page_index', '1'), | |
| 418 cgi.MiniFieldStorage('@page_size', '1'), | |
| 419 ] | |
| 420 results = self.server.get_collection('user', form) | |
| 421 self.assertEqual(self.dummy_client.response_code, 200) | |
| 422 self.assertEqual(len(results['data']['collection']), 1) | |
| 423 self.assertEqual(results['data']['collection'][0]['id'], "1") | |
| 424 self.assertEqual(results['data']['@total_size'], 3) | |
| 425 print(self.dummy_client.additional_headers["X-Count-Total"]) | |
| 426 self.assertEqual( | |
| 427 self.dummy_client.additional_headers["X-Count-Total"], | |
| 428 "3" | |
| 429 ) | |
| 430 self.dummy_client.additional_headers.clear() | |
| 431 | |
| 432 # set max number of returned rows | |
| 433 self.stored_max = RestfulInstance.max_response_row_size | |
| 434 RestfulInstance.max_response_row_size = 2 | |
| 435 | |
| 436 # Retrieve whole class (no @page_*) with max rows restricted. | |
| 437 form = cgi.FieldStorage() | |
| 438 results = self.server.get_collection('user', self.empty_form) | |
| 439 # reset so changes don't affect other tests if any assetion fails. | |
| 440 RestfulInstance.max_response_row_size = self.stored_max | |
| 441 self.assertEqual(self.dummy_client.response_code, 200) | |
| 442 self.assertEqual(len(results['data']['collection']), 2) | |
| 443 self.assertEqual(results['data']['collection'][1]['id'], "2") | |
| 444 self.assertEqual(results['data']['@total_size'], -1) | |
| 445 print(self.dummy_client.additional_headers["X-Count-Total"]) | |
| 446 self.assertEqual( | |
| 447 self.dummy_client.additional_headers["X-Count-Total"], | |
| 448 "-1" | |
| 449 ) | |
| 450 self.dummy_client.additional_headers.clear() | |
| 451 | |
| 452 # Make sure we can access items that are returned | |
| 453 # in rows RestfulInstance.max_response_row_size + 1. | |
| 454 # so can we access item 2 | |
| 455 form = cgi.FieldStorage() | |
| 456 form.list = [ | |
| 457 cgi.MiniFieldStorage('@page_index', '2'), | |
| 458 cgi.MiniFieldStorage('@page_size', '1'), | |
| 459 ] | |
| 460 RestfulInstance.max_response_row_size = 2 | |
| 461 results = self.server.get_collection('user', form) | |
| 462 RestfulInstance.max_response_row_size = self.stored_max | |
| 463 self.assertEqual(self.dummy_client.response_code, 200) | |
| 464 self.assertEqual(len(results['data']['collection']), 1) | |
| 465 self.assertEqual(results['data']['collection'][0]['id'], "2") | |
| 466 self.assertEqual(results['data']['@total_size'], -1) | |
| 467 print(self.dummy_client.additional_headers["X-Count-Total"]) | |
| 468 self.assertEqual( | |
| 469 self.dummy_client.additional_headers["X-Count-Total"], | |
| 470 "-1" | |
| 471 ) | |
| 472 self.dummy_client.additional_headers.clear() | |
| 473 | |
| 474 # Same as above, but access item 3 | |
| 475 form = cgi.FieldStorage() | |
| 476 form.list = [ | |
| 477 cgi.MiniFieldStorage('@page_index', '3'), | |
| 478 cgi.MiniFieldStorage('@page_size', '1'), | |
| 479 ] | |
| 480 RestfulInstance.max_response_row_size = 2 | |
| 481 results = self.server.get_collection('user', form) | |
| 482 RestfulInstance.max_response_row_size = self.stored_max | |
| 483 self.assertEqual(self.dummy_client.response_code, 200) | |
| 484 self.assertEqual(len(results['data']['collection']), 1) | |
| 485 self.assertEqual(results['data']['collection'][0]['id'], "3") | |
| 486 self.assertEqual(results['data']['@total_size'], 3) | |
| 487 print(self.dummy_client.additional_headers["X-Count-Total"]) | |
| 488 self.assertEqual( | |
| 489 self.dummy_client.additional_headers["X-Count-Total"], | |
| 490 "3" | |
| 491 ) | |
| 492 self.dummy_client.additional_headers.clear() | |
| 493 | |
| 494 # Retrieve one user but max number of rows is set to 2, | |
| 495 # and we retrieve two users from the db. | |
| 496 # So we don't know how many total users there are. | |
| 497 | |
| 498 form = cgi.FieldStorage() | |
| 499 form.list = [ | |
| 500 cgi.MiniFieldStorage('@page_index', '1'), | |
| 501 cgi.MiniFieldStorage('@page_size', '1'), | |
| 502 ] | |
| 503 RestfulInstance.max_response_row_size = 2 | |
| 504 results = self.server.get_collection('user', form) | |
| 505 RestfulInstance.max_response_row_size = self.stored_max | |
| 506 self.assertEqual(self.dummy_client.response_code, 200) | |
| 507 self.assertEqual(len(results['data']['collection']), 1) | |
| 508 self.assertEqual(results['data']['collection'][0]['id'], "1") | |
| 509 self.assertEqual(results['data']['@total_size'], -1) | |
| 510 print(self.dummy_client.additional_headers["X-Count-Total"]) | |
| 511 self.assertEqual( | |
| 512 self.dummy_client.additional_headers["X-Count-Total"], | |
| 513 "-1" | |
| 514 ) | |
| 515 self.dummy_client.additional_headers.clear() | |
| 516 | |
| 517 | |
| 518 # Set the page size to be >= the max number of rows returned. | |
| 519 # and verify the exception returned. | |
| 520 form = cgi.FieldStorage() | |
| 521 form.list = [ | |
| 522 cgi.MiniFieldStorage('@page_index', '2'), | |
| 523 cgi.MiniFieldStorage('@page_size', '2'), | |
| 524 ] | |
| 525 RestfulInstance.max_response_row_size = 2 | |
| 526 results = self.server.get_collection('user', form) | |
| 527 RestfulInstance.max_response_row_size = self.stored_max | |
| 528 self.assertEqual(self.dummy_client.response_code, 400) | |
| 529 self.assertEqual(results['error']['status'], 400) | |
| 530 self.assertTrue(isinstance(results['error']['msg'], UsageError)) | |
| 531 self.assertEqual(results['error']['msg'].args[0], | |
| 532 "Page size 2 must be less than " | |
| 533 "admin limit on query result size: 2.") | |
| 534 self.assertTrue('@total_size' not in results) | |
| 535 self.assertTrue('@data' not in results) | |
| 536 self.assertTrue("X-Count-Total" not in | |
| 537 self.dummy_client.additional_headers) | |
| 538 | |
| 539 # reset environment just in case I forgot a reset above. | |
| 540 RestfulInstance.max_response_row_size = self.stored_max | |
| 339 | 541 |
| 340 def testGet(self): | 542 def testGet(self): |
| 341 """ | 543 """ |
| 342 Retrieve all three users | 544 Retrieve all three users |
| 343 obtain data for 'joe' | 545 obtain data for 'joe' |