Skip to content

Commit 241f7d0

Browse files
DaneEverittDaneEveritt
committed
Fix some data integrity issues
1 parent d52f8d9 commit 241f7d0

File tree

6 files changed

+41
-37
lines changed

6 files changed

+41
-37
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ This project follows [Semantic Versioning](http://semver.org) guidelines.
1212
* `[rc.2]` — Fix data integrity exception that could occur when an email containing non-username characters was passed.
1313
* `[rc.2]` — Fix data integrity exception occurring when no default value is provided for an egg variable.
1414
* `[rc.2]` — Fixes a bug that would cause non-editable variables on the front-end to throw a validation error.
15+
* `[rc.2]` — Fixes a data integrity exception occurring when saving egg variables with no value.
1516

1617
### Added
1718
* Added ability to search the following API endpoints: list users, list servers, and list locations.

‎app/Models/EggVariable.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class EggVariable extends Model implements CleansAttributes, ValidableContract
6565
protected static $dataIntegrityRules = [
6666
'egg_id' => 'exists:eggs,id',
6767
'name' => 'string|between:1,255',
68-
'description' => 'nullable|string',
68+
'description' => 'string',
6969
'env_variable' => 'regex:/^[\w]{1,255}$/|notIn:' . self::RESERVED_ENV_NAMES,
7070
'default_value' => 'string',
7171
'user_viewable' => 'boolean',

‎app/Services/Eggs/Variables/VariableCreationService.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,15 @@ public function handle(int $egg, array $data): EggVariable
4444

4545
$options = array_get($data, 'options') ?? [];
4646

47-
return $this->repository->create(array_merge($data, [
47+
return $this->repository->create([
4848
'egg_id' => $egg,
49+
'name' => $data['name'] ?? '',
50+
'description' => $data['description'] ?? '',
51+
'env_variable' => $data['env_variable'] ?? '',
52+
'default_value' => $data['default_value'] ?? '',
4953
'user_viewable' => in_array('user_viewable', $options),
5054
'user_editable' => in_array('user_editable', $options),
51-
]));
55+
'rules' => $data['rules'] ?? '',
56+
]);
5257
}
5358
}

‎app/Services/Eggs/Variables/VariableUpdateService.php‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public function handle(EggVariable $variable, array $data)
4646
}
4747

4848
$search = $this->repository->setColumns('id')->findCountWhere([
49-
['env_variable', '=', array_get($data, 'env_variable')],
49+
['env_variable', '=', $data['env_variable']],
5050
['egg_id', '=', $variable->egg_id],
5151
['id', '!=', $variable->id],
5252
]);
@@ -60,10 +60,14 @@ public function handle(EggVariable $variable, array $data)
6060

6161
$options = array_get($data, 'options') ?? [];
6262

63-
return $this->repository->withoutFreshModel()->update($variable->id, array_merge($data, [
64-
'default_value' => array_get($data, 'default_value') ?? '',
63+
return $this->repository->withoutFreshModel()->update($variable->id, [
64+
'name' => $data['name'] ?? '',
65+
'description' => $data['description'] ?? '',
66+
'env_variable' => $data['env_variable'] ?? '',
67+
'default_value' => $data['default_value'] ?? '',
6568
'user_viewable' => in_array('user_viewable', $options),
6669
'user_editable' => in_array('user_editable', $options),
67-
]));
70+
'rules' => $data['rules'] ?? '',
71+
]);
6872
}
6973
}

‎tests/Unit/Services/Eggs/Variables/VariableCreationServiceTest.php‎

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,14 @@ public function setUp()
3737
*/
3838
public function testVariableIsCreatedAndStored()
3939
{
40-
$data = ['env_variable' => 'TEST_VAR_123'];
41-
$this->repository->shouldReceive('create')->with([
40+
$data = ['env_variable' => 'TEST_VAR_123', 'default_value' => 'test'];
41+
$this->repository->shouldReceive('create')->with(m::subset([
4242
'egg_id' => 1,
43+
'default_value' => 'test',
4344
'user_viewable' => false,
4445
'user_editable' => false,
4546
'env_variable' => 'TEST_VAR_123',
46-
])->once()->andReturn(new EggVariable);
47+
]))->once()->andReturn(new EggVariable);
4748

4849
$this->assertInstanceOf(EggVariable::class, $this->service->handle(1, $data));
4950
}
@@ -54,33 +55,31 @@ public function testVariableIsCreatedAndStored()
5455
public function testOptionsPassedInArrayKeyAreParsedProperly()
5556
{
5657
$data = ['env_variable' => 'TEST_VAR_123', 'options' => ['user_viewable', 'user_editable']];
57-
$this->repository->shouldReceive('create')->with([
58-
'egg_id' => 1,
58+
$this->repository->shouldReceive('create')->with(m::subset([
59+
'default_value' => '',
5960
'user_viewable' => true,
6061
'user_editable' => true,
6162
'env_variable' => 'TEST_VAR_123',
62-
'options' => ['user_viewable', 'user_editable'],
63-
])->once()->andReturn(new EggVariable);
63+
]))->once()->andReturn(new EggVariable);
6464

6565
$this->assertInstanceOf(EggVariable::class, $this->service->handle(1, $data));
6666
}
6767

