Pagination fix

[Simon128]

Pull Request Template for FastCRUD

Description

This pull request is about fixing issue #197 including some pagination related changes listed below.

Changes

• Support for cursor based pagination on _read_items function in EndpointCreator (single PK and composite)
• Added ASC column sorting by model primary key for pagination in _read_items
• Added cursor field in pagination response and changed has_more to work with cursor based pagination
• Added cursor parsing function based on SqlAlchemy column.type.python_type (see considerations below)
• Allow for multiple sort columns and tuple cursor in FastCRUD.get_multi_by_cursor for composite PKs
• Added missing _parse_filters in FastCRUD.get_multi_by_cursor
• Added missing total_count in return value of FastCRUD.get_multi_by_cursor if corresponding argument is set to True
• Changed dev-dependency of testcontainers in pyproject.toml, as this was a source of errors for me when running pytest

Considerations

1. Using SQLAlchemy column.type.python_type to parse the cursor might be error prone, as I assume a str-type if python_type is not implemented (e.g. AutoStr for SqlModel) -- maybe there is a better way?
2. The first "page" with cursor pagination can be retrieved by setting cursor=True in the query params. This is considered when parsing the cursor.
3. Composite cursors are assumed to be comma-separated in the url query param
4. FastCRUD.get_multi_by_cursor has now the arguments sort_columns as well as the previously existing sort_column for backward compatibility. Do we want to include a deprecation warning for sort_column, as this is a bit redundant, or do we want to keep both for convenience? Multiple sort orders for different parts of the (possibly) composite PK is not an option, as I use tuple-comparison for the cursor in the case of composite PKs (stolen from here: https://stackoverflow.com/questions/68953290/sqlalchemy-query-with-where-with-comparison-on-tuple-of-multiple-columns).
5. I did not include [BUG] Pagination does not work properly #197 (comment) yet, as this could be a breaking change. Until now, sorting_columns in get_multi_joined are only allowed if they exist on the (not-joined) original model. If I introduce the discussed changes, It would suddenly be necessary for the user to for example set "test.name" as sorting column and not just "name" anymore. Even If I introduce a uniqueness check on the columns when applying the sorting, this could still break if for example the joined model also has a name attribute. I think this breaking change would be necessary though, as imo just supplying "name" is unclear to the user of the function if they join multiple tables with a name attribute (or is it stated somewhere, that they can only sort on the main tables attributes?) I think checking for unique columns and allowing sort_columns without the table name if the attribute is unique might be a good compromise between backwards compatibility and clearness.

Tests

All test stuff was equivalently added to sqlalchemy and sqlmodel.

• Added test_data_multipk_list function to conftest.py as I did not wanna touch test_data_multipk
• Added several more tests in crud/test_get_multi as well as endpoint/test_get_paginated to test the fixed Bug as well as the new cursor based pagination endpoint.

Checklist

• I have read the CONTRIBUTING document.
• My code follows the code style of this project.
• I have added necessary documentation (if appropriate).
• I have added tests that cover my changes (if applicable).
• All new and existing tests passed.

Additional Notes

Not done yet (have to check the code style stuff and add to the documentation). But wanted to get early feedback as well as open the discussion to the points of considerations.

[igorbenav]

@Simon128 is this ready for review?

[Simon128]

@Simon128 is this ready for review?

It's ready for review. What do you use to generate the github pages from the docs folder? Is there something I can install to serve them locally, to checkout my changes in the doc?

Also, be aware that a lot of files have been changed by the "ruff format" command. If you don't want this, we can just revert that single commit.

[igorbenav]

@Simon128 you can just install mkdocs material and run mkdocs serve. Tbh I should add this to the pyproject with a in a "docs" group.

[igorbenav]

Hey, @Simon128, great PR!

1. Maybe we could have a type mapping that handles some specific cases directly, and the fallback could be using python_type. We could even let the user specify this information
2. Very nice solution
3. Won't this break if some part of the cursor has commas? Maybe we could use json encoding or url safe encoding. Not sure how often this would happen though

def encode_cursor(cursor_values):
    """Encode cursor values safely for URL transmission"""
    cursor_json = json.dumps(cursor_values)
    encoded = base64.urlsafe_b64encode(cursor_json.encode()).decode()
    return encoded

def decode_cursor(cursor_string):
    """Decode a cursor from URL-safe Base64 to original values"""
    try:
        decoded_json = base64.urlsafe_b64decode(cursor_string.encode()).decode()
        return json.loads(decoded_json)
    except Exception as e:
        raise ValueError(f"Invalid cursor format: {e}")

4. I think adding a deprecation warning is a good approach to avoid redundancy, we can drop it in the next major release
5. I think we can go with an opt in approach, something like:
   1. Add a configuration parameter require_qualified_sort_columns defaulting to False
   2. When False (default):
      • Check if column names are unique across all tables
      • If unique, use the column without qualification
      • If not unique, issue a warning and use the column from the main table
      • The warning should explicitly state which table's column is being used
      • Suggest using a fully qualified name in future code for clarity
   3. When True:
      • Require fully qualified column names
      • Raise an exception for ambiguous column names

Then we eventually can make default True for require_qualified_sort_columns and after some time remove the parameter entirely. Of course at least one major version before each of these changes

[igorbenav]

Plus, since your at it, maybe take a look at this one as well