-
Notifications
You must be signed in to change notification settings - Fork 0
feat(Storage) : Implement Object Contexts with filter and validation #2
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
base: main
Are you sure you want to change the base?
Changes from all commits
c2a16fb
b941e81
fa985a8
409e321
8f5051f
49d60ad
59732ed
ffc8b00
fbf594e
37b7051
e94a00d
a563bb4
d258bd9
711c118
d70d66a
41aec81
1e86369
3520f8b
7e433b8
248ffe8
fdd1e97
902eb86
e87efaf
e78441d
5bb84fb
f31760e
d1fb39c
eda39e8
be06d29
64b6c01
1241ae6
500b9f2
0ed4e1e
3996210
0e84582
cfd37d1
54e9eac
b7afe81
b966e67
c5f7aa8
6956993
fe2f2aa
08fb2de
5d7f458
f8a845d
eac4fd3
b919376
b65b013
6ae7138
60fab43
7a90dad
e4ec9bf
f37252c
e1a4699
1270cf2
af178b1
c4021cb
8eaf4de
51d3e09
a8a67ff
fa23edb
99065d2
b30316a
bd9c720
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -274,6 +274,18 @@ public function exists(array $options = []) | |
| * @type array $metadata The full list of available options are outlined | ||
| * at the [JSON API docs](https://cloud.google.com/storage/docs/json_api/v1/objects/insert#request-body). | ||
| * @type array $metadata.metadata User-provided metadata, in key/value pairs. | ||
| * @type array $contexts User-defined or system-defined object contexts. | ||
| * Each object context is a key-payload pair, where the key provides the | ||
| * identification and the payload holds the associated value and additional metadata. | ||
| * @type array $contexts.custom Custom user-defined contexts. Keys must start | ||
| * with an alphanumeric character and cannot contain double quotes (`"`). | ||
| * @type string $contexts.custom.{key}.value The value associated with the context. | ||
| * If not empty, must start with an alphanumeric character and cannot contain double quotes (`"`) | ||
| * or forward slashes (`/`). | ||
| * @type string $contexts.custom.{key}.createTime The time the context | ||
| * was created in RFC 3339 format. **(read only)** | ||
| * @type string $contexts.custom.{key}.updateTime The time the context | ||
| * was last updated in RFC 3339 format. **(read only)** | ||
| * @type string $encryptionKey A base64 encoded AES-256 customer-supplied | ||
| * encryption key. If you would prefer to manage encryption | ||
| * utilizing the Cloud Key Management Service (KMS) please use the | ||
|
|
@@ -294,6 +306,13 @@ public function upload($data, array $options = []) | |
| throw new \InvalidArgumentException('A name is required when data is of type string or null.'); | ||
| } | ||
|
|
||
| if (isset($options['contexts'])) { | ||
| if (!is_array($options['contexts'])) { | ||
| throw new \InvalidArgumentException('Object contexts must be an array.'); | ||
| } | ||
| $this->validateContexts($options['contexts']); | ||
| } | ||
|
Comment on lines
+309
to
+314
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The PR description mentions that if (isset($options['contexts'])) {
if (!is_array($options['contexts'])) {
throw new \InvalidArgumentException('Object contexts must be an array.');
}
$this->validateContexts($options['contexts']);
}
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, That means before throwing an error we need to throw clean message. |
||
|
|
||
| $encryptionKey = $options['encryptionKey'] ?? null; | ||
| $encryptionKeySHA256 = $options['encryptionKeySHA256'] ?? null; | ||
|
|
||
|
|
@@ -314,6 +333,49 @@ public function upload($data, array $options = []) | |
| ); | ||
| } | ||
|
|
||
| private function validateContexts(?array $contexts) | ||
| { | ||
| if (!isset($contexts['custom'])) { | ||
| return; | ||
| } | ||
| if (!is_array($contexts['custom'])) { | ||
| throw new \InvalidArgumentException('Object contexts custom field must be an array.'); | ||
| } | ||
| foreach ($contexts['custom'] as $key => $data) { | ||
| if (!preg_match('/^[a-zA-Z0-9]/', (string) $key)) { | ||
| throw new \InvalidArgumentException('Object context key must start with an alphanumeric.'); | ||
| } | ||
| if (strpos((string) $key, '"') !== false) { | ||
| throw new \InvalidArgumentException('Object context key cannot contain double quotes.'); | ||
| } | ||
| if ($data === null) { | ||
| continue; | ||
| } | ||
| if (!is_array($data)) { | ||
| throw new \InvalidArgumentException(sprintf( | ||
| 'Context data for key "%s" must be an array.', | ||
| $key | ||
| )); | ||
| } | ||
|
Comment on lines
+354
to
+359
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The validation logic currently prevents deleting individual context keys by setting them to if ($data === null) {
continue;
}
if (!is_array($data)) {
throw new \InvalidArgumentException(sprintf(
'Context data for key "%s" must be an array.',
$key
));
} |
||
| if (!isset($data['value'])) { | ||
| throw new \InvalidArgumentException(sprintf( | ||
| 'Context for key "%s" must have a \'value\' property.', | ||
| $key | ||
| )); | ||
| } | ||
| if (!is_scalar($data['value'])) { | ||
| throw new \InvalidArgumentException(sprintf( | ||
| 'Context value for key "%s" must be a scalar type.', | ||
| $key | ||
| )); | ||
| } | ||
| $val = (string) $data['value']; | ||
| if ($val !== '' && !preg_match('/^[a-zA-Z0-9][^"\/]*$/', $val)) { | ||
| throw new \InvalidArgumentException('Object context value must start with an alphanumeric.'); | ||
| } | ||
|
Comment on lines
+373
to
+375
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency with the key validation logic and to provide more specific error messages, consider splitting the value validation into separate checks. The current error message is also incomplete as it doesn't mention the restrictions on double quotes and forward slashes. if ($val !== '') {
if (!preg_match('/^[a-zA-Z0-9]/', $val)) {
throw new \InvalidArgumentException('Object context value must start with an alphanumeric.');
}
if (preg_match('/["\/]/', $val)) {
throw new \InvalidArgumentException('Object context value cannot contain double quotes or forward slashes.');
}
}
Comment on lines
+373
to
+375
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error message is misleading. If the value starts with an alphanumeric character but contains a forward slash or a double quote, the regex will fail, but the exception message will only mention the alphanumeric requirement. It should be updated to reflect all constraints. if ($val !== '' && !preg_match('/^[a-zA-Z0-9][^"\/]*$/', $val)) {
throw new \InvalidArgumentException('Object context value must start with an alphanumeric and cannot contain double quotes or forward slashes.');
} |
||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Asynchronously uploads an object. | ||
| * | ||
|
|
@@ -703,6 +765,9 @@ public function restore($name, $generation, array $options = []) | |
| * distinct results. **Defaults to** `false`. | ||
| * @type string $fields Selector which will cause the response to only | ||
| * return the specified fields. | ||
| * @type string $filter Filter results to include only objects to which the | ||
| * specified context is attached. You can filter by the presence, | ||
| * absence, or specific value of context keys. | ||
| * @type string $matchGlob A glob pattern to filter results. The string | ||
| * value must be UTF-8 encoded. See: | ||
| * https://cloud.google.com/storage/docs/json_api/v1/objects/list#list-object-glob | ||
|
|
@@ -712,7 +777,6 @@ public function restore($name, $generation, array $options = []) | |
| public function objects(array $options = []) | ||
| { | ||
| $resultLimit = $this->pluck('resultLimit', $options, false); | ||
|
|
||
| return new ObjectIterator( | ||
| new ObjectPageIterator( | ||
| function (array $object) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -231,6 +231,18 @@ public function delete(array $options = []) | |
| * This is the retention configuration set for this object. | ||
| * @type string $retention.mode The mode of the retention configuration, | ||
| * which can be either `"Unlocked"` or `"Locked"`. | ||
| * @type array $contexts User-defined or system-defined object contexts. | ||
| * Each object context is a key-payload pair, where the key provides the | ||
| * identification and the payload holds the associated value and additional metadata. | ||
| * @type array $contexts.custom Custom user-defined contexts. Keys must start | ||
| * with an alphanumeric character and cannot contain double quotes (`"`). | ||
| * @type string $contexts.custom.{key}.value The value associated with the context. | ||
| * If not empty, must start with an alphanumeric character and cannot contain double quotes (`"`) | ||
| * or forward slashes (`/`). | ||
|
Comment on lines
+239
to
+241
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The documentation for the context value here is inconsistent with the one in * @type string $contexts.custom.{key}.value The value associated with the context.
* If not empty, must start with an alphanumeric character and cannot contain double quotes (`"`)
* or forward slashes (`/`). |
||
| * @type string $contexts.custom.{key}.createTime The time the context | ||
| * was created in RFC 3339 format. **(read only)** | ||
| * @type string $contexts.custom.{key}.updateTime The time the context | ||
| * was last updated in RFC 3339 format. **(read only)** | ||
| * @type bool $overrideUnlockedRetention Applicable for objects that | ||
| * have an unlocked retention configuration. Required to be set to | ||
| * `true` if the operation includes a retention property that | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation states that the context value "Must start with an alphanumeric character", but the implementation in
validateContexts(line 367) explicitly allows an empty string ($val !== ''). If empty strings are permitted, the documentation should be updated to reflect this (e.g., "If not empty, must start with...").