6868
/**
6969
* Test that an empty (null) value passed in the option key is handled
70-
* properly as an array.
70+
* properly as an array. Also tests the same case aganist the default_value.
7171
*
7272
* @see https://github.com/Pterodactyl/Panel/issues/841
73+
* @see https://github.com/Pterodactyl/Panel/issues/943
7374
*/
7475
public function testNullOptionValueIsPassedAsArray()
7576
{
76-
$data = ['env_variable' => 'TEST_VAR_123', 'options' => null];
77-
$this->repository->shouldReceive('create')->with([
78-
'egg_id' => 1,
77+
$data = ['env_variable' => 'TEST_VAR_123', 'options' => null, 'default_value' => null];
78+
$this->repository->shouldReceive('create')->with(m::subset([
79+
'default_value' => '',
7980
'user_viewable' => false,
8081
'user_editable' => false,
81-
'env_variable' => 'TEST_VAR_123',
82-
'options' => null,
83-
])->once()->andReturn(new EggVariable);
82+
]))->once()->andReturn(new EggVariable);
8483

8584
$this->assertInstanceOf(EggVariable::class, $this->service->handle(1, $data));
8685
}
@@ -103,12 +102,9 @@ public function testExceptionIsThrownIfEnvironmentVariableIsInListOfReservedName
103102
public function testEggIdPassedInDataIsNotApplied()
104103
{
105104
$data = ['egg_id' => 123456, 'env_variable' => 'TEST_VAR_123'];
106-
$this->repository->shouldReceive('create')->with([
105+
$this->repository->shouldReceive('create')->with(m::subset([
107106
'egg_id' => 1,
108-
'user_viewable' => false,
109-
'user_editable' => false,
110-
'env_variable' => 'TEST_VAR_123',
111-
])->once()->andReturn(new EggVariable);
107+
]))->once()->andReturn(new EggVariable);
112108

113109
$this->assertInstanceOf(EggVariable::class, $this->service->handle(1, $data));
114110
}

‎tests/Unit/Services/Eggs/Variables/VariableUpdateServiceTest.php‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,9 @@ public function testVariableIsUpdatedWhenNoEnvironmentVariableIsPassed()
4949
->shouldReceive('update')->with($this->model->id, m::subset([
5050
'user_viewable' => false,
5151
'user_editable' => false,
52-
'test-data' => 'test-value',
5352
]))->once()->andReturn(true);
5453

55-
$this->assertTrue($this->service->handle($this->model, ['test-data' => 'test-value']));
54+
$this->assertTrue($this->service->handle($this->model, []));
5655
}
5756

5857
/**
@@ -62,12 +61,11 @@ public function testVariableIsUpdatedWhenNoEnvironmentVariableIsPassed()
6261
*/
6362
public function testNullDefaultValue()
6463
{
65-
$this->repository->shouldReceive('withoutFreshModel')->withNoArgs()->once()->andReturnSelf()
66-
->shouldReceive('update')->with($this->model->id, [
67-
'user_viewable' => false,
68-
'user_editable' => false,
69-
'default_value' => '',
70-
])->once()->andReturn(true);
64+
$this->repository->shouldReceive('withoutFreshModel->update')->with($this->model->id, m::subset([
65+
'user_viewable' => false,
66+
'user_editable' => false,
67+
'default_value' => '',
68+
]))->once()->andReturn(true);
7169

7270
$this->assertTrue($this->service->handle($this->model, ['default_value' => null]));
7371
}
@@ -96,7 +94,7 @@ public function testVariableIsUpdatedWhenValidEnvironmentVariableIsPassed()
9694

9795
/**
9896
* Test that an empty (null) value passed in the option key is handled
99-
* properly as an array.
97+
* properly as an array. Also tests that a null description is handled.
10098
*
10199
* @see https://github.com/Pterodactyl/Panel/issues/841
102100
*/
@@ -106,10 +104,10 @@ public function testNullOptionValueIsPassedAsArray()
106104
->shouldReceive('update')->with($this->model->id, m::subset([
107105
'user_viewable' => false,
108106
'user_editable' => false,
109-
'options' => null,
107+
'description' => '',
110108
]))->once()->andReturn(true);
111109

112-
$this->assertTrue($this->service->handle($this->model, ['options' => null]));
110+
$this->assertTrue($this->service->handle($this->model, ['options' => null, 'description' => null]));
113111
}
114112

115113
/**

0 commit comments

Comments
 (0)