Add Cast for JSON #265
Add Cast for JSON #265
Conversation
WalkthroughThe pull request introduces a new "JSON Type" section in the README to document handling JSON columns, including a code snippet that demonstrates casting JSON attributes using the Changes
Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 PHPStan (2.0.3)/bin/bash: line 1: /vendor/bin/phpstan: No such file or directory ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/Casts/SpannerJson.php (1)
22-28: Add class and method documentation.Consider adding PHPDoc blocks to explain the purpose of the class and method.
+/** + * Custom JSON type for Google Cloud Spanner. + * Extends PgJsonb to provide JSON support with unspecified type annotation. + */ class SpannerJsonType extends PgJsonb { + /** + * Returns the type annotation code for JSON values. + * + * @return int + */ public function typeAnnotation() { return TypeAnnotationCode::TYPE_ANNOTATION_CODE_UNSPECIFIED; } }README.md (1)
237-247: Enhance JSON Type documentation.Consider adding more context about JSON type support:
- Purpose and benefits of using JSON columns
- Any limitations or performance considerations
- Example of reading/writing JSON data
### JSON Type -In order to use JSON columns, use the provided Cast on your model as below +Google Cloud Spanner supports storing and querying JSON data. To work with JSON columns in your Laravel models, use the provided `SpannerJson` cast as shown below: ```php use Colopl\Spanner\Casts\SpannerJson; protected $casts = [ 'json_column_name' => SpannerJson::class, ]
+#### Example Usage
+
+php +// Writing JSON data +$model->json_column_name = ['key' => 'value']; + +// Reading JSON data +$data = $model->json_column_name; // Returns array +
+
+#### Limitations and Considerations
+
+- JSON columns are stored as strings in the database
+- Consider indexing specific JSON fields if you need to query them frequently
+- Large JSON objects may impact performance<details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary> [style] ~238-~238: Consider a shorter alternative to avoid wordiness. Context: ...have these conversions. ### JSON Type In order to use JSON columns, use the provided Cast... (IN_ORDER_TO_PREMIUM) </details> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> 237-237: Heading style Expected: setext; Actual: atx (MD003, heading-style) </details> </details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 03dbf66b58566989e1eb77c1546dc615eeb32e1d and f671a3db1e2610b7ec454878daaabdc128d8369b. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `README.md` (1 hunks) * `src/Casts/SpannerJson.php` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 LanguageTool</summary> <details> <summary>README.md</summary> [style] ~238-~238: Consider a shorter alternative to avoid wordiness. Context: ...have these conversions. ### JSON Type In order to use JSON columns, use the provided Cast... (IN_ORDER_TO_PREMIUM) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>README.md</summary> 237-237: Heading style Expected: setext; Actual: atx (MD003, heading-style) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms (1)</summary> * GitHub Check: Run-PHPUnit </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| public function get($model, $key, $value, $attributes) | ||
| { | ||
| return json_decode((string) $value, true); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for JSON decoding.
The get method should handle potential JSON decoding errors and null values explicitly.
public function get($model, $key, $value, $attributes)
{
- return json_decode((string) $value, true);
+ if ($value === null) {
+ return null;
+ }
+ $decoded = json_decode((string) $value, true);
+ if (json_last_error() !== JSON_ERROR_NONE) {
+ throw new \JsonException(json_last_error_msg());
+ }
+ return $decoded;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function get($model, $key, $value, $attributes) | |
| { | |
| return json_decode((string) $value, true); | |
| } | |
| public function get($model, $key, $value, $attributes) | |
| { | |
| if ($value === null) { | |
| return null; | |
| } | |
| $decoded = json_decode((string) $value, true); | |
| if (json_last_error() !== JSON_ERROR_NONE) { | |
| throw new \JsonException(json_last_error_msg()); | |
| } | |
| return $decoded; | |
| } |
| public function set($model, $key, $value, $attributes) | ||
| { | ||
| return [$key => new SpannerJsonType($value)]; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation for JSON values.
The set method should validate the input value to ensure it can be encoded as JSON.
public function set($model, $key, $value, $attributes)
{
+ if ($value !== null && !is_array($value) && !is_object($value)) {
+ throw new \InvalidArgumentException('The given value must be array, object or null.');
+ }
return [$key => new SpannerJsonType($value)];
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function set($model, $key, $value, $attributes) | |
| { | |
| return [$key => new SpannerJsonType($value)]; | |
| } | |
| public function set($model, $key, $value, $attributes) | |
| { | |
| if ($value !== null && !is_array($value) && !is_object($value)) { | |
| throw new \InvalidArgumentException('The given value must be array, object or null.'); | |
| } | |
| return [$key => new SpannerJsonType($value)]; | |
| } |
Added a custom Cast class to support handling JSON columns, updated readme to remove JSON from unsupported
I am not sure if this is still necessary but this is the approach I use to get JSON columns working smoothly
Summary by CodeRabbit
New Features
Documentation