Merge pull request #413 from hydephp/code-quality

Code quality improvements
hydephp · Aug 11, 2022 · 81caef1 · 81caef1
2 parents 866ede9 + 49e886d
commit 81caef1
Show file tree

Hide file tree

Showing 12 changed files with 69 additions and 50 deletions.
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
@@ -13,7 +13,7 @@ This serves two purposes:
 - for new features.
 
 ### Changed
-- for changes in existing functionality.
+- Blog posts now have the same opengraph title format as other pages
 
 ### Deprecated
 - for soon-to-be removed features.

diff --git a/packages/framework/src/Models/Metadata/Metadata.php b/packages/framework/src/Models/Metadata/Metadata.php
@@ -76,42 +76,36 @@ protected function generate(): void
             ]));
         }
 
-        if ($this->page->has('canonicalUrl')) {
-            $this->add(Meta::link('canonical', $this->page->get('canonicalUrl')));
+        $this->addDynamicPageMetadata($this->page);
+    }
+
+    protected function addDynamicPageMetadata(AbstractPage $page): void
+    {
+        if ($page->has('canonicalUrl')) {
+            $this->add(Meta::link('canonical', $page->get('canonicalUrl')));
         }
 
-        if ($this->page->has('title')) {
-            $this->add(Meta::name('twitter:title', $this->page->htmlTitle()));
-            $this->add(Meta::property('title', $this->page->htmlTitle()));
+        if ($page->has('title')) {
+            $this->add(Meta::name('twitter:title', $page->htmlTitle()));
+            $this->add(Meta::property('title', $page->htmlTitle()));
         }
 
-        if ($this->page instanceof MarkdownPost) {
-            $this->addMetadataForMarkdownPost($this->page);
+        if ($page instanceof MarkdownPost) {
+            $this->addMetadataForMarkdownPost($page);
         }
     }
 
     protected function addMetadataForMarkdownPost(MarkdownPost $page): void
     {
-        if ($page->has('description')) {
-            $this->add(Meta::name('description', $page->get('description')));
-        }
-
-        if ($page->has('author')) {
-            $this->add(Meta::name('author', $page->get('author')));
-        }
-
-        if ($page->has('category')) {
-            $this->add(Meta::name('keywords', $page->get('category')));
-        }
+        $this->addPostMetadataIfExists($page, 'description');
+        $this->addPostMetadataIfExists($page, 'author');
+        $this->addPostMetadataIfExists($page, 'category', 'keywords');
+        $this->addPostMetadataIfExists($page, 'canonicalUrl', 'url');
 
         if ($page->has('canonicalUrl')) {
             $this->add(Meta::property('url', $page->get('canonicalUrl')));
         }
 
-        if ($page->has('title')) {
-            $this->add(Meta::property('title', $page->get('title')));
-        }
-
         if ($page->has('date')) {
             $this->add(Meta::property('og:article:published_time', $page->date->datetime));
         }
@@ -123,6 +117,13 @@ protected function addMetadataForMarkdownPost(MarkdownPost $page): void
         $this->add(Meta::property('type', 'article'));
     }
 
+    protected function addPostMetadataIfExists(MarkdownPost $page, string $property, ?string $name = null): void
+    {
+        if ($page->has($property)) {
+            $this->add(Meta::name($name ?? $property, $page->get($property)));
+        }
+    }
+
     protected function getPrefixedArray(string $group): array
     {
         $array = [];

diff --git a/packages/framework/src/Services/RssFeedService.php b/packages/framework/src/Services/RssFeedService.php
@@ -44,9 +44,9 @@ public function generate(): static
         return $this;
     }
 
-    public function getXML(): string|bool
+    public function getXML(): string
     {
-        return $this->feed->asXML();
+        return (string) $this->feed->asXML();
     }
 
     protected function addItem(MarkdownPost $post): void

diff --git a/packages/framework/src/Services/SitemapService.php b/packages/framework/src/Services/SitemapService.php
@@ -44,11 +44,11 @@ public function generate(): static
         return $this;
     }
 
-    public function getXML(): string|bool
+    public function getXML(): string
     {
         $this->xmlElement->addAttribute('processing_time_ms', (string) round((microtime(true) - $this->time_start) * 1000, 2));
 
-        return $this->xmlElement->asXML();
+        return (string) $this->xmlElement->asXML();
     }
 
     public function addRoute(RouteContract $route): void

diff --git a/packages/framework/tests/Feature/Commands/StaticSiteBuilderPostModuleTest.php b/packages/framework/tests/Feature/Commands/StaticSiteBuilderPostModuleTest.php
@@ -95,7 +95,7 @@ public function test_post_contains_expected_meta_tags()
             '<meta name="author" content="Lewis Carroll">',
             '<meta name="keywords" content="novels">',
             '<meta property="og:type" content="article">',
-            '<meta property="og:title" content="Adventures in Wonderland">',
+            '<meta property="og:title" content="HydePHP - Adventures in Wonderland">',
             '<meta property="og:article:published_time" content="1865-11-18T18:52:00+00:00">',
         ]);
     }

diff --git a/packages/framework/tests/Feature/MetadataTest.php b/packages/framework/tests/Feature/MetadataTest.php
@@ -394,13 +394,13 @@ public function test_does_not_add_url_property_when_canonical_url_is_null()
     public function test_adds_title_property_when_title_is_set_in_post()
     {
         $page = MarkdownPost::make(matter: ['title' => 'My Title']);
-        $this->assertPageHasMetadata($page, '<meta property="og:title" content="My Title">');
+        $this->assertPageHasMetadata($page, '<meta property="og:title" content="HydePHP - My Title">');
     }
 
     public function test_does_not_add_title_property_when_title_is_not_set_in_post()
     {
         $page = new MarkdownPost();
-        $this->assertPageDoesNotHaveMetadata($page, '<meta property="og:title" content="My Title">');
+        $this->assertPageDoesNotHaveMetadata($page, '<meta property="og:title"');
     }
 
     public function test_adds_published_time_property_when_date_is_set_in_post()

diff --git a/packages/framework/tests/Feature/MetadataViewTest.php b/packages/framework/tests/Feature/MetadataViewTest.php
@@ -51,18 +51,19 @@ protected function assertSee(string $page, string|array $text): string|array
 
     protected function assertAllTagsWereCovered(string $page, array $tags): void
     {
-        $haystack = file_get_contents(Hyde::path("_site/$page.html"));
-        $links = substr_count($haystack, '<link');
-        $meta = substr_count($haystack, '<meta');
+        $expected = file_get_contents(Hyde::path("_site/$page.html"));
+        $actual = json_encode($tags);
 
-        $haystack = json_encode($tags);
-        $actualLinks = substr_count($haystack, '<link');
-        $actualMeta = substr_count($haystack, '<meta');
+        $this->assertEquals(
+            substr_count($expected, '<meta'),
+            substr_count($actual, '<meta'),
+            "Failed asserting that all meta tags were covered in the page '$page'"
+        );
 
         $this->assertEquals(
-            $links + $meta,
-            $actualLinks + $actualMeta,
-            "Failed asserting that all tags were covered in the page '$page'"
+            substr_count($expected, '<link'),
+            substr_count($actual, '<link'),
+            "Failed asserting that all link tags were covered in the page '$page'"
         );
     }
 
@@ -137,7 +138,8 @@ public function test_metadata_tags_in_empty_markdown_post()
             '<link rel="stylesheet" href="../media/app.css">',
             '<link rel="canonical" href="http://localhost/posts/test.html">',
             '<meta name="twitter:title" content="HydePHP - Test">',
-            '<meta property="og:title" content="Test">',
+            '<meta name="url" content="http://localhost/posts/test.html">',
+            '<meta property="og:title" content="HydePHP - Test">',
             '<meta property="og:url" content="http://localhost/posts/test.html">',
             '<meta property="og:type" content="article">',
             '<meta itemprop="identifier" content="test">',
@@ -175,7 +177,8 @@ public function test_metadata_tags_in_markdown_post_with_flat_front_matter()
             '<meta name="description" content="My description">',
             '<meta name="author" content="Mr. Hyde">',
             '<meta name="keywords" content="My category">',
-            '<meta property="og:title" content="My title">',
+            '<meta name="url" content="http://localhost/posts/test.html">',
+            '<meta property="og:title" content="HydePHP - My title">',
             '<meta property="og:url" content="http://localhost/posts/test.html">',
             '<meta property="og:type" content="article">',
             '<meta property="og:article:published_time" content="2022-01-01T00:00:00+00:00">',

