Skip to content

Commit 2a62c8a

Browse files
feat: implement model caching and indexing
1 parent af2d0e1 commit 2a62c8a

File tree

4 files changed

+293
-41
lines changed

4 files changed

+293
-41
lines changed

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/Model.inc

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -986,9 +986,6 @@ class Model {
986986
# When `many` is enabled, obtain the object with our ID. Otherwise, just assign the direct object
987987
$internal_object = $this->many ? $internal_objects[$this->id] : $internal_objects;
988988
$this->from_internal_object($internal_object);
989-
990-
# Re-index the ModelCache for this object since it has been refreshed from internal
991-
self::get_model_cache()::cache_model($this);
992989
}
993990

994991
/**
@@ -1946,7 +1943,7 @@ class Model {
19461943
$model = new $model_name();
19471944
$model_objects = [];
19481945
$requests_pagination = ($limit or $offset);
1949-
$cache_exempt = ($requests_pagination or $reverse);
1946+
$cache_exempt = ($requests_pagination or $reverse or $model->internal_callable);
19501947

19511948
# Throw an error if pagination was requested on a Model without $many enabled
19521949
if (!$model->many and $requests_pagination) {
@@ -2102,9 +2099,6 @@ class Model {
21022099
# Obtain the requested Model object from config by ID
21032100
$this->from_internal();
21042101

2105-
# Cache the newly read Model object
2106-
self::get_model_cache()::cache_model($this);
2107-
21082102
return $this;
21092103
}
21102104

@@ -2467,7 +2461,6 @@ class Model {
24672461
public static function delete_many(
24682462
array $query_params = [],
24692463
array $excluded = [],
2470-
mixed $parent_id = null,
24712464
int $limit = 0,
24722465
int $offset = 0,
24732466
...$vl_query_params,
@@ -2487,7 +2480,7 @@ class Model {
24872480
$model_objects = self::query(query_params: $query_params, excluded: $excluded, limit: $limit, offset: $offset);
24882481

24892482
# Reset the object cache as the config has changed
2490-
Model::clear_model_cache();
2483+
self::clear_model_cache();
24912484

24922485
# Delete the Model objects that matched the query
24932486
return $model_objects->delete();
@@ -2501,7 +2494,7 @@ class Model {
25012494
$model_objects = self::read_all();
25022495

25032496
# Reset the object cache as the config has changed
2504-
Model::clear_model_cache();
2497+
self::clear_model_cache();
25052498

25062499
# Delete all Model objects for this Model
25072500
return $model_objects->delete();

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Core/ModelCache.inc

Lines changed: 50 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace RESTAPI\Core;
44

55
use RESTAPI\Responses\NotFoundError;
6+
use RESTAPI\Responses\ServerError;
67

78
require_once 'RESTAPI/autoloader.inc';
89

@@ -82,6 +83,51 @@ class ModelCache {
8283
return self::$cache[$model_class];
8384
}
8485

86+
/**
87+
* Raises an error if the given Model object does not support being indexed by the specified field.
88+
* @param Model $model The Model object to check for uniqueness.
89+
* @param string $index_field The index field name to check for uniqueness.
90+
* @throws ServerError If the Model is attempting to be indexed by a non-unique field.
91+
* @throws ServerError If the Model is not many-enabled.
92+
*/
93+
private static function ensure_model_supports_indexing(Model $model, string $index_field): void {
94+
# Only many-enabled Models can be indexed
95+
if (!$model->many) {
96+
throw new ServerError(
97+
message: "Cannot index Model class '" . $model->get_class_fqn() . 'because it is not many-enabled.',
98+
response_id: 'MODEL_CACHE_INDEX_FIELD_ON_NON_MANY_MODEL',
99+
);
100+
}
101+
102+
# Models with parent model classes cannot be indexed
103+
if ($model->parent_model_class) {
104+
throw new ServerError(
105+
message: "Cannot index Model class '" .
106+
$model->get_class_fqn() .
107+
"' because it has a parent model class '" .
108+
$model->parent_model_class .
109+
"'.",
110+
response_id: 'MODEL_CACHE_INDEX_FIELD_ON_PARENTED_MODEL',
111+
);
112+
}
113+
114+
# If indexing by 'id', it's always unique
115+
if ($index_field === 'id') {
116+
return;
117+
}
118+
119+
# Check if the index field is unique on the Model object
120+
if (!$model->$index_field->unique) {
121+
throw new ServerError(
122+
message: "Cannot index Model class '" .
123+
$model->get_class_fqn() .
124+
"' by non-unique field " .
125+
"'$index_field'.",
126+
response_id: 'MODEL_CACHE_INDEX_FIELD_NOT_UNIQUE',
127+
);
128+
}
129+
}
130+
85131
/**
86132
* Indexes the ModelSet cache for a given Model class by a specified field. This method will populate
87133
* the $index array for the specified Model class and index field.
@@ -96,8 +142,11 @@ class ModelCache {
96142
return self::$index[$model_class][$index_field];
97143
}
98144

99-
# Ensure a cached ModelSet exists for the specified Model class
145+
# Ensure the Model can be indexed by the specified field
100146
$model = new $model_class();
147+
self::ensure_model_supports_indexing($model, $index_field);
148+
149+
# Fetch or create the ModelSet for this Model class
101150
$model_set = $model->read_all();
102151

103152
# Create an associative array to hold the indexed Model objects
@@ -136,36 +185,6 @@ class ModelCache {
136185
return isset(self::$index[$model_class][$index_field][$index_value]);
137186
}
138187

139-
/**
140-
* Caches a Model object in the ModelCache by its index field/value.
141-
* @param Model $model The Model object to cache.
142-
* @param string $index_field The field to index the Model object by. Only used for many enabled Models.
143-
*/
144-
public static function cache_model(Model $model, string $index_field = 'id'): void {
145-
# Determine the Model class of the Model object
146-
$model_class = $model->get_class_fqn();
147-
148-
# Ensure an index collection exists for this model's class
149-
if (!isset(self::$index[$model_class])) {
150-
self::$index[$model_class] = [];
151-
}
152-
153-
# Ensure an index exists for this model's index field if many is enabled
154-
if ($model->many and !isset(self::$index[$model_class][$index_field])) {
155-
self::$index[$model_class][$index_field] = [];
156-
}
157-
158-
# For many enabled models, index the model under the specified index field and value
159-
if ($model->many) {
160-
$index_value = $index_field === 'id' ? $model->id : $model->$index_field->value;
161-
self::$index[$model_class][$index_field][$index_value] = $model;
162-
return;
163-
}
164-
165-
# Otherwise, index the model under the Model class only
166-
self::$index[$model_class] = $model;
167-
}
168-
169188
/**
170189
* Fetches a cached Model object by its Model class name and index field/value.
171190
* @param string $model_class The Model class name to load from the cache.
Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
<?php
2+
3+
namespace RESTAPI\Tests;
4+
5+
use RESTAPI\Core\ModelCache;
6+
use RESTAPI\Core\ModelSet;
7+
use RESTAPI\Core\TestCase;
8+
use RESTAPI\Models\DNSResolverHostOverrideAlias;
9+
use RESTAPI\Models\FirewallAlias;
10+
use RESTAPI\Models\RESTAPISettings;
11+
12+
/**
13+
* Tests for the RESTAPI\Core\ModelCache class. These tests ensure that the central ModelCache object correctly
14+
* caches and retrieves Model objects as expected.
15+
*/
16+
class APICoreModelCacheTestCase extends TestCase {
17+
/**
18+
* Ensures that the ModelCache is always a singleton instance.
19+
*/
20+
public function test_model_cache_singleton_instance(): void {
21+
$instance_1 = ModelCache::get_instance();
22+
$instance_2 = ModelCache::get_instance();
23+
24+
$this->assert_equals($instance_1, $instance_2);
25+
}
26+
27+
/**
28+
* Ensures the has_modelset() method correctly identifies if a given Model class is cached in the ModelCache.
29+
*/
30+
public function test_model_cache_has_modelset(): void {
31+
# Obtain and clear the ModelCache
32+
$model_cache = ModelCache::get_instance();
33+
$model_cache::clear();
34+
35+
# Ensure the ModelCache does not have a cached ModelSet for the MockModelClass
36+
$this->assert_is_false($model_cache::has_modelset('MockModelClass'));
37+
38+
# Populate the ModelCache with a mock ModelSet for testing
39+
ModelCache::$cache['MockModelClass'] = new ModelSet();
40+
$this->assert_is_true($model_cache::has_modelset('MockModelClass'));
41+
42+
# Clear the ModelCache and ensure the ModelSet is removed
43+
$model_cache::clear();
44+
$this->assert_is_false($model_cache::has_modelset('MockModelClass'));
45+
}
46+
47+
/**
48+
* Ensures the cache_modelset() correctly caches a ModelSet in the ModelCache.
49+
*/
50+
public function test_model_cache_cache_modelset(): void {
51+
# Obtain and clear the ModelCache
52+
$model_cache = ModelCache::get_instance();
53+
$model_cache::clear();
54+
55+
# Create a mock ModelSet to cache
56+
$mock_model_set = new ModelSet();
57+
58+
# Cache the mock ModelSet in the ModelCache
59+
$model_cache::cache_modelset('MockModelClass', $mock_model_set);
60+
61+
# Ensure the ModelSet was cached correctly
62+
$this->assert_is_true($model_cache::has_modelset('MockModelClass'));
63+
$this->assert_equals($model_cache::fetch_modelset('MockModelClass'), $mock_model_set);
64+
65+
# Clear the ModelCache
66+
$model_cache::clear();
67+
}
68+
69+
/**
70+
* Ensures the fetch_modelset() method correctly retrieves a cached ModelSet from the ModelCache.
71+
*/
72+
public function test_model_cache_fetch_nonexisting_modelset_throws_error(): void {
73+
# Obtain and clear the ModelCache
74+
$model_cache = ModelCache::get_instance();
75+
$model_cache::clear();
76+
77+
# Ensure fetching a non-cached ModelSet throws an error
78+
$this->assert_throws_response(
79+
response_id: 'MODEL_CACHE_MODELSET_NOT_FOUND',
80+
code: 404,
81+
callable: function () {
82+
ModelCache::fetch_modelset('MockModelClass');
83+
},
84+
);
85+
}
86+
87+
/**
88+
* Ensures Models must be many-enabled to be indexed in the ModelCache.
89+
*/
90+
public function test_model_index_must_be_many_enabled(): void {
91+
# Ensure attempting to index a non-many enabled Model throws an error
92+
$this->assert_throws_response(
93+
response_id: 'MODEL_CACHE_INDEX_FIELD_ON_NON_MANY_MODEL',
94+
code: 500,
95+
callable: function () {
96+
ModelCache::index_modelset_by_field(model_class: RESTAPISettings::get_class_fqn(), index_field: 'id');
97+
},
98+
);
99+
}
100+
101+
/**
102+
* Ensures we cannot index a cached ModelSet by a non-unique field.
103+
*/
104+
public function test_model_index_field_must_be_unique(): void {
105+
# Ensure indexing by id is always allowed
106+
$this->assert_does_not_throw(function () {
107+
ModelCache::index_modelset_by_field(model_class: FirewallAlias::get_class_fqn(), index_field: 'id');
108+
});
109+
110+
# Ensure attempting to index the Model by the non-unique field throws an error
111+
$this->assert_throws_response(
112+
response_id: 'MODEL_CACHE_INDEX_FIELD_NOT_UNIQUE',
113+
code: 500,
114+
callable: function () {
115+
ModelCache::index_modelset_by_field(model_class: FirewallAlias::get_class_fqn(), index_field: 'descr');
116+
},
117+
);
118+
}
119+
120+
/**
121+
* Ensures we cannot index a cached ModelSet for a Model that has a parent model class.
122+
*/
123+
public function test_model_index_field_cannot_have_parent_model_class(): void {
124+
# Ensure attempting to index the Model by the non-unique field throws an error
125+
$this->assert_throws_response(
126+
response_id: 'MODEL_CACHE_INDEX_FIELD_ON_PARENTED_MODEL',
127+
code: 500,
128+
callable: function () {
129+
ModelCache::index_modelset_by_field(
130+
model_class: DNSResolverHostOverrideAlias::get_class_fqn(),
131+
index_field: 'id',
132+
);
133+
},
134+
);
135+
}
136+
137+
/**
138+
* Ensures the index_modelset_by_field() method correctly indexes a cached ModelSet by the specified field. This
139+
* test also ensures the has_model() and fetch_model() methods work as expected for indexed Model objects.
140+
*/
141+
public function test_model_index_modelset_by_field(): void {
142+
# Create FirewallAlias model objects to test with
143+
$alias1 = new FirewallAlias(data: ['name' => 'host_alias', 'descr' => 'First alias', 'type' => 'host']);
144+
$alias2 = new FirewallAlias(data: ['name' => 'network_alias', 'descr' => 'Second alias', 'type' => 'network']);
145+
$alias3 = new FirewallAlias(data: ['name' => 'port_alias', 'descr' => 'Third alias', 'type' => 'port']);
146+
$alias1->create();
147+
$alias2->create();
148+
$alias3->create();
149+
150+
# Index the FirewallAlias ModelSet by the 'name' field
151+
ModelCache::index_modelset_by_field(model_class: FirewallAlias::get_class_fqn(), index_field: 'name');
152+
153+
# Ensure the ModelCache has the indexed Models
154+
$this->assert_is_true(
155+
ModelCache::has_model(
156+
model_class: FirewallAlias::get_class_fqn(),
157+
index_field: 'name',
158+
index_value: 'host_alias',
159+
),
160+
);
161+
$this->assert_is_true(
162+
ModelCache::has_model(
163+
model_class: FirewallAlias::get_class_fqn(),
164+
index_field: 'name',
165+
index_value: 'network_alias',
166+
),
167+
);
168+
$this->assert_is_true(
169+
ModelCache::has_model(
170+
model_class: FirewallAlias::get_class_fqn(),
171+
index_field: 'name',
172+
index_value: 'port_alias',
173+
),
174+
);
175+
176+
# Fetch the indexed Models and ensure they are correct
177+
$fetched_alias1 = ModelCache::fetch_model(
178+
model_class: FirewallAlias::get_class_fqn(),
179+
index_field: 'name',
180+
index_value: 'host_alias',
181+
);
182+
$this->assert_equals($fetched_alias1->name->value, 'host_alias');
183+
$fetched_alias2 = ModelCache::fetch_model(
184+
model_class: FirewallAlias::get_class_fqn(),
185+
index_field: 'name',
186+
index_value: 'network_alias',
187+
);
188+
$this->assert_equals($fetched_alias2->name->value, 'network_alias');
189+
$fetched_alias3 = ModelCache::fetch_model(
190+
model_class: FirewallAlias::get_class_fqn(),
191+
index_field: 'name',
192+
index_value: 'port_alias',
193+
);
194+
$this->assert_equals($fetched_alias3->name->value, 'port_alias');
195+
196+
# Clean up the created aliases
197+
FirewallAlias::delete_all();
198+
}
199+
}

0 commit comments

Comments
 (0)