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

Bug: Calling exists() with filters doesn't work SQLAlchemy contrib #2221

Closed
1 of 4 tasks
nielsvanhooy opened this issue Aug 25, 2023 · 1 comment · Fixed by #2228
Closed
1 of 4 tasks

Bug: Calling exists() with filters doesn't work SQLAlchemy contrib #2221

nielsvanhooy opened this issue Aug 25, 2023 · 1 comment · Fixed by #2228
Assignees
Labels
Bug 🐛 This is something that is not working as expected

Comments

@nielsvanhooy
Copy link
Contributor

nielsvanhooy commented Aug 25, 2023

Description

Hi There.

i was testing something out and came across something of which i think is unintended.

in the file litestar/contrib/sqlalchemy/repository/_async.py specifically the function exists()

the function eventually calls self.count method which accepts filters.
however the function exists() doesn't accept *args ( or *filters in this case). (this is the same for the sync variant).
I tested it out with overriding this function and the behaviour that i was expecting seems to work then with filters.

signature of the function count() that exists() calls

    async def count(
        self,
        *filters: FilterTypes,
        statement: Select[tuple[ModelT]] | StatementLambdaElement | None = None,
        **kwargs: Any,
    ) -> int:

original code:

    async def exists(self, **kwargs: Any) -> bool:
        """Return true if the object specified by ``kwargs`` exists.

        Args:
            **kwargs: Identifier of the instance to be retrieved.

        Returns:
            True if the instance was found.  False if not found..

        """
        existing = await self.count(**kwargs)
        return existing > 0

overrided and working

    async def exists(self, *filters: FilterTypes, **kwargs: Any) -> bool:
        """Return true if the object specified by ``kwargs`` exists.

        Args:
            *filters: Types for specific filtering operations.
            **kwargs: Identifier of the instance to be retrieved.

        Returns:
            True if the instance was found.  False if not found..

        """
        existing = await self.count(*filters, **kwargs)
        return existing > 0

just wanted to know if was a bug/oversight for this functionality so that i can open a pull request.

URL to code causing the issue

No response

MCVE

# Your MCVE code here

Steps to reproduce

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Screenshots

"![SCREENSHOT_DESCRIPTION](SCREENSHOT_LINK.png)"

Logs

No response

Litestar Version

2.0

Platform

  • Linux
  • Mac
  • Windows
  • Other (Please specify in the description above)

Funding

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
Fund with Polar
@nielsvanhooy nielsvanhooy added Bug 🐛 This is something that is not working as expected Triage Required 🏥 This requires triage labels Aug 25, 2023
@cofin cofin removed the Triage Required 🏥 This requires triage label Aug 25, 2023
@cofin
Copy link
Member

cofin commented Aug 25, 2023

Thanks for reporting. A fix is incoming for this.

Edit - I missed the last part of your message. Please go ahead and submit a PR.

@cofin cofin assigned nielsvanhooy and unassigned cofin Aug 25, 2023
cofin pushed a commit that referenced this issue Aug 26, 2023
)

* fixed exists() method for sqlalchemy sync and async.
also changed the functions in the repository abc. and the generic_mock_repository.py. that one used get_one_or_none that doesn't seem right

* added tests that give a filter argument in the mock_repository and test_sqlalchemy.py.
 
---------

Co-authored-by: niels van hooij <niels.vanhooij@kpn.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐛 This is something that is not working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants