Merge branch 'release'

danskernesdigitalebibliotek · Aug 9, 2023 · 242202d · 242202d
2 parents 9f94ed2 + 732ee8d
commit 242202d
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 10 deletions.
diff --git a/app/Http/Controllers/v1/ListController.php b/app/Http/Controllers/v1/ListController.php
@@ -7,9 +7,11 @@
 use Illuminate\Http\Request;
 use Illuminate\Http\Response;
 use Illuminate\Support\Facades\DB;
+use Illuminate\Log\Logger;
 use Laravel\Lumen\Routing\Controller;
 use Illuminate\Database\Query\Builder;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
+use Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException;
 
 class ListController extends Controller
 {
@@ -28,6 +30,16 @@ class ListController extends Controller
      */
     protected $idFilterName = 'material_ids';
 
+    /**
+     * @var \Illuminate\Log\Logger
+     */
+    protected $log;
+
+    public function __construct(Logger $log)
+    {
+        $this->log = $log;
+    }
+
     /**
      * @return mixed[]
      */
@@ -64,8 +76,12 @@ protected function getItems(Request $request, string $listId): array
             // the URL.
             $query->where(function ($query) use ($itemIds) {
                 foreach ($itemIds as $itemId) {
-                    $item = ListItem::createFromString($itemId);
-                    $this->idQuery($query, $item, true);
+                    try {
+                        $item = ListItem::createFromString($itemId);
+                        $this->idQuery($query, $item, true);
+                    } catch (\InvalidArgumentException $e) {
+                        throw new UnprocessableEntityHttpException($e->getMessage());
+                    }
                 }
             });
         }
@@ -75,8 +91,12 @@ protected function getItems(Request $request, string $listId): array
 
         return $items->map(function (\stdClass $item_data) {
             $id = $item_data->material ?? $item_data->collection;
-            return ListItem::createFromString($id);
-        })->toArray();
+            try {
+                return ListItem::createFromString($id);
+            } catch (\InvalidArgumentException $e) {
+                $this->log->error('Unable to process stored item id: ' . $e->getMessage());
+            }
+        })->filter()->values()->toArray();
     }
 
     public function hasItem(Request $request, string $listId, ListItem $item): Response

diff --git a/app/ListItem.php b/app/ListItem.php
@@ -2,8 +2,6 @@
 
 namespace App;
 
-use Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException;
-
 final class ListItem
 {
     const DEFAULT_LIST_ID = 'default';
@@ -38,7 +36,7 @@ public static function createFromString(string $parameter): self
         $itemList = new static();
 
         if (!preg_match('/(work-of:)?(\d+)-(\w+):(\w+)/', urldecode($parameter), $matches)) {
-            throw new UnprocessableEntityHttpException('Invalid pid: ' . $parameter);
+            throw new \InvalidArgumentException('Invalid pid: ' . $parameter);
         }
 
         [$itemList->fullId, $workOf, $itemList->agency, $itemList->base, $itemList->id] = $matches;

diff --git a/app/Providers/RouteBindingServiceProvider.php b/app/Providers/RouteBindingServiceProvider.php
@@ -5,6 +5,7 @@
 use App\ListItem;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use mmghv\LumenRouteBinding\RouteBindingServiceProvider as BaseServiceProvider;
+use Symfony\Component\HttpKernel\Exception\UnprocessableEntityHttpException;
 
 class RouteBindingServiceProvider extends BaseServiceProvider
 {
@@ -24,7 +25,11 @@ public function boot()
         });
 
         $binder->bind('item', function ($value): ListItem {
-            return ListItem::createFromString($value);
+            try {
+                return ListItem::createFromString($value);
+            } catch (\InvalidArgumentException $e) {
+                throw new UnprocessableEntityHttpException($e->getMessage());
+            }
         });
     }
 }
diff --git a/infrastructure/docker/nginx/docker-entrypoint.sh b/infrastructure/docker/nginx/docker-entrypoint.sh
@@ -20,11 +20,11 @@ if [ "$1" = "nginx" -o "$1" = "nginx-debug" ]; then
             case "$f" in
                 *.envsh)
                     if [ -x "$f" ]; then
-                        echo >&3 "$0: Sourcing $f";
+                        entrypoint_log "$0: Sourcing $f";
                         . "$f"
                     else
                         # warn on shell scripts without exec bit
-                        echo >&3 "$0: Ignoring $f, not executable";
+                        entrypoint_log "$0: Ignoring $f, not executable";
                     fi
                     ;;
                 *.sh)

diff --git a/tests/features/get_list.feature b/tests/features/get_list.feature
@@ -97,3 +97,20 @@ Feature: Fetching list
       | 123-katalog:3 |
     When checking if "321-basis:2" is on the list
     Then the system should return success
+
+  Scenario: Invalid list items are not returned as a part of the list
+    Given a known user
+    And they have the following items on the list:
+      | material      |
+      | 123-basis:1   |
+      # This item is invalid, and should not be part of the response.
+      | banana        |
+      | 123-basis:2   |
+    Then fetching the list should return:
+      | material      |
+      # The order of the list is intentionally reversed. The last item in the precondition should be the first in the
+      # response as items are returned with the most recently added first.
+      | 123-basis:2   |
+      | 123-basis:1   |
+    And the system should return success
+