diff --git a/tests/Benchmarks/MarkdownBenchmark.php b/tests/Benchmarks/MarkdownBenchmark.php
@@ -8,7 +8,8 @@ class MarkdownBenchmark extends BenchCase
 {
     /**
      * Results history:
-     * #f4d8d452b 'avg_iteration_time': '0.42493788ms',.
+     * - #f4d8d452b: 0.42493788ms
+     * - #fd90ee987: 0.09853745ms.
      */
     public function testMarkdownParserFacadeShort()
     {
@@ -19,7 +20,8 @@ public function testMarkdownParserFacadeShort()
 
     /**
      * Results history:
-     * #f4d8d452b 'avg_iteration_time': '2.62538815ms',.
+     * - #f4d8d452b 2.62538815ms
+     * - #fd90ee987: 2.07124043ms.
      */
     public function testMarkdownParserFacadeFull()
     {

diff --git a/tests/Benchmarks/PageParserBenchmark.php b/tests/Benchmarks/PageParserBenchmark.php
@@ -15,6 +15,7 @@ class PageParserBenchmark extends BenchCase
      * Results history:
      * - #14c34beb1: 0.17022841ms.
      * - #0db04eaef: 0.2702086ms.
+     * - #fd90ee987: 0.28220031ms.
      */
     public function testParseBladePageFile()
     {
@@ -34,6 +35,7 @@ public function testParseBladePageFile()
      * Results history:
      * - #8e165366a: 0.1986541ms.
      * - #0db04eaef: 0.29952221ms.
+     * - #fd90ee987: 0.30080938ms.
      */
     public function testParseMarkdownPageFile()
     {
@@ -53,6 +55,7 @@ public function testParseMarkdownPageFile()
      * Results history:
      * - #5d044679b: 0.30337441ms.
      * - #0db04eaef: 0.39583502ms.
+     * - #fd90ee987: 0.41671791ms.
      */
     public function testParseMarkdownPostFile()
     {
@@ -72,6 +75,7 @@ public function testParseMarkdownPostFile()
      * Results history:
      * - #6f63f5016: 0.16660199ms.
      * - #0db04eaef: 0.2729635ms.
+     * - #fd90ee987: 0.27649839ms.
      */
     public function testParseDocumentationPageFile()
     {

diff --git a/tests/Benchmarks/SearchIndexBenchmark.php b/tests/Benchmarks/SearchIndexBenchmark.php
@@ -7,9 +7,10 @@
 class SearchIndexBenchmark extends BenchCase
 {
     /**
-     * Results history: (with xdebug)
-     * #5555b676b 'avg_iteration_time': '26.75ms'
-     * #2d73931ad 'avg_iteration_time': '19.41ms'.
+     * Results history:
+     * - #5555b676b: 26.75ms (with xdebug)
+     * - #2d73931ad: 19.41ms (with xdebug)
+     * - #fd90ee987: 10.0887394ms.
      */
     public function testBuildSearchIndexCommandNoFiles()
     {
@@ -22,9 +23,10 @@ public function testBuildSearchIndexCommandNoFiles()
     }
 
     /**
-     * Results history: (with xdebug)
-     * #5555b676b 'avg_iteration_time': '120.45ms'
-     * #2d73931ad 'avg_iteration_time': '105.91ms'.
+     * Results history:
+     * - #5555b676b: 120.45ms (with xdebug)
+     * - #2d73931ad: 105.91ms (with xdebug)
+     * - #fd90ee987: 36.29438877ms.
      */
     public function testBuildSearchIndexCommandWithFiles()
     {

diff --git a/tests/Benchmarks/StaticSiteBuilderBenchmark.php b/tests/Benchmarks/StaticSiteBuilderBenchmark.php
@@ -10,7 +10,8 @@ class StaticSiteBuilderBenchmark extends BenchCase
     /**
      * Results history:
      * - #49834e374: 76.05051994ms
-     * - #1cf136fc0: 85.29273033ms.
+     * - #1cf136fc0: 85.29273033ms
+     * - #fd90ee987: 49.32703972ms.
      */
     public function testParseBladePageFile()
     {

diff --git a/tests/Browser/HighLevelViewTest.php b/tests/Browser/HighLevelViewTest.php
@@ -18,6 +18,12 @@ class HighLevelViewTest extends DuskTestCase
 {
     public $mockConsoleOutput = false;
 
+    protected function tearDown(): void
+    {
+        parent::tearDown();
+        sleep(1);
+    }
+
     public function test_welcome_homepage()
     {
         $this->browse(function (Browser $browser) {