8000 Merge pull request #1835 from erikn69/upgrade · PaolaRuby/laravel-permission@58d5eb6 · GitHub
[go: up one dir, main page]
Skip to content

Commit 58d5eb6

Browse files
drbytedrbyte
authored
Merge pull request spatie#1835 from erikn69/upgrade
[V5] Merge pull request spatie#1834 from danilopinotti/avoid-overhydratation
2 parents 9b7523b + a2f478a commit 58d5eb6

File tree

3 files changed

+66
-23
lines changed

3 files changed

+66
-23
lines changed

‎CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
All notable changes to `laravel-permission` will be documented in this file
44

5+
## 5.1.1 - 2021-09-01
6+
- Avoid Roles over-hydration #1834
7+
58
## 5.1.0 - 2021-08-31
69
- No longer flush cache on User role/perm assignment changes #1832
710
NOTE: You should test your app to be sure that you don't accidentally have deep dependencies on cache resets happening automatically in these cases.
@@ -12,6 +15,8 @@ All notable changes to `laravel-permission` will be documented in this file
1215
- Teams/Groups feature (see docs, or PR #1804)
1316
- Customized pivots instead of `role_id`,`permission_id` #1823
1417

18+
## 4.4.1 - 2021-09-01
19+
- Avoid Roles over-hydration #1834
1520

1621
## 4.4.0 - 2021-08-28
1722
- Avoid BC break (removed interface change) on cache change added in 4.3.0 #1826

‎src/PermissionRegistrar.php

Lines changed: 50 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@ class PermissionRegistrar
4747
/** @var string */
4848
public static $cacheKey;
4949

50+
/** @var array */
51+
private $cachedRoles = [];
52+
5053
/**
5154
* PermissionRegistrar constructor.
5255
*
@@ -153,30 +156,36 @@ public function clearClassPermissions()
153156
*/
154157
private function loadPermissions()
155158
{
156-
if ($this->permissions === null) {
157-
$this->permissions = $this->cache->remember(self::$cacheKey, self::$cacheExpirationTime, function () {
158-
// make the cache smaller using an array with only required fields
159-
return $this->getPermissionClass()->select('id', 'id as i', 'name as n', 'guard_name as g')
160-
->with('roles:id,id as i,name as n,guard_name as g')->get()
161-
->map(function ($permission) {
162-
return $permission->only('i', 'n', 'g') +
163-
['r' => $permission->roles->map->only('i', 'n', 'g')->all()];
164-
})->all();
159+
if ($this->permissions !== null) {
160+
return;
161+
}
162+
163+
$this->permissions = $this->cache->remember(self::$cacheKey, self::$cacheExpirationTime, function () {
164+
// make the cache smaller using an array with only required fields
165+
return $this->getPermissionClass()->select('id', 'id as i', 'name as n', 'guard_name as g')
166+
->with('roles:id,id as i,name as n,guard_name as g')->get()
167+
->map(function ($permission) {
168+
return $permission->only('i', 'n', 'g') +
169+
['r' => $permission->roles->map->only('i', 'n', 'g')->all()];
170+
})->all();
171+
});
172+
173+
if (is_array($this->permissions)) {
174+
$this->permissions = $this->getPermissionClass()::hydrate(
175+
collect($this->permissions)->map(function ($item) {
176+
return ['id' => $item['i'] ?? $item['id'], 'name' => $item['n'] ?? $item['name'], 'guard_name' => $item['g'] ?? $item['guard_name']];
177+
})->all()
178+
)
179+
->each(function ($permission, $i) {
180+
$roles = Collection::make($this->permissions[$i]['r'] ?? $this->permissions[$i]['roles'] ?? [])
181+
->map(function ($item) {
182+
return $this->getHydratedRole($item);
183+
});
184+
185+
$permission->setRelation('roles', $roles);
165186
});
166-
if (is_array($this->permissions)) {
167-
$this->permissions = $this->getPermissionClass()::hydrate(
168-
collect($this->permissions)->map(function ($item) {
169-
return ['id' => $item['i'] ?? $item['id'], 'name' => $item['n'] ?? $item['name'], 'guard_name' => $item['g'] ?? $item['guard_name']];
170-
})->all()
171-
)
172-
->each(function ($permission, $i) {
173-
$permission->setRelation('roles', $this->getRoleClass()::hydrate(
174-
collect($this->permissions[$i]['r'] ?? $this->permissions[$i]['roles'] ?? [])->map(function ($item) {
175-
return ['id' => $item['i'] ?? $item['id'], 'name' => $item['n'] ?? $item['name'], 'guard_name' => $item['g'] ?? $item['guard_name']];
176-
})->all()
177-
));
178-
});
179-
}
187+
188+
$this->cachedRoles = [];
180189
}
181190
}
182191

@@ -247,4 +256,22 @@ public function getCacheStore(): \Illuminate\Contracts\Cache\Store
247256
{
248257
return $this->cache->getStore();
249258
}
259+
260+
private function getHydratedRole(array $item)
261+
{
262+
$roleId = $item['i'] ?? $item['id'];
263+
264+
if (isset($this->cachedRoles[$roleId])) {
265+
return $this->cachedRoles[$roleId];
266+
}
267+
268+
$roleClass = $this->getRoleClass();
269+
$roleInstance = new $roleClass;
270+
271+
return $this->cachedRoles[$roleId] = $roleInstance->newFromBuilder([
272+
'id' => $roleId,
273+
'name' => $item['n'] ?? $item['name'],
274+
'guard_name' => $item['g'] ?? $item['guard_name'],
275+
]);
276+
}
250277
}

‎tests/CacheTest.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,17 @@ public function get_all_permissions_should_use_the_cache()
247247
$this->assertQueryCount(2);
248248
}
249249

250+
/** @test */
251+
public function get_all_permissions_should_not_over_hydrate_roles()
252+
{
253+
$this->testUserRole->givePermissionTo(['edit-articles', 'edit-news']);
254+
$permissions = $this->registrar->getPermissions();
255+
$roles = $permissions->flatMap->roles;
256+
257+
// Should have same object reference
258+
$this->assertSame($roles[0], $roles[1]);
259+
}
260+
250261
/** @test */
251262
public function it_can_reset_the_cache_with_artisan_command()
252263
{

0 commit comments

Comments
 (0)
0