Merge pull request #523 from jaredhendrickson13/next_patch

jaredhendrickson13 · web-flow · commit 82b6baeb3e7d · 2024-07-26T17:59:18.000-06:00
diff --git a/docs/QUERIES_AND_FILTERS.md b/docs/QUERIES_AND_FILTERS.md
@@ -103,3 +103,4 @@ reduce the amount of data returned in a single request.
     - By default, the REST API does not paginate responses. If you want to paginate the response, you must include the
     `limit` and `offset` query parameters in your request.
     - Pagination is only available for `GET` requests to [plural endpoints](./ENDPOINT_TYPES.md#plural-many-endpoints).
+    - If combined with a query, pagination will be applied after the initial query is executed.
diff --git a/pfSense-pkg-RESTAPI/files/etc/inc/priv/restapi.priv.inc b/pfSense-pkg-RESTAPI/files/etc/inc/priv/restapi.priv.inc
@@ -13,14 +13,11 @@
 //   See the License for the specific language governing permissions and
 //   limitations under the License.
 
-require_once 'RESTAPI/Caches/PrivilegesCache.inc';
-
-use RESTAPI\Caches\PrivilegesCache;
-
 global $priv_list;
 
 # Include the privileges that are auto-generated by \RESTAPI\Caches\PrivilegesCache
-$privileges_cache = new PrivilegesCache();
-if (is_file($privileges_cache->get_file_path())) {
-    $priv_list += $privileges_cache->read();
+if (is_file('/usr/local/pkg/RESTAPI/.resources/cache/PrivilegesCache.json')) {
+    $privs_json = file_get_contents('/usr/local/pkg/RESTAPI/.resources/cache/PrivilegesCache.json');
+    $privileges_cache = json_decode($privs_json, associative: true);
+    $priv_list += $privileges_cache;
 }
diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Model.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Model.inc
@@ -356,6 +356,8 @@ class Model {
     private function construct_from_internal(mixed $id = null, mixed $parent_id = null): void {
         # Do not try to load the object from internal if the skip_init flag is set
         if ($this->skip_init) {
+            $this->id = $id;
+            $this->parent_id = $parent_id;
             return;
         }
 
@@ -1567,79 +1569,6 @@ class Model {
         $this->apply();
     }
 
-    /**
-     * Fetches Model objects for all objects stored in the internal pfSense values. If `config_path` is set, this will
-     * load Model objects for each object stored at the config path. If `internal_callable` is set, this will create
-     * Model objects for each object returned by the specified callable.
-     * @param mixed|null $parent_id Specifies the ID of the parent Model to read all objects from. This is required for
-     * $many Models with a $parent_model_class. This value has no affect otherwise.
-     * @param int $offset The starting point in the dataset to be used with $limit. This is only applicable to $many
-     * enabled Models.
-     * @param int $limit The maximum number of Model objects to retrieve. This is only applicable to $many
-     * enabled Models.
-     * @return ModelSet|Model Returns a ModelSet of Models if `many` is enabled or a single Model object if `many` is
-     * not enabled.
-     */
-    public static function read_all(mixed $parent_id = null, int $limit = 0, int $offset = 0): ModelSet|Model {
-        # Variables
-        $model_name = get_called_class();
-        $model = new $model_name(parent_id: $parent_id);
-        $model_objects = [];
-        $is_parent_model_many = $model->is_parent_model_many();
-        $requests_pagination = ($limit or $offset);
-        $offset_counter = 0;
-
-        # Throw an error if this Model has a $many parent Model, but no parent Model ID was given
-        if ($is_parent_model_many and !isset($parent_id)) {
-            throw new ValidationError(
-                message: 'Field `parent_id` is required to read all.',
-                response_id: 'MODEL_PARENT_ID_REQUIRED',
-            );
-        }
-
-        # Throw an error if pagination was requested on a Model without $many enabled
-        if (!$model->many and $requests_pagination) {
-            throw new ValidationError(
-                message: "Model `$model->verbose_name` does not support pagination. Please remove the `limit` and/or. " .
-                    '`offset`parameters and try again.',
-                response_id: 'MODEL_DOES_NOT_SUPPORT_PAGINATION',
-            );
-        }
-
-        # Obtain all of this Model's internally stored objects
-        $internal_objects = $model->get_internal_objects();
-
-        # For non `many` Models, wrap the internal object in an array so we can loop
-        $internal_objects = $model->many ? $internal_objects : [$internal_objects];
-
-        # Loop through each internal object and create a Model object for it
-        foreach ($internal_objects as $internal_id => $internal_object) {
-            # Do not include this object if we have not reached our offset
-            if ($offset_counter < $offset) {
-                $offset_counter++;
-                continue;
-            }
-
-            # Create a new Model object for this internal object and assign its ID
-            $model_object = new $model(id: $internal_id, parent_id: $parent_id);
-
-            # Populate the Model object using its current internal values and add it to the array of all Model objects
-            $model_object->from_internal_object($internal_object);
-            $model_objects[] = $model_object;
-
-            # Break the loop if $limit is set, and we've met our limit of objects to obtain
-            if ($limit and $limit === count($model_objects)) {
-                break;
-            }
-
-            # Increase the offset counter
-            $offset_counter++;
-        }
-
-        # Unwrap the array for non `many` Models, otherwise return all objects
-        return $model->many ? new ModelSet($model_objects) : $model_objects[0];
-    }
-
     /**
      * Sorts `many` Model entries internally before writing the changes to config. This is useful for Model's whose
      * internal objects must be written in a specific order.
@@ -1739,6 +1668,82 @@ class Model {
         }
     }
 
+    /**
+     * Fetches Model objects for all objects stored in the internal pfSense values. If `config_path` is set, this will
+     * load Model objects for each object stored at the config path. If `internal_callable` is set, this will create
+     * Model objects for each object returned by the specified callable.
+     * @param mixed|null $parent_id Specifies the ID of the parent Model to read all objects from. This is required for
+     * $many Models with a $parent_model_class. This value has no affect otherwise.
+     * @param int $offset The starting point in the dataset to be used with $limit. This is only applicable to $many
+     * enabled Models.
+     * @param int $limit The maximum number of Model objects to retrieve. This is only applicable to $many
+     * enabled Models.
+     * @return ModelSet|Model Returns a ModelSet of Models if `many` is enabled or a single Model object if `many` is
+     * not enabled.
+     */
+    public static function read_all(mixed $parent_id = null, int $limit = 0, int $offset = 0): ModelSet|Model {
+        # Variables
+        $model_name = get_called_class();
+        $model = new $model_name(parent_id: $parent_id);
+        $model_objects = [];
+        $is_parent_model_many = $model->is_parent_model_many();
+        $requests_pagination = ($limit or $offset);
+        $offset_counter = 0;
+
+        # Throw an error if this Model has a $many parent Model, but no parent Model ID was given
+        if ($is_parent_model_many and !isset($parent_id)) {
+            throw new ValidationError(
+                message: 'Field `parent_id` is required to read all.',
+                response_id: 'MODEL_PARENT_ID_REQUIRED',
+            );
+        }
+
+        # Throw an error if pagination was requested on a Model without $many enabled
+        if (!$model->many and $requests_pagination) {
+            throw new ValidationError(
+                message: "Model `$model->verbose_name` does not support pagination. Please remove the `limit` and/or. " .
+                    '`offset`parameters and try again.',
+                response_id: 'MODEL_DOES_NOT_SUPPORT_PAGINATION',
+            );
+        }
+
+        # Obtain all of this Model's internally stored objects
+        $internal_objects = $model->get_internal_objects();
+
+        # For non `many` Models, wrap the internal object in an array so we can loop
+        $internal_objects = $model->many ? $internal_objects : [$internal_objects];
+
+        # Loop through each internal object and create a Model object for it
+        foreach ($internal_objects as $internal_id => $internal_object) {
+            # Do not include this object if we have not reached our offset
+            if ($offset_counter < $offset) {
+                $offset_counter++;
+                continue;
+            }
+
+            # Create a new Model object for this internal object and assign its ID
+            $model_object = new $model(id: $internal_id, parent_id: $parent_id, skip_init: true);
+
+            # Obtain the parent Model object if this Model has a parent Model class assigned
+            $model_object->get_parent_model();
+
+            # Populate the Model object using its current internal values and add it to the array of all Model objects
+            $model_object->from_internal_object($internal_object);
+            $model_objects[] = $model_object;
+
+            # Break the loop if $limit is set, and we've met our limit of objects to obtain
+            if ($limit and $limit === count($model_objects)) {
+                break;
+            }
+
+            # Increase the offset counter
+            $offset_counter++;
+        }
+
+        # Unwrap the array for non `many` Models, otherwise return all objects
+        return $model->many ? new ModelSet($model_objects) : $model_objects[0];
+    }
+
     /**
      * Performs a query on all Model objects for this Model. This is essentially a shorthand way of calling
      * `query()`. This method is only applicable to `many` Models.
@@ -1766,10 +1771,43 @@ class Model {
         # Merge the $query_params and any provided variable-length arguments into a single variable
         $query_params = array_merge($query_params, $vl_query_params);
 
-        return self::read_all(parent_id: $parent_id, limit: $limit, offset: $offset)->query(
-            query_params: $query_params,
-            excluded: $excluded,
-        );
+        # If no query parameters were provided, just run read_all() with pagination for optimal performance
+        if (!$query_params) {
+            return self::read_all(parent_id: $parent_id, limit: $limit, offset: $offset);
+        }
+
+        # Perform the query against all Model objects for this Model first
+        $modelset = self::read_all(parent_id: $parent_id)->query(query_params: $query_params, excluded: $excluded);
+
+        # Apply pagination to limit the number of objects returned if requested
+        $modelset->model_objects = self::paginate($modelset->model_objects, $limit, $offset);
+        return $modelset;
+    }
+
+    /**
+     * Paginates a given array but obtaining a smaller subset of the array based on the provided limit and offset values.
+     * @param int $limit The maximum number of items to return in the paginated array. Use 0 to impose no limit.
+     * @param int $offset The starting point in the array to begin the paginated array.
+     * @return array The paginated array containing only the subset of items that match the limit and offset values.
+     */
+    public static function paginate(array $array, int $limit, int $offset): array {
+        # Return the entire array if no limit or offset is set
+        if (!$limit and !$offset) {
+            return $array;
+        }
+
+        # Return an empty array if the offset is greater than the array length
+        if ($offset >= count($array)) {
+            return [];
+        }
+
+        # Return the entire array if the limit is set to 0
+        if (!$limit) {
+            return array_slice($array, $offset);
+        }
+
+        # Return the subset of the array that matches the limit and offset values
+        return array_slice($array, $offset, $limit);
     }
 
     /**
diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/ModelSet.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/ModelSet.inc
@@ -45,6 +45,14 @@ class ModelSet {
         return count($this->model_objects) > 0;
     }
 
+    /**
+     * Returns the number of Model objects in this ModelSet.
+     * @return int The number of Model objects in this ModelSet.
+     */
+    public function count(): int {
+        return count($this->model_objects);
+    }
+
     /**
      * Returns the first Model object in the ModelSet. This is helpful when locating the first Model object that matched
      * a query.
diff --git a/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APICoreModelTestCase.inc b/pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APICoreModelTestCase.inc
@@ -764,4 +764,47 @@ class APICoreModelTestCase extends RESTAPI\Core\TestCase {
         # Delete the alias
         $alias->delete();
     }
+
+    /**
+     * Checks that the 'paginate()' method works correctly.
+     */
+    public function test_paginate(): void {
+        # Ensure the paginate method correctly returns the paginated subset of a given array
+        $this->assert_equals(Model::paginate([1, 2, 3, 4, 5], limit: 2, offset: 1), [2, 3]);
+        $this->assert_equals(Model::paginate([1, 2, 3, 4, 5], limit: 2, offset: 3), [4, 5]);
+        $this->assert_equals(Model::paginate([1, 2, 3, 4, 5], limit: 2, offset: 5), []);
+        $this->assert_equals(Model::paginate([1, 2, 3, 4, 5], limit: 4, offset: 0), [1, 2, 3, 4]);
+        $this->assert_equals(Model::paginate([1, 2, 3, 4, 5], limit: 4, offset: 1), [2, 3, 4, 5]);
+
+        # Ensure a limit of 0 imposes no limit
+        $this->assert_equals(Model::paginate([1, 2, 3, 4, 5], 0, 0), [1, 2, 3, 4, 5]);
+    }
+
+    /**
+     * Ensures that queries are performed before pagination is applied.
+     */
+    public function test_paginate_with_query(): void {
+        # Create FirewallAlias models to test this
+        $alias_alias_0 = new FirewallAlias(name: 'alias0', type: 'host');
+        $alias_alias_0->create();
+        $alias_alias_1 = new FirewallAlias(name: 'alias1', type: 'port');
+        $alias_alias_1->create();
+        $alias_alias_2 = new FirewallAlias(name: 'alias2', type: 'network');
+        $alias_alias_2->create();
+        $alias_alias_3 = new FirewallAlias(name: 'alias3', type: 'host');
+        $alias_alias_3->create();
+        $alias_alias_4 = new FirewallAlias(name: 'alias4', type: 'port');
+        $alias_alias_4->create();
+
+        # Query for only port aliases, but limit it to 2
+        $port_aliases = FirewallAlias::query(limit: 2, type: 'port');
+        $this->assert_equals($port_aliases->count(), 2);
+        $this->assert_equals($port_aliases->model_objects[0]->name->value, 'alias1');
+        $this->assert_equals($port_aliases->model_objects[1]->name->value, 'alias4');
+
+        # Query again but change to offset to exclude the first port alias
+        $port_aliases = FirewallAlias::query(limit: 2, offset: 1, type: 'port');
+        $this->assert_equals($port_aliases->count(), 1);
+        $this->assert_equals($port_aliases->model_objects[0]->name->value, 'alias4');
+    }
 }
