chore: update sdk-internal #1370
Conversation
|
Great job! No new security vulnerabilities introduced in this pull request |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1370 +/- ##
==========================================
- Coverage 15.42% 15.34% -0.08%
==========================================
Files 21 21
Lines 1193 1199 +6
==========================================
Hits 184 184
- Misses 1009 1015 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
schemars >1.x uses https://json-schema.org/draft/2020-12/schema instead of https://json-schema.org/draft-07/schema#. I'm not exactly sure why, but something about this seems to have changed our schema output to use names like Response7 instead of ResponseForSecretsResponse, which breaks the language bindings that rely on them. Introduce a manual JsonSchema impl that restores the SchemaTypes.json output to what it was before
foldhash v0.1.5
└── hashbrown v0.15.5
└── hashlink v0.10.0
└── rusqlite v0.37.0
└── bitwarden-state v1.0.0
└── bitwarden-core v1.0.0
├── bitwarden v1.0.0
│ └── bws 1.0.0
├── bitwarden-generators v1.0.0
│ └── bitwarden 1.0.0
└── bitwarden-sm v1.0.0
└── bitwarden v1.0.0
| "Unicode-DFS-2016", | ||
| "Unicode-3.0", | ||
| "OpenSSL", | ||
| "Zlib", |
There was a problem hiding this comment.
The need for this comes from bitwarden-state, which has a direct dependency on rusqlite, which has an indirect dependency on foldhash, which is Zlib-licensed:
foldhash v0.1.5
└── hashbrown v0.15.5
└── hashlink v0.10.0
└── rusqlite v0.37.0
└── bitwarden-state v1.0.0
└── bitwarden-core v1.0.0
├── bitwarden v1.0.0
│ └── bws 1.0.0
├── bitwarden-generators v1.0.0
│ └── bitwarden 1.0.0
└── bitwarden-sm v1.0.0
└── bitwarden v1.0.0
we can no longer build our test suite with the --release profile because of: bitwarden/sdk-internal@de03567
removing this meant that maturin wasn't being installed before running the tests...
| pub data: Option<T>, | ||
| } | ||
|
|
||
| impl<T: Serialize + JsonSchema> JsonSchema for Response<T> { |
There was a problem hiding this comment.
This is far from ideal, but schemars 1.0 changed how schema generation is handled and it was breaking our wrappers. Without the manual JsonSchema impl, our generated schemas would contain unusable objects, like Response7 instead of ResponseForSecretsDeleteRequest etc.
The manual impl restores the generated schemas in such a way that does not break our language wrappers.
this avoids needing the manual JsonSchema impl
|
Setting this back to a draft until bitwarden/sdk-internal#562 is merged to fix the WASM builds. |
🎟️ Tracking
https://bitwarden.atlassian.net/browse/SM-1738
📔 Objective
Update sdk-internal crates. This, unfortunately required some pretty heavy, less-than-ideal changes, which I've added comments for throughout the PR.
⏰ Reminders before review
team
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmedissue and could potentially benefit from discussion
:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes