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.

Sign up for GitHub

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

Jump to bottom

Fix: iterate with get_next in handle_get_notes and handle_get_overrides #2203

Merged
merged 5 commits into from
Jun 14, 2024
Merged

Fix: iterate with get_next in handle_get_notes and handle_get_overrides #2203

merged 5 commits into from
Jun 14, 2024

Conversation

mattmundell
Copy link
Contributor

@mattmundell mattmundell commented May 28, 2024 •
edited
Loading

What

In handle_get_notes, loop using get_next instead of letting buffer_notes_xml do the looping with next.

Likewise for handle_get_overrides.

Why

Using get_next is consistent with the other GET handlers, like handle_get_targets.

get_next moves the iterator to the beginning if the filter keyword first is out of bounds. For example with a db containing 3 targets:

$ o m m '<get_targets filter="rows=1 first=3000000"/>'
<get_targets_response status="200" status_text="OK">
  <target id="0de4c2b4-987d-4a3b-9eba-fe92b3eaa8b2">...

Before this PR GET_NOTES would have an empty response:

$ o m m '<get_notes filter="rows=1 first=3000000"/>'
<get_notes_response status="200" status_text="OK">
  ...
  <note_count>5
  <filtered>5</filtered>
  <page>0</page>...

After this PR it behaves like the other commands:

$ o m m '<get_notes filter="rows=1 first=3000000"/>'
<get_notes_response status="200" status_text="OK">
  <note id="4ca00487-2c6a-4eeb-a645-dbebb6aa956a">...

Likewise for overrides.

References

This was a leftover TODO from 74263a8 in 2013.

@mattmundell mattmundell requested a review from a team as a code owner May 28, 2024 20:53
@github-actions GitHub Actions
Copy link

Conventional Commits Report

Type Number
Bug Fixes 2

🚀 Conventional commits found.

@mattmundell mattmundell mentioned this pull request May 29, 2024
Closed
@mattmundell @a-h-abdelsalam
Move body of buffer_notes_xml to new function buffer_note_xml
df92297
@mattmundell @a-h-abdelsalam
Remove const from get_next initializer arg
8e61336 
Some of the GET iterator initializers modify their get_data_t arg, so
this allows these initializers to be passed to get_next.

In particular this allows init_note_iterator_all to be passed to
get_next, for a fix to handle_get_notes.  Note that
init_note_iterator_all itself does not modify its get_data_t arg, but
init_note_iterator_all is passed to select_resource_iterator which
also accepts initializers that do modify their get_data_t args.

As a result drop the const from any initializer passed to get_next.
@mattmundell @a-h-abdelsalam
Fix: iterate with get_next in handle_get_notes
e238c71
@mattmundell @a-h-abdelsalam
Move body of buffer_overrides_xml to new function buffer_override_xml
7751b12
@mattmundell @a-h-abdelsalam
Fix: iterate with get_next in handle_get_overrides
26d9b4d
@a-h-abdelsalam a-h-abdelsalam merged commit 7e50546 into main Jun 14, 2024
11 checks passed
@a-h-abdelsalam a-h-abdelsalam deleted the get-notes-with-get-next branch June 14, 2024 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@a-h-abdelsalam a-h-abdelsalam a-h-abdelsalam approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mattmundell @a-h-abdelsalam