Skip to content

CBG-4907: fix for panic in CheckProposedVersion for legacy revision case#7802

Merged
torcolvin merged 5 commits intomainfrom
CBG-4907
Oct 7, 2025
Merged

CBG-4907: fix for panic in CheckProposedVersion for legacy revision case#7802
torcolvin merged 5 commits intomainfrom
CBG-4907

Conversation

@gregns1
Copy link
Copy Markdown
Contributor

@gregns1 gregns1 commented Oct 7, 2025

CBG-4907

  • If we have no local HLV and parentRev doesn't match local current rev, we should return conflict with current rev as conflict rev for proposeChangesResponse
  • Test repros the panic if you take out my fix in CheckProposedVersion and run the test.
  • Test is skipped, given there is no conflict resolution for rev tree Ids in the blip tester client, I've had a go a doing this but running out of time to get it done properly and its not something I would want to rush, I will file a ticket to get this implemented and remove the skip for this test (I don't think this needs to go into 4.0.0 though)

Pre-review checklist

  • Removed debug logging (fmt.Print, log.Print, ...)
  • Logging sensitive data? Make sure it's tagged (e.g. base.UD(docID), base.MD(dbName))
  • Updated relevant information in the API specifications (such as endpoint descriptions, schemas, ...) in docs/api

Dependencies (if applicable)

  • Link upstream PRs
  • Update Go module dependencies when merged

Integration Tests

Copilot AI review requested due to automatic review settings October 7, 2025 17:25
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a panic in the CheckProposedVersion function when handling legacy revision cases where there's no local HLV (Hybrid Logical Vector) but the parent revision doesn't match the local current revision.

Key changes:

  • Added a condition to return conflict status with current revision when no HLV exists but revisions don't match
  • Updated test client conflict detection logic to handle revision tree IDs and missing HLV cases
  • Added a comprehensive test case to reproduce the panic scenario

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
db/crud.go Added null HLV check in CheckProposedVersion to prevent panic and return proper conflict status
rest/utilities_testing_blip_client.go Updated conflict detection method to accept revOptions and handle revision tree ID cases
rest/blip_api_crud_test.go Added test case reproducing the panic scenario in legacy revision handling

Comment thread rest/blip_api_crud_test.go Outdated
@torcolvin torcolvin merged commit 7282868 into main Oct 7, 2025
42 checks passed
@torcolvin torcolvin deleted the CBG-4907 branch October 7, 2025 20:31
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.

4 participants