Data Sync - #138
Data Sync#138jakub-grzesiowski wants to merge 8 commits into
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
| * Two clients are involved: an admin signing its requests with the secret key, which grants the | ||
| * tokens and owns the fixtures, and a token-only client that has no credentials beyond the token | ||
| * under test. That split is what makes the denials meaningful - a client carrying both a signature | ||
| * and a token is rejected outright, so the token is the only thing granting the access asserted |
There was a problem hiding this comment.
What does it mean "a client carrying both a signature
- and a token is rejected outright" ?
Is this server rejecting such request?
There was a problem hiding this comment.
Yes DataSync will reject a request if it has both auth and signature
| foreach ($this->sort as $key => $value) { | ||
| if (is_int($key)) { | ||
| $entries[] = $value; | ||
| } elseif ($value === 'asc' || $value === 'desc') { |
There was a problem hiding this comment.
Is this possible that $value is 'DESC' ?
If so then it seem that else branch will be executed causing ascending sort?
| if (in_array($typeName, self::ENTITY_TYPES, true)) { | ||
| $entity = PNDataSyncEntity::fromPayload(array_merge($data, [ | ||
| 'entityClass' => $className, | ||
| 'entityClassVersion' => $classVersion, |
There was a problem hiding this comment.
Shouldn't we include also:
'entityClassLevel' => $classLevel
?
| } else { | ||
| $messageError = null; | ||
| try { | ||
| $extractedMessage = $this->processMessage($message->getPayload()); |
There was a problem hiding this comment.
There might be a problem when Crypto is configured processMessage() calls decrypt() that takes only string and objects not array DataSync event payload is an array causing TypeError
| protected const PATCH_MEDIA_TYPE = "application/json-patch+json"; | ||
|
|
||
| /** A delete answers 200 with no body. */ | ||
| protected const RESPONSE_MAY_BE_EMPTY = true; |
There was a problem hiding this comment.
Should't this apply only to Deleted operations that could return empty body. Other operations should not delete empty body?
| /** | ||
| * Response of the paginated membership list operation. | ||
| */ | ||
| class PNDataSyncMembershipsResult |
There was a problem hiding this comment.
This class seems very similar to:
PNDataSyncEntitiesResult
PNDataSyncRelationshipsResult
PNDataSyncUsersResult
PNDataSyncChannelsResult
Did you consider creation of base class that those classes can extend. Is such approach we could avoid code duplication?
| /** | ||
| * Response of the single-entity DataSync operations (create, get, set and update). | ||
| */ | ||
| class PNDataSyncEntityResult |
There was a problem hiding this comment.
Seem that this class is very similar to:
PNDataSyncRelationshipResult
PNDataSyncMembershipResult
PNDataSyncUserResult
PNDataSyncChannelResult
Did you consider creating baseClass to avoid duplication?
3 participants
No description provided.