Fix non-paginating entity finders, add CI, and release 0.2.1 - #8
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚪 Why?
Follow-up to the v0.2.0 release (#7). Auditing the gem after that merge turned up one
silent-wrong-answer bug in the newly-added entity finders, a latent
requiregap, and theabsence of any CI — which is the actual reason a sandbox
base_urlshipped inc09ac21andbroke
configuration_spec.rb:9unnoticed until release day.This PR fixes those and cleans up release metadata. It does not attempt the two known
architectural bugs (see Deferred below) — those are documented rather than silently carried.
No consumer needs to re-pin unless it starts using the subscription or custom-list-pagination
paths, neither of which is reachable from catalog provisioning today.
What changed
Fixed:
find_by_*searched only the first page (the real bug)find_by_custom_propertyandfind_by_integration_entity_idissued exactly one request andsearched
response['data']. The comment directly above them explains why that's unsafe:If the server drops the filter, the response is page 1 of the unfiltered list. Any entity
past that page returned
nil— indistinguishable from "does not exist". Anilhere reads as"not imported yet", so the failure mode is a duplicate create, not an exception.
Both finders now walk the full result set through a shared private
search_findhelper. Twoconstraints from the existing specs shaped it:
skipentirely rather than sendingskip=0. The finder specs stubwith exact-match
.with(query: …)and noskipkey, so emitting it would have broken all five.It also keeps the unpaged-search request shape byte-identical to before.
meta: { total: 1 }, notaken/skipped), sonext_offsetfalls back todata.sizeand terminates cleanly instead oflooping forever or raising on
nil. This also hardens the finder path against endpoints thatanswer with a non-standard envelope.
Three specs cover it: a match on page 2 for each finder, plus one asserting a single request when
meta omits the counters.
Fixed: missing
require 'json'BaseResource#parse_error_bodycallsJSON.parse, butjsonwas never required anywhere inlib/. It worked only because Faraday loads it transitively — a dependency change would haveturned the error-handling path into a
NameError. One line, on the path you hit precisely whensomething is already going wrong.
Fixed: gemspec metadata URLs
source_code_uriandchangelog_uripointed atgithub.com/hyperline/hyperline-ruby, whichdoes not exist — both 404 for anyone installing the gem. Now
factorialco/hyperline-cli.Added: CI
.github/workflows/ci.ymlrunsrspec+rubocopon Ruby 3.3.5 (matching.tool-versions) forpushes to
mainand all PRs. The repo had no CI at all. This is the highest-leverage change here:it is exactly what would have caught the v0.2.0
base_urlregression at PR time.Release hygiene
0.2.1entry and the0.2.0entry that was never written — thefive resources added in Feat/adding bundles #7 were undocumented. Plus a Known issues section recording the
deferred bugs.
.gitignorenow covers*.gem,/pkg/, bundler and lint caches — it previously had twolines, which is how a build artifact got committed.
0.2.1,Gemfile.lockregenerated.Deferred, deliberately
Both are real and both are documented under Known issues in the CHANGELOG rather than quietly
carried. Neither is reachable from catalog provisioning, which is the only consumer today:
Subscriptions#listhits v1 while#get/#updateand search use v2.BaseResource#listbuilds from
resource_path, notsearch_path, so it ignores the v2 override — despite twocomments in the code stating that listing subscriptions is a v2 operation. Fixing it means
deciding whether the comments or the code are wrong, and
subscriptions_spec.rb:11currentlyasserts the v1 path, so a fix has to change a passing test. Bites whenever
subscription-management code lands.
Collection#next_pagealways re-issues the resource's default#list. So paginating acustom list method —
Subscriptions#list_templatesis the only one today — returns page 1 oftemplates and every subsequent page of subscriptions. The fix is for
Collectionto capturethe path or a callable instead of the resource, which is a wider change than this PR's scope.
Standard
.listpagination is unaffected:products.listandcustom_properties.listbothre-page their own
base_pathcorrectly.Out of scope but required
lib/hyperline/errors.rb— adding rubocop to CI with 5 standing offenses means CI fails onits first run. Four were blank lines between error classes. For the fifth (a nested ternary),
rubocop's autocorrect produced an unreadable multi-line
ifnested inside a||, so Iextracted
ErrorMapper.error_class_forby hand. Logic is equivalent —MAPPED[nil]was alreadynil, so thestatus.nil?guard behaves identically.hyperline-0.1.0.gem— a committed build artifact. Gitignoring*.gemwhileleaving a tracked 0.1.0 file beside a 0.2.1 release is half a fix. Easy to drop if you'd rather
keep it.
Testing
bundle exec rspec→ 71 examples, 0 failures (68 before; +3 for the pagination fix)bundle exec rubocop→ 26 files, no offenses (5 before)Gem::Specification.load→hyperline 0.2.1, metadata URLs corrected,rubygems_mfa_requiredstill"true"ci.ymlparses as valid YAMLRequires a
v0.2.1tag onmainafter merge.