From de1cf7b06fabe7dd4aa89563b97762d40407d494 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Janou=C5=A1ek?= Date: Thu, 24 Sep 2026 00:24:34 +0200 Subject: [PATCH 1/3] api: align assignment and user-data cursors with deterministic order Continue IP history after the scoped cursor's (from_date, id) tuple in the requested direction. Reject unavailable cursors instead of returning a misleading terminal page. Order user-data explicitly by ascending ID. Cover tied and nonmonotonic dates, owner filters, terminal pages and invalid cursors with API specs. Keep this proposal local pending approval to open an upstream PR; no shared API or database changes are included. --- .../api/resources/ip_address_assignment.rb | 25 +++-- .../vpsadmin/api/resources/vps_user_data.rb | 4 +- .../resources/ip_address_assignment_spec.rb | 91 ++++++++++++++++++- api/spec/api/resources/vps_user_data_spec.rb | 28 ++++++ 4 files changed, 137 insertions(+), 11 deletions(-) diff --git a/api/lib/vpsadmin/api/resources/ip_address_assignment.rb b/api/lib/vpsadmin/api/resources/ip_address_assignment.rb index eb2515309..6652e9379 100644 --- a/api/lib/vpsadmin/api/resources/ip_address_assignment.rb +++ b/api/lib/vpsadmin/api/resources/ip_address_assignment.rb @@ -25,6 +25,10 @@ class Index < HaveAPI::Actions::Default::Index desc 'List IP address assignments' input do + patch :from_id, + desc: 'Continue after the last assignment from the previous page. Keep the same filters and order. ' \ + 'Assignments are ordered by from_date and ID in the selected direction. ' \ + 'An unavailable or differently scoped cursor returns HTTP 400.' use :all, include: %i[ ip_address ip_addr @@ -109,15 +113,22 @@ def count def exec q = query - - case input[:order] - when 'newest' - q = q.order('ip_address_assignments.from_date DESC') - when 'oldest' - q = q.order('ip_address_assignments.from_date ASC') + ascending = input[:order] == 'oldest' + comparison = ascending ? '>' : '<' + direction = ascending ? 'ASC' : 'DESC' + + q = ar_with_pagination(q, check: true) do |scope, from_id| + cursor_date = q.where(id: from_id).pick(:from_date) + error!('Invalid pagination cursor', {}, http_status: 400) unless cursor_date + + scope.where( + "ip_address_assignments.from_date #{comparison} ? OR " \ + "(ip_address_assignments.from_date = ? AND ip_address_assignments.id #{comparison} ?)", + cursor_date, cursor_date, from_id + ) end - with_pagination(q) + q.order("ip_address_assignments.from_date #{direction}, ip_address_assignments.id #{direction}") end end diff --git a/api/lib/vpsadmin/api/resources/vps_user_data.rb b/api/lib/vpsadmin/api/resources/vps_user_data.rb index 0f9a2c1af..851df88fe 100644 --- a/api/lib/vpsadmin/api/resources/vps_user_data.rb +++ b/api/lib/vpsadmin/api/resources/vps_user_data.rb @@ -18,7 +18,7 @@ class VpsUserData < HaveAPI::Resource end class Index < HaveAPI::Actions::Default::Index - desc 'List VPS user data' + desc 'List VPS user data ordered by ascending ID' input do use :all, include: %i[user format] @@ -50,7 +50,7 @@ def count end def exec - with_pagination(query) + with_pagination(query.order(:id)) end end diff --git a/api/spec/api/resources/ip_address_assignment_spec.rb b/api/spec/api/resources/ip_address_assignment_spec.rb index 31911fd8f..337699474 100644 --- a/api/spec/api/resources/ip_address_assignment_spec.rb +++ b/api/spec/api/resources/ip_address_assignment_spec.rb @@ -457,11 +457,98 @@ def expect_status(code) expect_status(200) expect(assignments.length).to eq(2) - boundary = assignment_user_active_v4.id + boundary = assignment_user_active_v6.id as(SpecSeed.admin) { json_get index_path, ip_address_assignment: { from_id: boundary } } expect_status(200) - expect(assignment_ids).to all(be > boundary) + expected = [ + assignment_other_active_v6.id, + assignment_user_inactive_v4.id, + assignment_user_active_v4.id + ] + expect(assignment_ids).to eq(expected) + end + + context 'with a chronological cursor' do + let(:cursor_rows) do + base = Time.utc(2026, 1, 1) + [40, 10, 60, 30, 60, 20, 50, 10, 70].map do |offset| + create_assignment!( + ip: ip_v4_primary, + user: SpecSeed.user, + vps: vps_user, + from_date: base + offset, + to_date: base + 100 + ) + end + end + + %w[newest oldest].each do |order| + it "traverses tied/nonmonotonic dates exactly once in #{order} order" do + expected = cursor_rows.sort_by { |row| [row.from_date, row.id] } + expected.reverse! if order == 'newest' + seen = [] + cursor = nil + + 6.times do + input = { order: order, limit: 2, ip_addr: '192.0.2.10', active: false, + user: SpecSeed.user.id, vps: vps_user.id } + input[:from_id] = cursor if cursor + as(SpecSeed.admin) { json_get index_path, ip_address_assignment: input } + expect_status(200) + ids = assignment_ids + break if ids.empty? + + expect(ids & seen).to be_empty + seen.concat(ids) + cursor = ids.last + end + + expect(seen).to eq(expected.map(&:id)) + expect(assignment_ids).to be_empty + end + end + + it 'rejects unavailable or differently scoped cursors without returning rows' do + cursor_rows + [assignment_other_active_v6.id, IpAddressAssignment.maximum(:id) + 100].each do |cursor| + as(SpecSeed.user) do + json_get index_path, ip_address_assignment: { from_id: cursor } + end + expect_status(400) + expect(json['status']).to be(false) + end + + as(SpecSeed.admin) do + json_get index_path, ip_address_assignment: { + from_id: cursor_rows.first.id, active: true + } + end + expect_status(400) + expect(json['status']).to be(false) + end + + it 'keeps member scope across cursor pages and an exact terminal boundary' do + expected = cursor_rows.sort_by { |row| [row.from_date, row.id] }.reverse + seen = [] + cursor = nil + + 4.times do + input = { limit: 3, ip_addr: '192.0.2.10', active: false } + input[:from_id] = cursor if cursor + as(SpecSeed.user) { json_get index_path, ip_address_assignment: input } + expect_status(200) + assignments.each do |row| + expect(row).not_to have_key('user') + expect(row).not_to have_key('raw_user_id') + end + seen.concat(assignment_ids) + cursor = assignment_ids.last + end + + expect(seen).to eq(expected.map(&:id)) + expect(assignment_ids).to be_empty + end end it 'returns total_count meta when requested' do diff --git a/api/spec/api/resources/vps_user_data_spec.rb b/api/spec/api/resources/vps_user_data_spec.rb index 461bdbd06..84091b45c 100644 --- a/api/spec/api/resources/vps_user_data_spec.rb +++ b/api/spec/api/resources/vps_user_data_spec.rb @@ -281,6 +281,34 @@ def nixos_flake_uri_content expect_status(200) expect(json.dig('response', '_meta', 'total_count')).to eq(VpsUserData.count) end + + it 'walks scoped ID pages despite nonmonotonic timestamps' do + base = Time.utc(2026, 1, 1) + expected = [40, 10, 60, 30, 60, 20, 50].map do |offset| + row = create_user_data!(user: SpecSeed.user, label: "Page #{offset}", format: 'script', + content: script_content) + row.update_columns(created_at: base + offset, updated_at: base - offset) + row.id + end + create_user_data!(user: SpecSeed.other_user, label: 'Foreign', format: 'script', content: script_content) + seen = [] + cursor = nil + + 5.times do + input = { limit: 2, format: 'script' } + input[:from_id] = cursor if cursor + as(SpecSeed.user) { json_get index_path, vps_user_data: input } + expect_status(200) + ids = list.map { |row| row['id'] } + break if ids.empty? + + seen.concat(ids) + cursor = ids.last + end + + expect(seen).to eq(expected) + expect(list).to be_empty + end end describe 'Show' do From ee81404cad15b299eced907676bbd2c98d825d9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Janou=C5=A1ek?= Date: Thu, 24 Sep 2026 03:51:15 +0200 Subject: [PATCH 2/3] api: align dataset cursors with visible ordering Resolve cursor anchors inside authorized filtered queries and continue by name/time plus ID, preventing omitted and repeated rows across pages. Deduplicate pool joins and reject invalid anchors consistently. Qualify history date filters when resolving anchors through included properties. Cover nonmonotonic values, tied timestamps, scope boundaries, invalid anchors and terminal pages using actual API resource specs. --- api/lib/vpsadmin/api/resources/dataset.rb | 47 +++++++++++++--- .../dataset_property_history_spec.rb | 49 +++++++++++++++++ api/spec/api/resources/dataset_read_spec.rb | 55 +++++++++++++++++++ .../api/resources/dataset_snapshot_spec.rb | 47 ++++++++++++++++ 4 files changed, 191 insertions(+), 7 deletions(-) diff --git a/api/lib/vpsadmin/api/resources/dataset.rb b/api/lib/vpsadmin/api/resources/dataset.rb index 9e0f64561..96118a1ce 100644 --- a/api/lib/vpsadmin/api/resources/dataset.rb +++ b/api/lib/vpsadmin/api/resources/dataset.rb @@ -3,6 +3,31 @@ class Dataset < HaveAPI::Resource desc 'Manage datasets' model ::Dataset + # Keep each endpoint's visible order aligned with its scoped ID anchor. + # A renamed dataset or concurrent write requires a fresh traversal. + module OrderedCursor + protected + + def with_ordered_cursor(query, column:, descending: false) + table = query.klass.arel_table + value = table[column] + id = table[:id] + comparison = descending ? :lt : :gt + direction = descending ? :desc : :asc + + scope = ar_with_pagination(query, check: true) do |rows, from_id| + anchor = query.where(id.eq(from_id)).pick(value) + error!('Invalid pagination cursor', {}, http_status: 400) if anchor.nil? + + rows.where(value.public_send(comparison, anchor).or( + value.eq(anchor).and(id.public_send(comparison, from_id)) + )) + end + + scope.reorder(value.public_send(direction), id.public_send(direction)) + end + end + params(:id) do id :id end @@ -37,6 +62,8 @@ class Dataset < HaveAPI::Resource end class Index < HaveAPI::Actions::Default::Index + include OrderedCursor + desc 'List datasets' input do @@ -81,7 +108,7 @@ def query q = q.where(vps: input[:vps]) if input.has_key?(:vps) q = q.to_depth(input[:to_depth]) if input[:to_depth] - q + q.distinct end def count @@ -91,10 +118,11 @@ def count def exec ret = [] - with_pagination(query.includes( + q = query.includes( :dataset_properties, dataset_in_pools: [{ pool: [{ node: [{ location: [:environment] }] }] }] - ).order('full_name')).each do |ds| + ) + with_ordered_cursor(q, column: :full_name).each do |ds| ret << ds end @@ -489,6 +517,8 @@ class Snapshot < HaveAPI::Resource end class Index < HaveAPI::Actions::Default::Index + include OrderedCursor + desc 'List snapshots' input do @@ -516,7 +546,7 @@ def count end def exec - with_pagination(query.order('created_at')) + with_ordered_cursor(query, column: :created_at) end end @@ -850,6 +880,8 @@ class PropertyHistory < HaveAPI::Resource end class Index < HaveAPI::Actions::Default::Index + include OrderedCursor + input do datetime :from datetime :to @@ -876,8 +908,9 @@ def query q = ::DatasetPropertyHistory.includes(:dataset_property).where( dataset_property_id: props.pluck(:id) ) - q = q.where('created_at >= ?', input[:from]) if input[:from] - q = q.where('created_at <= ?', input[:to]) if input[:to] + created_at = ::DatasetPropertyHistory.arel_table[:created_at] + q = q.where(created_at.gteq(input[:from])) if input[:from] + q = q.where(created_at.lteq(input[:to])) if input[:to] q end @@ -886,7 +919,7 @@ def count end def exec - with_pagination(query.order('created_at DESC')) + with_ordered_cursor(query, column: :created_at, descending: true) end end diff --git a/api/spec/api/resources/dataset_property_history_spec.rb b/api/spec/api/resources/dataset_property_history_spec.rb index 9965d7ac3..42c36304c 100644 --- a/api/spec/api/resources/dataset_property_history_spec.rb +++ b/api/spec/api/resources/dataset_property_history_spec.rb @@ -74,6 +74,55 @@ def create_history(property:, value:, created_at:) end describe 'Index' do + it 'rejects ordered cursor anchors excluded by name, date or dataset scope' do + excluded_name = create_history(property: quota_prop, value: 10, created_at: Time.utc(2024, 1, 1)) + excluded_date = create_history(property: used_prop, value: 10, created_at: Time.utc(2024, 1, 1)) + deleted = create_history(property: used_prop, value: 10, created_at: Time.utc(2024, 1, 1)) + deleted_id = deleted.id + deleted.destroy! + foreign_ds, = create_dataset_with_pool!(user: other_user, pool: pool, name: 'cursor-foreign') + foreign = create_history(property: foreign_ds.dataset_properties.find_by!(name: 'used'), + value: 10, created_at: Time.utc(2024, 1, 3)) + + [2_147_483_647, deleted_id, excluded_name.id, foreign.id].each do |cursor| + as(user) { json_get property_history_path(dataset.id), property_history: { from_id: cursor, name: 'used' } } + expect_status(400) + expect(json['status']).to be(false) + end + + as(user) do + json_get property_history_path(dataset.id), property_history: { + from_id: excluded_date.id, from: Time.utc(2024, 1, 2).iso8601 + } + end + expect_status(400) + end + + %i[member admin].each do |role| + it "traverses ordered cursor pages by descending timestamp and ID for #{role}" do + rows = [2, 5, 1, 5, 3, 4].map do |day| + create_history(property: used_prop, value: day, created_at: Time.utc(2024, 1, day)) + end + expected = rows.sort_by { |row| [row.created_at, row.id] }.reverse.map(&:id) + actor = role == :admin ? SpecSeed.admin : user + collected = [] + cursor = nil + + 4.times do + params = { limit: 2, name: 'used', from: Time.utc(2024, 1, 1).iso8601, + to: Time.utc(2024, 1, 5).iso8601 } + params[:from_id] = cursor if cursor + as(actor) { json_get property_history_path(dataset.id), property_history: params } + expect_status(200) + ids = history_rows.map { |row| row['id'] } + collected.concat(ids) + cursor = ids.last unless ids.empty? + end + + expect(history_rows).to be_empty + expect(collected).to eq(expected) + end + end it 'rejects unauthenticated access' do json_get property_history_path(dataset.id) diff --git a/api/spec/api/resources/dataset_read_spec.rb b/api/spec/api/resources/dataset_read_spec.rb index 9f3d4a3fb..2b5e9addf 100644 --- a/api/spec/api/resources/dataset_read_spec.rb +++ b/api/spec/api/resources/dataset_read_spec.rb @@ -88,6 +88,61 @@ def with_current_user(user) end describe 'Index' do + it 'rejects ordered cursor anchors outside the authorized filtered dataset query' do + [2_147_483_647, other_dataset.id].each do |cursor| + as(user) { json_get datasets_path, dataset: { from_id: cursor } } + expect_status(400) + expect(json['status']).to be(false) + end + + as(user) do + json_get datasets_path, dataset: { from_id: user_dataset.id, role: 'hypervisor' } + end + expect_status(400) + + as(SpecSeed.admin) do + json_get datasets_path, dataset: { from_id: other_dataset.id, user: user.id } + end + expect_status(400) + end + + it 'deduplicates ordered cursor rows with multiple matching pools' do + second_pool = pool.dup + second_pool.assign_attributes(label: 'cursor-secondary', filesystem: 'cursor-secondary') + second_pool.save! + DatasetInPool.create!(dataset: user_dataset, pool: second_pool, confirmed: DatasetInPool.confirmed(:confirmed)) + + as(user) { json_get datasets_path, dataset: { limit: 2 } } + expect_status(200) + expect(datasets.map { |row| row['id'] }).to eq([user_dataset.id]) + end + + %i[member admin].each do |role| + it "traverses ordered cursor pages by name for #{role}" do + user_dataset.update!(name: 'zz-cursor-root') + rows = %w[z b d a c].map do |name| + create_dataset_with_pool!(user: user, pool: pool, name: "cursor-#{name}").first + end + expected = (rows + [user_dataset]).sort_by { |row| [row.full_name, row.id] }.map(&:id) + actor = role == :admin ? SpecSeed.admin : user + collected = [] + cursor = nil + + 4.times do + params = { limit: 2 } + params[:user] = user.id if role == :admin + params[:from_id] = cursor if cursor + as(actor) { json_get datasets_path, dataset: params } + expect_status(200) + ids = datasets.map { |row| row['id'] } + collected.concat(ids) + cursor = ids.last unless ids.empty? + end + + expect(datasets).to be_empty + expect(collected).to eq(expected) + end + end it 'rejects unauthenticated access' do json_get datasets_path diff --git a/api/spec/api/resources/dataset_snapshot_spec.rb b/api/spec/api/resources/dataset_snapshot_spec.rb index 96fd7e1ca..171b6eaec 100644 --- a/api/spec/api/resources/dataset_snapshot_spec.rb +++ b/api/spec/api/resources/dataset_snapshot_spec.rb @@ -86,6 +86,53 @@ def expect_status(code) end describe 'Index' do + it 'rejects ordered cursor anchors from missing, deleted, foreign or different datasets' do + foreign_ds, foreign_dip = create_dataset_with_pool!(user: other_user, pool: pool, name: 'cursor-foreign') + foreign, = create_snapshot!(dataset: foreign_ds, dip: foreign_dip) + own, = create_snapshot!(dataset: dataset, dip: dip) + deleted, deleted_sip = create_snapshot!(dataset: dataset, dip: dip) + deleted_id = deleted.id + deleted_sip.destroy! + deleted.destroy! + + [2_147_483_647, foreign.id, deleted_id].each do |cursor| + as(user) { json_get snapshots_path(dataset.id), snapshot: { from_id: cursor } } + expect_status(400) + expect(json['status']).to be(false) + end + + as(other_user) { json_get snapshots_path(dataset.id), snapshot: { from_id: own.id } } + expect_status(400) + as(SpecSeed.admin) { json_get snapshots_path(foreign_ds.id), snapshot: { from_id: own.id } } + expect_status(400) + end + + %i[member admin].each do |role| + it "traverses ordered cursor pages by timestamp and ID for #{role}" do + rows = [5, 1, 3, 1, 4, 2].map do |day| + snap, = create_snapshot!(dataset: dataset, dip: dip) + snap.update!(created_at: Time.utc(2024, 1, day)) + snap + end + expected = rows.sort_by { |row| [row.created_at, row.id] }.map(&:id) + actor = role == :admin ? SpecSeed.admin : user + collected = [] + cursor = nil + + 4.times do + params = { limit: 2 } + params[:from_id] = cursor if cursor + as(actor) { json_get snapshots_path(dataset.id), snapshot: params } + expect_status(200) + ids = snapshots.map { |row| row['id'] } + collected.concat(ids) + cursor = ids.last unless ids.empty? + end + + expect(snapshots).to be_empty + expect(collected).to eq(expected) + end + end it 'rejects unauthenticated access' do json_get snapshots_path(dataset.id) From 320af0e152ed223bf0365e0f1cf4b38cf00d7b1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Janou=C5=A1ek?= Date: Thu, 24 Sep 2026 20:37:30 +0200 Subject: [PATCH 3/3] api: normalize ordered cursor translations Separate IP assignment and payment cursor descriptions after changing the assignment contract. Regenerate both catalogs and retain the Czech payment translation so API i18n health checks pass. --- api/lib/vpsadmin/api/locales/cs.yml | 15 ++++++++++----- api/lib/vpsadmin/api/locales/en.yml | 15 ++++++++++----- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/api/lib/vpsadmin/api/locales/cs.yml b/api/lib/vpsadmin/api/locales/cs.yml index 566a287f2..e15716869 100644 --- a/api/lib/vpsadmin/api/locales/cs.yml +++ b/api/lib/vpsadmin/api/locales/cs.yml @@ -814,11 +814,6 @@ cs: from_duration: description: Stránkovat podle trvání label: Od trvání - from_id: - description: Pokračovat za platbou se zadaným ID v pořadí od nejnovějších - podle času vytvoření. Použij ID poslední platby z předchozí stránky a zachovej - stejné filtry. Pokud platba není dostupná nebo neodpovídá filtrům, vrátí - se prázdný seznam. from_personal: description: Odečíst přidané zdroje z osobního balíčku label: Z osobního balíčku @@ -2807,6 +2802,11 @@ cs: attributes: created_at: label: Vytvořeno + from_id: + description: Pokračovat za posledním přiřazením z předchozí stránky. Zachovej + stejné filtry a pořadí. Přiřazení jsou řazena podle from_date a ID ve + zvoleném směru. Nedostupný cursor nebo cursor mimo zvolený rozsah vrátí + HTTP 400. id: label: ID ip_addr: @@ -3930,6 +3930,11 @@ cs: attributes: created_at: label: Vytvořeno + from_id: + description: Pokračovat za platbou se zadaným ID v pořadí od nejnovějších + podle času vytvoření. Použij ID poslední platby z předchozí stránky + a zachovej stejné filtry. Pokud platba není dostupná nebo neodpovídá + filtrům, vrátí se prázdný seznam. id: label: ID user_request: diff --git a/api/lib/vpsadmin/api/locales/en.yml b/api/lib/vpsadmin/api/locales/en.yml index 10b42838e..0d9c91108 100644 --- a/api/lib/vpsadmin/api/locales/en.yml +++ b/api/lib/vpsadmin/api/locales/en.yml @@ -815,11 +815,6 @@ en: from_duration: description: Paginate by duration label: From_duration - from_id: - description: Continue after this payment, ordered by creation time from newest - to oldest. Use the last payment ID from the previous page and keep the same - filters. Returns an empty list if the payment is unavailable or does not - match the filters. from_personal: description: Substract the added resources from the personal package label: From personal package @@ -2812,6 +2807,11 @@ en: attributes: created_at: label: Created_at + from_id: + description: Continue after the last assignment from the previous page. + Keep the same filters and order. Assignments are ordered by from_date + and ID in the selected direction. An unavailable or differently scoped + cursor returns HTTP 400. id: label: ID ip_addr: @@ -3933,6 +3933,11 @@ en: attributes: created_at: label: Created_at + from_id: + description: Continue after this payment, ordered by creation time from + newest to oldest. Use the last payment ID from the previous page and + keep the same filters. Returns an empty list if the payment is unavailable + or does not match the filters. id: label: Id user_request: