Add developer cache #110
Add developer cache #110
Conversation
Apigee/ManagementAPI/Developer.php
Outdated
| $developer->attributes[$attribute['name']] = @$attribute['value']; | ||
| foreach ($response['attributes'] as $key => $attribute) { | ||
| if (is_array($attribute)) { | ||
| $developer->attributes[$attribute['name']] = @$attribute['value']; |
There was a problem hiding this comment.
What kind of errors needs to be suppressed here? The only that I can imagine is the 'value' index does not exist which should not happen.
| $developer->createdBy = $response['createdBy']; | ||
| $developer->modifiedAt = $response['lastModifiedAt']; | ||
| $developer->modifiedBy = $response['lastModifiedBy']; | ||
| $developer->modifiedAt = array_key_exists('lastModifiedAt', $response) ? $response['lastModifiedAt'] : $response['modifiedAt']; |
There was a problem hiding this comment.
In which circumstances we get back "modifiedAt" instead of "lastModifiedAt"?
There was a problem hiding this comment.
It's different on the object and on the response... I had to make sure it is compatible.
Apigee/ManagementAPI/Developer.php
Outdated
| $data[$organization] = array( | ||
| 'developers' => array(), | ||
| 'all_cached' => false, | ||
| 'all_expanded' => false, |
There was a problem hiding this comment.
Why would you cache non-expanded developer objects? That is just an other complexity that you have to deal with. Developer cache should contain only fully loaded developer objects.
Also, if you would like to use a complex structure like this for static cache I'd rather recommend you to create an object for that and instead of this array. It is not just complicated to work with an array, but also can cause performance degradation.
https://steemit.com/php/@crell/php-use-associative-arrays-basically-never
There was a problem hiding this comment.
Also, are you sure that we should introduce the static cache in the PHP SDK level not just in the Drupal 7 module? I do not know how many 3rd party apps relies on the V1 SDK but maybe we could handle this performance problem only in the Drupal 7 module. Therefore we would only need to deal with possible problems (bugs) that these changes may introduce in one place instead of two.
(Just an idea.)
PS.: I haven't implemented static caching in the V2 PHP API client, because if consumer apps needs that they can easily add that and after that they have to deal with the cache invalidation problem. ;) Besides the V1 SDK becomes deprecated sooner or later thanks for the new https://github.com/apigee/apigee-client-php.
2 participants
Related PR: https://github.com/apigee-internal/devportal-profile/pull/568