Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for UUIDs in PHP #2407

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pengpixel
Copy link

For PHP the UUID is already mapped as a string, but had no implementation yet and threw an error.
If you have a schema which includes UUIDs and want to generate code for e.g. C# and PHP, the latter will fail. Giving you the choice to either maintain 2 schema files or just use strings for UUID values.

So this PR will:

  • keep the string mapping and resolve the todo by validating the value with a regex, as PHP does not have built-in support for UUIDs

pengpixel and others added 3 commits September 18, 2023 14:05
* UUID is already mapped as a string,
  but had no implementation yet

* as PHP does not have built-in support for UUIDs,
  only validate UUID strings with a regex
tangowithfoxtrot added a commit to bitwarden/sdk that referenced this pull request Sep 18, 2024
## 🎟️ Tracking

https://bitwarden.atlassian.net/browse/SM-1402

## 📔 Objective

Update PHP bindings in accordance with our other wrappers. This renames
any "put" methods to "update", refactors `access_token_login` to
`auth().login_access_token`, re-orders function args for `create` and
`update`, and adds secret syncing.

This update required quite a few changes to the schemas. However, since
we cannot auto-generate them with `quicktype` (see the error referenced
in glideapps/quicktype/pull/2407), schemas were generated with the
swaggest/json-cli:

```sh
json-cli gen-php ../../support/schemas/schema_types/SchemaTypes.json --ns '\Bitwarden\Sdk\Schemas' --ns-path ./src/schemas/
```

The generated schemas still required hand modification to get
human-readable class names for things like `ProjectCommand`,
`SecretCommand`, etc.

To validate the changes, I've run the `example.php` file after updating
the schemas.

## ⏰ Reminders before review

- Contributor guidelines followed
- All formatters and local linters executed and passed
- Written new unit and / or integration tests where applicable
- Protected functional changes with optionality (feature flags)
- Used internationalization (i18n) for all UI strings
- CI builds passed
- Communicated to DevOps any deployment requirements
- Updated any necessary documentation (Confluence, contributing docs) or
informed the documentation
  team

## 🦮 Reviewer guidelines

<!-- Suggested interactions but feel free to use (or not) as you desire!
-->

- 👍 (`:+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 confirmed
  issue 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

---------

Co-authored-by: Maciej Zieniuk <[email protected]>
Co-authored-by: vphan916 <[email protected]>
Co-authored-by: Maciej Zieniuk <[email protected]>
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