Skip to content

Add missing __len__() implementation for IterableQueryMixin to fix len() on Alert query#242

Merged
abowersox-cb merged 1 commit into
carbonblack:developfrom
crahan:alertquery_len_fix
Jul 13, 2020
Merged

Add missing __len__() implementation for IterableQueryMixin to fix len() on Alert query#242
abowersox-cb merged 1 commit into
carbonblack:developfrom
crahan:alertquery_len_fix

Conversation

@crahan
Copy link
Copy Markdown
Contributor

@crahan crahan commented Jul 6, 2020

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Tests have been added that prove the fix is effective or that the feature works.
  • New and existing tests pass locally with the changes.
  • Code follows the style guidelines of this project (PEP8, clean code).
  • Linter has passed locally and any fixes were made for failures.
  • A self-review of the code has been done.

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes (not tied to bugs/features)
  • Other (please describe):

What is the ticket or issue number?

  • Ticket Number: N/A
  • Issue Number: N/A

Pull Request Description

Requesting the length of a BaseAlert query always returns 0. This is because the _count() function in BaseAlertSearchQuery is never called. __len__() in the IterableQueryMixin class it inherits from is not implemented and always returns a length of 0, causing this behavior.

This PR provides a fix by updating the __len__() function in IterableQueryMixin to call the _count() function. It also initializes self._count_valid and self._total_results in BaseAlertSearchQuery with default values (as self._count_valid is checked by _count() prior to making an API call for an initial count).

Looking at the implementation for _count() and __len__() for Query in cbapi/psc/threathunter/query.py and PaginatedQuery in cbapi/query.py the default values there are set in the PaginatedQuery class. For this fix I've initialized the variables in BaseAlertSearchQuery instead and not the IterableQueryMixin class it inherits from. If this needs to be changed (i.e. initialization moved to IterableQueryMixin), please let me know.

Does this introduce a breaking change?

  • Yes
  • No

How Has This Been Tested?

The following script has been used to test this PR:

from cbapi.psc import CbPSCBaseAPI
from cbapi.psc.models import BaseAlert
from datetime import datetime, time

cbth = CbPSCBaseAPI(profile='profile-name')
alerts1 = cbth.select(BaseAlert).where('alert_id:ZG5U3VVQ')
alerts2 = cbth.select(BaseAlert)

print(f'Results: {len(alerts1)}')
print(f'Results: {len(alerts2)}')

Which results in the following output:

❯ python test2.py
Results: 1
Results: 581353

@avanbrunt-cb avanbrunt-cb changed the base branch from master to develop July 13, 2020 22:12
@abowersox-cb abowersox-cb self-requested a review July 13, 2020 22:44
Copy link
Copy Markdown
Contributor

@abowersox-cb abowersox-cb left a comment

Choose a reason for hiding this comment

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

This one's straightforward.

@abowersox-cb abowersox-cb merged commit f06a1bc into carbonblack:develop Jul 13, 2020
@crahan crahan deleted the alertquery_len_fix branch July 14, 2020 12: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.

3 participants