Skip to content

Commit 15f5d10

Browse files
mvorisekmvorisek
authored
ErrorException should not be wrapped (#2132)
1 parent e88cca2 commit 15f5d10

File tree

5 files changed

+145
-42
lines changed

5 files changed

+145
-42
lines changed

‎.github/workflows/test-unit.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ jobs:
8989
services:
9090
mysql:
9191
image: mysql:8
92-
options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=5 -e MYSQL_ROOT_PASSWORD=atk4_pass_root -e MYSQL_USER=atk4_test_user -e MYSQL_PASSWORD=atk4_pass -e MYSQL_DATABASE=atk4_test --entrypoint sh mysql:8 -c "exec docker-entrypoint.sh mysqld --default-authentication-plugin=mysql_native_password"
92+
options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=5 -e MYSQL_ROOT_PASSWORD=atk4_pass_root -e MYSQL_USER=atk4_test_user -e MYSQL_PASSWORD=atk4_pass -e MYSQL_DATABASE=atk4_test
9393
mariadb:
9494
image: mariadb
9595
options: --health-cmd="mariadb-admin ping" --health-interval=10s --health-timeout=5s --health-retries=5 -e MYSQL_ROOT_PASSWORD=atk4_pass_root -e MYSQL_USER=atk4_test_user -e MYSQL_PASSWORD=atk4_pass -e MYSQL_DATABASE=atk4_test
@@ -253,7 +253,7 @@ jobs:
253253
services:
254254
mysql:
255255
image: mysql:8
256-
options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=5 -e MYSQL_ROOT_PASSWORD=atk4_pass_root -e MYSQL_USER=atk4_test_user -e MYSQL_PASSWORD=atk4_pass -e MYSQL_DATABASE=atk4_test --entrypoint sh mysql:8 -c "exec docker-entrypoint.sh mysqld --default-authentication-plugin=mysql_native_password"
256+
options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=5 -e MYSQL_ROOT_PASSWORD=atk4_pass_root -e MYSQL_USER=atk4_test_user -e MYSQL_PASSWORD=atk4_pass -e MYSQL_DATABASE=atk4_test
257257
mariadb:
258258
image: mariadb
259259
options: --health-cmd="mariadb-admin ping" --health-interval=10s --health-timeout=5s --health-retries=5 -e MYSQL_ROOT_PASSWORD=atk4_pass_root -e MYSQL_USER=atk4_test_user -e MYSQL_PASSWORD=atk4_pass -e MYSQL_DATABASE=atk4_test

‎src/Behat/RwDemosContextTrait.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ protected function createDatabaseBackup(): void
9595
}
9696

9797
/**
98-
* @return array<string, \stdClass&object{ addedIds: list<int>, changedIds: list<int>, deletedIds: list<int> }>
98+
* @return array<string, \stdClass&object{ addedIds: list<int>, updatedIds: list<int>, deletedIds: list<int> }>
9999
*/
100100
protected function discoverDatabaseChanges(): array
101101
{
@@ -106,7 +106,7 @@ protected function discoverDatabaseChanges(): array
106106

107107
$changes = new \stdClass();
108108
$changes->addedIds = [];
109-
$changes->changedIds = [];
109+
$changes->updatedIds = [];
110110
$changes->deletedIds = array_fill_keys(array_keys($data), true);
111111
foreach ($model as $entity) {
112112
$id = $entity->getId();
@@ -123,15 +123,15 @@ protected function discoverDatabaseChanges(): array
123123
}
124124

125125
if ($isChanged) {
126-
$changes->changedIds[] = $id;
126+
$changes->updatedIds[] = $id;
127127
}
128128

129129
unset($changes->deletedIds[$id]);
130130
}
131131
}
132132
$changes->deletedIds = array_keys($changes->deletedIds);
133133

134-
if (count($changes->addedIds) > 0 || count($changes->changedIds) > 0 || count($changes->deletedIds) > 0) {
134+
if (count($changes->addedIds) > 0 || count($changes->updatedIds) > 0 || count($changes->deletedIds) > 0) {
135135
$changesByTable[$table] = $changes;
136136
}
137137
}
@@ -156,8 +156,8 @@ protected function restoreDatabaseBackup(): void
156156
$model->delete($id);
157157
}
158158

