diff --git a/app/Http/Controllers/Concerns/ManagesCachedPages.php b/app/Http/Controllers/Concerns/ManagesCachedPages.php new file mode 100644 index 000000000..1ae103696 --- /dev/null +++ b/app/Http/Controllers/Concerns/ManagesCachedPages.php @@ -0,0 +1,32 @@ +whereActive(true)->first(); + + if (! $page) { + return null; + } + + return [ + 'title' => $page->title, + 'content' => $page->content, + 'created_at' => optional($page->created_at)->format('M d, Y'), + ]; + } +} diff --git a/app/Http/Controllers/MobileController.php b/app/Http/Controllers/MobileController.php index cc0dcb0eb..051db0a56 100644 --- a/app/Http/Controllers/MobileController.php +++ b/app/Http/Controllers/MobileController.php @@ -2,19 +2,19 @@ namespace App\Http\Controllers; -use App\Models\Page; +use App\Http\Controllers\Concerns\ManagesCachedPages; use Illuminate\Http\Request; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\View; class MobileController extends Controller { + use ManagesCachedPages; + public function terms(Request $request) { $page = Cache::remember('site:terms', now()->addDays(120), function () { - $slug = '/site/terms'; - - return Page::whereSlug($slug)->whereActive(true)->first(); + return $this->cachedPage('/site/terms'); }); return View::make('mobile.terms')->with(compact('page'))->render(); @@ -23,9 +23,7 @@ class MobileController extends Controller public function privacy(Request $request) { $page = Cache::remember('site:privacy', now()->addDays(120), function () { - $slug = '/site/privacy'; - - return Page::whereSlug($slug)->whereActive(true)->first(); + return $this->cachedPage('/site/privacy'); }); return View::make('mobile.privacy')->with(compact('page'))->render(); diff --git a/app/Http/Controllers/ProfileController.php b/app/Http/Controllers/ProfileController.php index a2a3fddaf..e70e604c7 100644 --- a/app/Http/Controllers/ProfileController.php +++ b/app/Http/Controllers/ProfileController.php @@ -76,7 +76,19 @@ class ProfileController extends Controller $key = 'profile:settings:'.$user->id; $ttl = now()->addHours(6); $settings = Cache::remember($key, $ttl, function () use ($user) { - return $user->user->settings; + $s = optional($user->user)->settings; + + return [ + 'crawlable' => $s->crawlable ?? true, + 'following' => [ + 'count' => $s->show_profile_following_count ?? true, + 'list' => $s->show_profile_following ?? false, + ], + 'followers' => [ + 'count' => $s->show_profile_follower_count ?? true, + 'list' => $s->show_profile_followers ?? false, + ], + ]; }); if ($user->is_private == true) { @@ -89,17 +101,6 @@ class ProfileController extends Controller $is_following = false; $profile = $user; - $settings = [ - 'crawlable' => $settings->crawlable, - 'following' => [ - 'count' => $settings->show_profile_following_count, - 'list' => $settings->show_profile_following, - ], - 'followers' => [ - 'count' => $settings->show_profile_follower_count, - 'list' => $settings->show_profile_followers, - ], - ]; if ($carousel) { return view('profile.show_carousel', compact('profile', 'settings')); @@ -110,7 +111,19 @@ class ProfileController extends Controller $key = 'profile:settings:'.$user->id; $ttl = now()->addHours(6); $settings = Cache::remember($key, $ttl, function () use ($user) { - return $user->user->settings; + $s = optional($user->user)->settings; + + return [ + 'crawlable' => $s->crawlable ?? true, + 'following' => [ + 'count' => $s->show_profile_following_count ?? true, + 'list' => $s->show_profile_following ?? false, + ], + 'followers' => [ + 'count' => $s->show_profile_follower_count ?? true, + 'list' => $s->show_profile_followers ?? false, + ], + ]; }); if ($user->is_private == true) { @@ -132,17 +145,6 @@ class ProfileController extends Controller $is_admin = is_null($user->domain) ? $user->user->is_admin : false; $profile = $user; - $settings = [ - 'crawlable' => $settings->crawlable, - 'following' => [ - 'count' => $settings->show_profile_following_count, - 'list' => $settings->show_profile_following, - ], - 'followers' => [ - 'count' => $settings->show_profile_follower_count, - 'list' => $settings->show_profile_followers, - ], - ]; if ($carousel) { return view('profile.show_carousel', compact('profile', 'settings')); } diff --git a/app/Http/Controllers/SiteController.php b/app/Http/Controllers/SiteController.php index cc58ca784..bc09a16a5 100644 --- a/app/Http/Controllers/SiteController.php +++ b/app/Http/Controllers/SiteController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers; +use App\Http\Controllers\Concerns\ManagesCachedPages; use App\Models\Page; use App\Models\Profile; use App\Models\User; @@ -18,6 +19,8 @@ use Illuminate\Support\Str; class SiteController extends Controller { + use ManagesCachedPages; + public function home(Request $request) { if ($request->user() !== null) { @@ -86,9 +89,7 @@ class SiteController extends Controller public function privacy(Request $request) { $page = Cache::remember('site:privacy', now()->addDays(120), function () { - $slug = '/site/privacy'; - - return Page::whereSlug($slug)->whereActive(true)->first(); + return $this->cachedPage('/site/privacy'); }); return View::make('site.privacy')->with(compact('page'))->render(); @@ -97,9 +98,7 @@ class SiteController extends Controller public function terms(Request $request) { $page = Cache::remember('site:terms', now()->addDays(120), function () { - $slug = '/site/terms'; - - return Page::whereSlug($slug)->whereActive(true)->first(); + return $this->cachedPage('/site/terms'); }); return View::make('site.terms')->with(compact('page'))->render(); @@ -171,9 +170,7 @@ class SiteController extends Controller public function legalNotice(Request $request) { $page = Cache::remember('site:legal-notice', now()->addDays(120), function () { - $slug = '/site/legal-notice'; - - return Page::whereSlug($slug)->whereActive(true)->first(); + return $this->cachedPage('/site/legal-notice'); }); abort_if(! $page, 404); diff --git a/app/Models/CustomEmoji.php b/app/Models/CustomEmoji.php index 61c24d849..12b62669d 100644 --- a/app/Models/CustomEmoji.php +++ b/app/Models/CustomEmoji.php @@ -27,20 +27,32 @@ class CustomEmoji extends Model ->matchAll(self::SCAN_RE) ->map(function ($match) use ($activitypub) { $tag = Cache::remember(self::CACHE_KEY.$match, 14400, function () use ($match) { - return self::orderBy('id')->whereDisabled(false)->whereShortcode(':'.$match.':')->first(); + $emoji = self::orderBy('id')->whereDisabled(false)->whereShortcode(':'.$match.':')->first(); + + if (! $emoji) { + return null; + } + + return [ + 'id' => $emoji->id, + 'shortcode' => $emoji->shortcode, + 'media_path' => $emoji->media_path, + 'updated_at' => optional($emoji->updated_at)->toAtomString(), + 'disabled' => $emoji->disabled, + ]; }); if ($tag) { - $url = url('/storage/'.$tag->media_path); + $url = url('/storage/'.$tag['media_path']); if ($activitypub == true) { $mediaType = Str::endsWith($url, '.png') ? 'image/png' : 'image/jpg'; return [ - 'id' => url('emojis/'.$tag->id), + 'id' => url('emojis/'.$tag['id']), 'type' => 'Emoji', - 'name' => $tag->shortcode, - 'updated' => $tag->updated_at->toAtomString(), + 'name' => $tag['shortcode'], + 'updated' => $tag['updated_at'], 'icon' => [ 'type' => 'Image', 'mediaType' => $mediaType, @@ -52,7 +64,7 @@ class CustomEmoji extends Model 'shortcode' => $match, 'url' => $url, 'static_url' => $url, - 'visible_in_picker' => $tag->disabled == false, + 'visible_in_picker' => $tag['disabled'] == false, ]; } } diff --git a/app/Services/InstanceService.php b/app/Services/InstanceService.php index e5ffe53a4..2cad911a7 100644 --- a/app/Services/InstanceService.php +++ b/app/Services/InstanceService.php @@ -34,7 +34,9 @@ class InstanceService public static function getByDomain($domain) { return Cache::remember(self::CACHE_KEY_BY_DOMAIN.$domain, 3600, function () use ($domain) { - return Instance::whereDomain($domain)->first(); + $instance = Instance::whereDomain($domain)->first(); + + return $instance ? $instance->toArray() : null; }); } diff --git a/app/Services/StoryService.php b/app/Services/StoryService.php index d77fc30fb..01f4146c0 100644 --- a/app/Services/StoryService.php +++ b/app/Services/StoryService.php @@ -35,9 +35,7 @@ class StoryService public static function getById($id) { - return Cache::remember(self::STORY_KEY.'by-id:id-'.$id, 3600, function () use ($id) { - return Story::find($id); - }); + return Story::find($id); } public static function delById($id) diff --git a/resources/views/mobile/privacy.blade.php b/resources/views/mobile/privacy.blade.php index 35dece3b9..24b6ddd97 100644 --- a/resources/views/mobile/privacy.blade.php +++ b/resources/views/mobile/privacy.blade.php @@ -8,8 +8,8 @@
- @if($page && $page->content) - {!! $page->content !!} + @if($page && $page['content']) + {!! $page['content'] !!} @else
1. What information do we collect?
diff --git a/resources/views/mobile/terms.blade.php b/resources/views/mobile/terms.blade.php index af049646c..1f73ad550 100644 --- a/resources/views/mobile/terms.blade.php +++ b/resources/views/mobile/terms.blade.php @@ -7,8 +7,8 @@

Last Updated: Sept 28, 2022

- @if($page && $page->content) - {!! $page->content !!} + @if($page && $page['content']) + {!! $page['content'] !!} @else
1. Terms
diff --git a/resources/views/site/legal-notice.blade.php b/resources/views/site/legal-notice.blade.php index 18e7d92cd..58d70b4c5 100644 --- a/resources/views/site/legal-notice.blade.php +++ b/resources/views/site/legal-notice.blade.php @@ -3,11 +3,11 @@ @section('content')
-

{{ $page->title ?? 'Legal Notice' }}

+

{{ $page['title'] ?? 'Legal Notice' }}

- @if($page && $page->content) - {!! $page->content !!} + @if($page && $page['content']) + {!! $page['content'] !!} @endif
@@ -15,5 +15,5 @@
@endsection @push('meta') - + @endpush diff --git a/resources/views/site/privacy.blade.php b/resources/views/site/privacy.blade.php index 24f44323d..6d3e26086 100644 --- a/resources/views/site/privacy.blade.php +++ b/resources/views/site/privacy.blade.php @@ -6,8 +6,8 @@

Privacy Policy

- @if($page && $page->content) - {!! $page->content !!} + @if($page && $page['content']) + {!! $page['content'] !!} @else
1. What information do we collect?
diff --git a/resources/views/site/terms.blade.php b/resources/views/site/terms.blade.php index e05254d46..2424929a9 100644 --- a/resources/views/site/terms.blade.php +++ b/resources/views/site/terms.blade.php @@ -6,8 +6,8 @@

Terms of Use

- @if($page && $page->content) - {!! $page->content !!} + @if($page && $page['content']) + {!! $page['content'] !!} @else
1. Terms
diff --git a/tests/Feature/CriticalRoutes/PublicRouteSmokeTest.php b/tests/Feature/CriticalRoutes/PublicRouteSmokeTest.php index 0a7e46b7e..a883fe961 100644 --- a/tests/Feature/CriticalRoutes/PublicRouteSmokeTest.php +++ b/tests/Feature/CriticalRoutes/PublicRouteSmokeTest.php @@ -1,5 +1,9 @@ assertJsonStructure(['client_id', 'client_secret']); }); }); + +/* +|-------------------------------------------------------------------------- +| Cached-model regression tests +|-------------------------------------------------------------------------- +| +| These guard against a class of bug where an Eloquent model was cached +| directly (e.g. Cache::remember(..., fn () => $model)). On a cache read the +| model could deserialize into a __PHP_Incomplete_Class, throwing +| "attempt to access a property on an incomplete object" and returning a 500. +| +| The failure only surfaced on the SECOND request (the cache-read path), so +| every test here hits the route twice. +| +*/ + +describe('guest profile page (regression: cached UserSetting model)', function () { + uses(RefreshDatabase::class); + + it('renders a public profile for an unauthenticated user', function () { + $user = User::factory()->create(); + $user->refresh(); + + $this->get('/'.$user->username) + ->assertStatus(200); + }); + + it('renders a public profile on a repeated request (cache-read path)', function () { + $user = User::factory()->create(); + $user->refresh(); + + // First request populates the profile:settings cache. + $this->get('/'.$user->username) + ->assertStatus(200); + + // Second request reads from cache — this is where a cached Eloquent + // model previously deserialized as an incomplete object and 500'd. + $this->get('/'.$user->username) + ->assertStatus(200); + }); + + it('renders a public profile when the user has no settings row', function () { + $user = User::factory()->create(); + $user->refresh(); + + // Simulate a missing user_settings row; the controller must fall back + // to defaults instead of throwing on a null relation. + UserSetting::where('user_id', $user->id)->delete(); + + $this->get('/'.$user->username)->assertStatus(200); + $this->get('/'.$user->username)->assertStatus(200); + }); +}); + +describe('static site pages (regression: cached Page model)', function () { + uses(RefreshDatabase::class); + + it('loads terms of use twice', function () { + $this->get('/site/terms')->assertStatus(200); + $this->get('/site/terms')->assertStatus(200); + }); + + it('loads privacy policy twice', function () { + $this->get('/site/privacy')->assertStatus(200); + $this->get('/site/privacy')->assertStatus(200); + }); + + it('renders db-backed terms content twice', function () { + Page::create([ + 'slug' => '/site/terms', + 'title' => 'Terms', + 'content' => '

Custom terms content

', + 'active' => true, + ]); + + $this->get('/site/terms') + ->assertStatus(200) + ->assertSee('Custom terms content', false); + + $this->get('/site/terms') + ->assertStatus(200) + ->assertSee('Custom terms content', false); + }); + + it('loads legal notice twice when a page exists', function () { + Page::create([ + 'slug' => '/site/legal-notice', + 'title' => 'Legal Notice', + 'content' => '

Legal notice body

', + 'active' => true, + ]); + + $this->get('/site/legal-notice') + ->assertStatus(200) + ->assertSee('Legal notice body', false); + + $this->get('/site/legal-notice') + ->assertStatus(200) + ->assertSee('Legal notice body', false); + }); + + it('loads mobile terms and privacy twice', function () { + $this->get('/e/terms')->assertStatus(200); + $this->get('/e/terms')->assertStatus(200); + $this->get('/e/privacy')->assertStatus(200); + $this->get('/e/privacy')->assertStatus(200); + }); +}); + +describe('static informational pages load for guests', function () { + test('help index loads', function () { + $this->get('/site/help')->assertStatus(200); + }); + + test('fediverse info page loads', function () { + $this->get('/site/fediverse')->assertStatus(200); + }); + + test('open source page loads', function () { + $this->get('/site/open-source')->assertStatus(200); + }); + + test('developer api page loads', function () { + $this->get('/site/developer-api')->assertStatus(200); + }); + + test('getting started kb page loads', function () { + $this->get('/site/kb/getting-started')->assertStatus(200); + }); + + test('what is the fediverse kb page loads', function () { + $this->get('/site/kb/what-is-the-fediverse')->assertStatus(200); + }); +}); + +describe('community guidelines page (regression: cached page render)', function () { + uses(RefreshDatabase::class); + + it('loads the fallback twice', function () { + // No Page row exists; the route caches the rendered view either way. + $this->get('/site/kb/community-guidelines')->assertStatus(200); + $this->get('/site/kb/community-guidelines')->assertStatus(200); + }); + + it('renders db-backed content twice', function () { + Page::create([ + 'slug' => '/site/kb/community-guidelines', + 'title' => 'Community Guidelines', + 'content' => '

Be excellent to each other

', + 'active' => true, + ]); + + $this->get('/site/kb/community-guidelines') + ->assertStatus(200) + ->assertSee('Be excellent to each other', false); + + $this->get('/site/kb/community-guidelines') + ->assertStatus(200) + ->assertSee('Be excellent to each other', false); + }); +}); + +describe('public status page (regression: cached status/media services)', function () { + uses(RefreshDatabase::class); + + it('renders a public post for a guest on repeated requests', function () { + $user = User::factory()->create(); + $user->refresh(); + + $status = Status::factory()->create([ + 'profile_id' => $user->profile_id, + 'type' => 'photo', + 'scope' => 'public', + 'visibility' => 'public', + 'uri' => null, + ]); + + $url = "/p/{$user->username}/{$status->id}"; + + // Two requests: the second exercises the cache-read path in + // StatusService/MediaService. + $this->get($url)->assertStatus(200); + $this->get($url)->assertStatus(200); + }); +}); + +describe('profile activitypub object (regression: cached AP object)', function () { + uses(RefreshDatabase::class); + + beforeEach(function () { + // config_cache() falls back to config() when the DB-backed config + // cache is disabled, so setting these makes the test deterministic. + config([ + 'instance.enable_cc' => false, + 'federation.activitypub.enabled' => true, + ]); + }); + + it('returns activitypub json for a guest on repeated requests', function () { + $user = User::factory()->create(); + $user->refresh(); + + $this->getJson('/users/'.$user->username, [ + 'Accept' => 'application/activity+json', + ]) + ->assertStatus(200) + ->assertHeader('Content-Type', 'application/activity+json'); + + // Second request reads the cached AP object. + $this->getJson('/users/'.$user->username, [ + 'Accept' => 'application/activity+json', + ]) + ->assertStatus(200) + ->assertHeader('Content-Type', 'application/activity+json'); + }); +});