Skip to content

Commit 0621d84

Browse files
committed
Return tests to passing now that we don't ignore a critical event...
1 parent 09832cc commit 0621d84

File tree

14 files changed

+44
-60
lines changed

14 files changed

+44
-60
lines changed

app/Models/Model.php

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use Illuminate\Validation\Rule;
88
use Illuminate\Container\Container;
99
use Illuminate\Contracts\Validation\Factory;
10+
use Illuminate\Validation\ValidationException;
1011
use Illuminate\Database\Eloquent\Factories\HasFactory;
1112
use Pterodactyl\Exceptions\Model\DataValidationException;
1213
use Illuminate\Database\Eloquent\Model as IlluminateModel;
@@ -30,13 +31,6 @@ abstract class Model extends IlluminateModel
3031
*/
3132
protected $skipValidation = false;
3233

33-
/**
34-
* The validator instance used by this model.
35-
*
36-
* @var \Illuminate\Validation\Validator
37-
*/
38-
protected $validator;
39-
4034
/**
4135
* @var \Illuminate\Contracts\Validation\Factory
4236
*/
@@ -60,8 +54,10 @@ protected static function boot()
6054
static::$validatorFactory = Container::getInstance()->make(Factory::class);
6155

6256
static::saving(function (Model $model) {
63-
if (!$model->validate()) {
64-
throw new DataValidationException($model->getValidator());
57+
try {
58+
$model->validate();
59+
} catch (ValidationException $exception) {
60+
throw new DataValidationException($exception->validator);
6561
}
6662

6763
return true;
@@ -101,14 +97,9 @@ public function skipValidation()
10197
*/
10298
public function getValidator()
10399
{
104-
$rules = $this->getKey() ? static::getRulesForUpdate($this) : static::getRules();
100+
$rules = $this->exists ? static::getRulesForUpdate($this) : static::getRules();
105101

106-
return $this->validator ?: $this->validator = static::$validatorFactory->make(
107-
[],
108-
$rules,
109-
[],
110-
[]
111-
);
102+
return static::$validatorFactory->make([], $rules, [], []);
112103
}
113104

114105
/**
@@ -139,14 +130,14 @@ public static function getRulesForField(string $field): array
139130
* Returns the rules associated with the model, specifically for updating the given model
140131
* rather than just creating it.
141132
*
142-
* @param \Illuminate\Database\Eloquent\Model|int|string $id
133+
* @param \Illuminate\Database\Eloquent\Model|int|string $model
143134
*
144135
* @return array
145136
*/
146-
public static function getRulesForUpdate($id, string $primaryKey = 'id')
137+
public static function getRulesForUpdate($model, string $column = 'id')
147138
{
148-
if ($id instanceof Model) {
149-
[$primaryKey, $id] = [$id->getKeyName(), $id->getKey()];
139+
if ($model instanceof Model) {
140+
[$id, $column] = [$model->getKey(), $model->getKeyName()];
150141
}
151142

152143
$rules = static::getRules();
@@ -163,7 +154,7 @@ public static function getRulesForUpdate($id, string $primaryKey = 'id')
163154
[, $args] = explode(':', $datum);
164155
$args = explode(',', $args);
165156

166-
$datum = Rule::unique($args[0], $args[1] ?? $key)->ignore($id, $primaryKey)->__toString();
157+
$datum = Rule::unique($args[0], $args[1] ?? $key)->ignore($id ?? $model, $column);
167158
}
168159
}
169160

@@ -172,24 +163,27 @@ public static function getRulesForUpdate($id, string $primaryKey = 'id')
172163

173164
/**
174165
* Determines if the model is in a valid state or not.
175-
*
176-
* @return bool
177166
*/
178-
public function validate()
167+
public function validate(): void
179168
{
180169
if ($this->skipValidation) {
181-
return true;
170+
return;
182171
}
183172

184-
return $this->getValidator()->setData(
173+
$validator = $this->getValidator();
174+
$validator->setData(
185175
// Trying to do self::toArray() here will leave out keys based on the whitelist/blacklist
186176
// for that model. Doing this will return all of the attributes in a format that can
187177
// properly be validated.
188178
$this->addCastAttributesToArray(
189179
$this->getAttributes(),
190180
$this->getMutatedAttributes()
191181
)
192-
)->passes();
182+
);
183+
184+
if (!$validator->passes()) {
185+
throw new ValidationException($validator);
186+
}
193187
}
194188

195189
/**

app/Services/Activity/ActivityLogTargetableService.php

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
namespace Pterodactyl\Services\Activity;
44

5-
use InvalidArgumentException;
65
use Illuminate\Database\Eloquent\Model;
76

87
class ActivityLogTargetableService
@@ -13,19 +12,11 @@ class ActivityLogTargetableService
1312

1413
public function setActor(Model $actor): void
1514
{
16-
if (!is_null($this->actor)) {
17-
throw new InvalidArgumentException('Cannot call ' . __METHOD__ . ' when an actor is already set on the instance.');
18-
}
19-
2015
$this->actor = $actor;
2116
}
2217

2318
public function setSubject(Model $subject): void
2419
{
25-
if (!is_null($this->subject)) {
26-
throw new InvalidArgumentException('Cannot call ' . __METHOD__ . ' when a target is already set on the instance.');
27-
}
28-
2920
$this->subject = $subject;
3021
}
3122

database/Factories/ServerFactory.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ public function definition()
4040
'oom_disabled' => 0,
4141
'startup' => '/bin/bash echo "hello world"',
4242
'image' => 'foo/bar:latest',
43-
'allocation_limit' => 0,
44-
'database_limit' => 0,
43+
'allocation_limit' => null,
44+
'database_limit' => null,
4545
'backup_limit' => 0,
4646
'created_at' => Carbon::now(),
4747
'updated_at' => Carbon::now(),

database/Factories/TaskFactory.php

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,25 +2,17 @@
22

33
namespace Database\Factories;
44

5-
use Pterodactyl\Models\Task;
65
use Illuminate\Database\Eloquent\Factories\Factory;
76

87
class TaskFactory extends Factory
98
{
10-
/**
11-
* The name of the factory's corresponding model.
12-
*
13-
* @var string
14-
*/
15-
protected $model = Task::class;
16-
179
/**
1810
* Define the model's default state.
1911
*/
2012
public function definition(): array
2113
{
2214
return [
23-
'sequence_id' => $this->faker->randomNumber(1),
15+
'sequence_id' => $this->faker->numberBetween(1, 10),
2416
'action' => 'command',
2517
'payload' => 'test command',
2618
'time_offset' => 120,

database/Factories/UserFactory.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Carbon\Carbon;
66
use Ramsey\Uuid\Uuid;
7+
use Illuminate\Support\Str;
78
use Pterodactyl\Models\User;
89
use Illuminate\Database\Eloquent\Factories\Factory;
910

@@ -24,10 +25,10 @@ public function definition(): array
2425
static $password;
2526

2627
return [
27-
'external_id' => $this->faker->unique()->isbn10,
28+
'external_id' => null,
2829
'uuid' => Uuid::uuid4()->toString(),
2930
'username' => $this->faker->unique()->userName,
30-
'email' => $this->faker->unique()->safeEmail,
31+
'email' => Str::random(32) . '@example.com',
3132
'name_first' => $this->faker->firstName,
3233
'name_last' => $this->faker->lastName,
3334
'password' => $password ?: $password = bcrypt('password'),

tests/Integration/Api/Application/Users/ExternalUserControllerTest.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Pterodactyl\Tests\Integration\Api\Application\Users;
44

5+
use Illuminate\Support\Str;
56
use Pterodactyl\Models\User;
67
use Illuminate\Http\Response;
78
use Pterodactyl\Tests\Integration\Api\Application\ApplicationApiIntegrationTestCase;
@@ -13,7 +14,7 @@ class ExternalUserControllerTest extends ApplicationApiIntegrationTestCase
1314
*/
1415
public function testGetRemoteUser()
1516
{
16-
$user = User::factory()->create();
17+
$user = User::factory()->create(['external_id' => Str::random()]);
1718

1819
$response = $this->getJson('/api/application/users/external/' . $user->external_id);
1920
$response->assertStatus(Response::HTTP_OK);
@@ -60,7 +61,7 @@ public function testGetMissingUser()
6061
*/
6162
public function testErrorReturnedIfNoPermission()
6263
{
63-
$user = User::factory()->create();
64+
$user = User::factory()->create(['external_id' => Str::random()]);
6465
$this->createNewDefaultApiKey($this->getApiUser(), ['r_users' => 0]);
6566

6667
$response = $this->getJson('/api/application/users/external/' . $user->external_id);

tests/Integration/Api/Client/AccountControllerTest.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Pterodactyl\Tests\Integration\Api\Client;
44

5+
use Illuminate\Support\Str;
56
use Pterodactyl\Models\User;
67
use Illuminate\Http\Response;
78
use Illuminate\Support\Facades\Hash;
@@ -41,13 +42,13 @@ public function testEmailIsUpdated()
4142
$user = User::factory()->create();
4243

4344
$response = $this->actingAs($user)->putJson('/api/client/account/email', [
44-
'email' => 'hodor@example.com',
45+
'email' => $email = Str::random() . '@example.com',
4546
'password' => 'password',
4647
]);
4748

4849
$response->assertStatus(Response::HTTP_NO_CONTENT);
4950

50-
$this->assertDatabaseHas('users', ['id' => $user->id, 'email' => 'hodor@example.com']);
51+
$this->assertDatabaseHas('users', ['id' => $user->id, 'email' => $email]);
5152
}
5253

5354
/**

tests/Integration/Api/Client/Server/Schedule/DeleteServerScheduleTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public function testNotFoundErrorIsReturnedIfScheduleDoesNotExistAtAll()
5050
public function testNotFoundErrorIsReturnedIfScheduleDoesNotBelongToServer()
5151
{
5252
[$user, $server] = $this->generateTestAccount();
53-
[, $server2] = $this->generateTestAccount(['user_id' => $user->id]);
53+
$server2 = $this->createServerModel(['owner_id' => $user->id]);
5454

5555
$schedule = Schedule::factory()->create(['server_id' => $server2->id]);
5656

tests/Integration/Api/Client/Server/Schedule/GetServerSchedulesTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public function testServerSchedulesAreReturned($permissions, $individual)
6464
public function testScheduleBelongingToAnotherServerCannotBeViewed()
6565
{
6666
[$user, $server] = $this->generateTestAccount();
67-
[, $server2] = $this->generateTestAccount(['user_id' => $user->id]);
67+
$server2 = $this->createServerModel(['owner_id' => $user->id]);
6868

6969
$schedule = Schedule::factory()->create(['server_id' => $server2->id]);
7070

tests/Integration/Api/Client/Server/Schedule/UpdateServerScheduleTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public function testScheduleCanBeUpdated($permissions)
5858
public function testErrorIsReturnedIfScheduleDoesNotBelongToServer()
5959
{
6060
[$user, $server] = $this->generateTestAccount();
61-
[, $server2] = $this->generateTestAccount(['user_id' => $user->id]);
61+
$server2 = $this->createServerModel(['owner_id' => $user->id]);
6262

6363
$schedule = Schedule::factory()->create(['server_id' => $server2->id]);
6464

0 commit comments

Comments
 (0)