159-
foreach ([...$changes->changedIds, ...$changes->deletedIds] as $id) {
160-
$entity = in_array($id, $changes->changedIds, true) ? $model->load($id) : $model->createEntity();
159+
foreach ([...$changes->updatedIds, ...$changes->deletedIds] as $id) {
160+
$entity = in_array($id, $changes->updatedIds, true) ? $model->load($id) : $model->createEntity();
161161
$entity->setMulti($data[$id]);
162162
$entity->save();
163163
}

‎src/Form.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,10 @@ protected function loadPost(): void
441441
try {
442442
$control->set($this->getApp()->uiPersistence->typecastLoadField($control->entityField->getField(), $postRawValue));
443443
} catch (\Exception $e) {
444+
if ($e instanceof \ErrorException) {
445+
throw $e;
446+
}
447+
444448
$messages = [];
445449
do {
446450
$messages[] = $e->getMessage();

‎src/Form/AbstractLayout.php

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public function addControl(string $name, $control = [], array $fieldSeed = []):
3939
if ($this->form->model === null) {
4040
$this->form->model = (new \Atk4\Ui\Misc\ProxyModel())->createEntity();
4141
}
42-
$this->form->model->assertIsEntity();
42+
$model = $this->form->model->getModel();
4343

4444
// TODO this class should not refer to any specific form control
4545
$controlClass = is_object($control)
@@ -55,19 +55,22 @@ public function addControl(string $name, $control = [], array $fieldSeed = []):
5555
}
5656

5757
try {
58-
if (!$this->form->model->hasField($name)) {
59-
$field = $this->form->model->getModel()->addField($name, $fieldSeed);
58+
if ($model->hasField($name)) {
59+
$field = $model->getField($name)->setDefaults($fieldSeed); // TODO assert same defaults only
6060
} else {
61-
$field = $this->form->model->getField($name)
62-
->setDefaults($fieldSeed);
61+
$field = $model->addField($name, $fieldSeed);
6362
}
6463

6564
$control = $this->form->controlFactory($field, $control);
6665
} catch (\Exception $e) {
66+
if ($e instanceof \ErrorException) {
67+
throw $e;
68+
}
69+
6770
throw (new Exception('Unable to create form control', 0, $e))
6871
->addMoreInfo('name', $name)
69-
->addMoreInfo('control', $control)
70-
->addMoreInfo('field', $fieldSeed);
72+
->addMoreInfo('control' . (!is_object($control) ? 'Seed' : ''), $control)
73+
->addMoreInfo('fieldSeed', $fieldSeed);
7174
}
7275

7376
return $this->_addControl($control, $field);

‎tests/FormTest.php

Lines changed: 123 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace Atk4\Ui\Tests;
66

77
use Atk4\Core\Phpunit\TestCase;
8+
use Atk4\Data\Field;
89
use Atk4\Data\Model;
910
use Atk4\Data\Model\EntityFieldPair;
1011
use Atk4\Data\ValidationException;
@@ -28,10 +29,10 @@ public function testGetField(): void
2829
$form->setApp($this->createApp());
2930
$form->invokeInit();
3031

31-
$form->addControl('test');
32+
$form->addControl('foo');
3233

33-
self::assertInstanceOf(Form\Control::class, $form->getControl('test'));
34-
self::assertSame($form->getControl('test'), $form->layout->getControl('test'));
34+
self::assertInstanceOf(Form\Control::class, $form->getControl('foo'));
35+
self::assertSame($form->getControl('foo'), $form->layout->getControl('foo'));
3536
}
3637

3738
public function testAddControlAlreadyExistsException(): void
@@ -131,11 +132,11 @@ public function testTextareaSubmit(): void
131132
{
132133
$this->assertFormSubmit(static function (App $app) {
133134
$form = Form::addTo($app);
134-
$form->addControl('Textarea');
135+
$form->addControl('foo');
135136

136137
return $form;
137-
}, ['Textarea' => '0'], static function (Model $m) {
138-
self::assertSame('0', $m->get('Textarea'));
138+
}, ['foo' => '0'], static function (Model $m) {
139+
self::assertSame('0', $m->get('foo'));
139140
});
140141
}
141142

@@ -203,35 +204,130 @@ public function testSubmitNonFormFieldError(): void
203204
$submitReached = false;
204205
$catchReached = false;
205206
try {
206-
try {
207-
$this->assertFormSubmit(static function (App $app) {
208-
$m = new Model();
209-
$m->addField('foo', ['nullable' => false]);
210-
$m->addField('bar', ['nullable' => false]);
211-
212-
$form = Form::addTo($app);
213-
$form->setModel($m->createEntity(), ['foo']);
214-
215-
return $form;
216-
}, ['foo' => 'x'], static function (Model $model) use (&$submitReached) {
217-
$submitReached = true;
218-
$model->set('bar', null);
207+
$this->assertFormSubmit(static function (App $app) {
208+
$m = new Model();
209+
$m->addField('foo', ['nullable' => false]);
210+
$m->addField('bar', ['nullable' => false]);
211+
212+
$form = Form::addTo($app);
213+
$form->setModel($m->createEntity(), ['foo']);
214+
215+
return $form;
216+
}, ['foo' => 'x'], static function (Model $model) use (&$submitReached) {
217+
$submitReached = true;
218+
$model->set('bar', null);
219+
});
220+
} catch (UnhandledCallbackExceptionError $e) {
221+
$catchReached = true;
222+
self::assertSame('bar', $e->getPrevious()->getParams()['field']->shortName); // @phpstan-ignore-line
223+
224+
$this->expectException(ValidationException::class);
225+
$this->expectExceptionMessage('Must not be null');
226+
227+
throw $e->getPrevious();
228+
} finally {
229+
self::assertTrue($submitReached);
230+
self::assertTrue($catchReached);
231+
}
232+
}
233+
234+
public function testLoadPostConvertedWarningNotWrappedException(): void
235+
{
236+
$catchReached = false;
237+
try {
238+
$this->assertFormSubmit(static function (App $app) {
239+
$m = new Model();
240+
$m->addField('foo', new class() extends Field {
241+
public function normalize($value)
242+
{
243+
TestCase::assertSame('x', $value);
244+
245+
throw new \ErrorException('Converted PHP warning');
246+
}
219247
});
220-
} catch (UnhandledCallbackExceptionError $e) {
221-
$catchReached = true;
222-
self::assertSame('bar', $e->getPrevious()->getParams()['field']->shortName); // @phpstan-ignore-line
223248

224-
$this->expectException(ValidationException::class);
225-
$this->expectExceptionMessage('Must not be null');
249+
$form = Form::addTo($app);
250+
$form->setModel($m->createEntity());
226251

227-
throw $e->getPrevious();
228-
}
252+
return $form;
253+
}, ['foo' => 'x']);
254+
} catch (UnhandledCallbackExceptionError $e) {
255+
$catchReached = true;
256+
257+
$this->expectException(\ErrorException::class);
258+
$this->expectExceptionMessage('Converted PHP warning');
259+
260+
throw $e->getPrevious();
229261
} finally {
230-
self::assertTrue($submitReached);
231262
self::assertTrue($catchReached);
232263
}
233264
}
234265

266+
public function testCreateControlException(): void
267+
{
268+
$form = new Form();
269+
$form->setApp($this->createApp());
270+
$form->invokeInit();
271+
272+
$controlClass = get_class(new class() extends Form\Control {
273+
public static bool $firstCreate = true;
274+
275+
public function __construct() // @phpstan-ignore-line
276+
{
277+
if (self::$firstCreate) {
278+
self::$firstCreate = false;
279+
280+
return;
281+
}
282+
283+
throw new Exception('x');
284+
}
285+
});
286+
287+
$this->expectException(Exception::class);
288+
$this->expectExceptionMessage('Unable to create form control');
289+
try {
290+
$form->addControl('foo', [$controlClass]);
291+
} catch (Exception $e) {
292+
self::assertSame(Exception::class, get_class($e->getPrevious()));
293+
self::assertSame('x', $e->getPrevious()->getMessage());
294+
295+
throw $e;
296+
} finally {
297+
$controlClass::$firstCreate = true;
298+
}
299+
}
300+
301+
public function testCreateControlConvertedWarningNotWrappedException(): void
302+
{
303+
$form = new Form();
304+
$form->setApp($this->createApp());
305+
$form->invokeInit();
306+
307+
$controlClass = get_class(new class() extends Form\Control {
308+
public static bool $firstCreate = true;
309+
310+
public function __construct() // @phpstan-ignore-line
311+
{
312+
if (self::$firstCreate) {
313+
self::$firstCreate = false;
314+
315+
return;
316+
}
317+
318+
throw new \ErrorException('Converted PHP warning');
319+
}
320+
});
321+
322+
$this->expectException(\ErrorException::class);
323+
$this->expectExceptionMessage('Converted PHP warning');
324+
try {
325+
$form->addControl('foo', [$controlClass]);
326+
} finally {
327+
$controlClass::$firstCreate = true;
328+
}
329+
}
330+
235331
public function testNoDisabledAttrWithHiddenType(): void
236332
{
237333
$input = new Form\Control\Line();

0 commit comments

Comments
 (0)