Skip to content

FIX: Copy the owid in the FodId constructor, not just fromOwid#170

Merged
oleksandrlazarenko-pi merged 1 commit into
mainfrom
fix/fodid-constructor-immutability
Jul 8, 2026
Merged

FIX: Copy the owid in the FodId constructor, not just fromOwid#170
oleksandrlazarenko-pi merged 1 commit into
mainfrom
fix/fodid-constructor-immutability

Conversation

@oleksandrlazarenko-pi

Copy link
Copy Markdown
Contributor

Follow-up to the 51Did reader (#166), addressing a review point from Eugene on the PHP port (pipeline-php-did#2) and propagating the fix for cross-port consistency (per the review guidelines).

Problem

FodId.fromOwid already copies the owid, but the public constructor new FodId(owidInstance) still aliased the caller's instance (this._owid = owidInstance). An owid instance is mutable, so new FodId(o) followed by mutating o reintroduced exactly the cached-hash vs live-payload desync that fromOwid was fixed to prevent. Java already avoids this (its constructor is private and Owid is final); Node and Python shared the gap.

Fix

Copy the owid in the constructor (re-parse from its base64), so every construction path is decoupled from the caller's instance:

this._owid = new owid(owidInstance.data);

fromOwid now simply delegates to the constructor (single copy site).

Test

Adds a constructor is decoupled from the source owid test (mirrors the existing fromOwid decouple test, but via the public constructor). Full suite: 33/33 pass.

@oleksandrlazarenko-pi oleksandrlazarenko-pi merged commit d8c0f74 into main Jul 8, 2026
1 check passed
@oleksandrlazarenko-pi oleksandrlazarenko-pi deleted the fix/fodid-constructor-immutability branch July 8, 2026 16:09
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.

2 participants