Skip to content

Commit a0c3a02

Browse files
committed
refact!: Throw exception for unkown field method
BREAKING CHANGE: When calling an unknown method on a field a BadMethodCallException is now thrown.
1 parent e2526f2 commit a0c3a02

File tree

6 files changed

+59
-26
lines changed

6 files changed

+59
-26
lines changed

src/Cms/Collection.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use Kirby\Toolkit\Collection as BaseCollection;
88
use Kirby\Toolkit\Str;
99
use Kirby\Uuid\Uuid;
10+
use Throwable;
1011

1112
/**
1213
* The Collection class serves as foundation
@@ -126,7 +127,13 @@ public function append(...$args): static
126127
is_object($args[0]) === true &&
127128
is_callable([$args[0], 'id']) === true
128129
) {
129-
return parent::append($args[0]->id(), $args[0]);
130+
try {
131+
return parent::append($args[0]->id(), $args[0]);
132+
} catch (Throwable) {
133+
// is_callable might be true when the object implements
134+
// the magic __call method, but __call can still throw
135+
// an exception
136+
}
130137
}
131138

132139
return parent::append($args[0]);

src/Content/Field.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Kirby\Cms\User;
2121
use Kirby\Cms\Users;
2222
use Kirby\Data\Data;
23+
use Kirby\Exception\BadMethodCallException;
2324
use Kirby\Exception\InvalidArgumentException;
2425
use Kirby\Exception\NotFoundException;
2526
use Kirby\Image\QrCode;
@@ -76,13 +77,14 @@ public function __call(string $method, array $arguments = []): mixed
7677
{
7778
$method = strtolower($method);
7879

80+
// call custom field method
7981
if ($this->hasMethod($method) === true) {
8082
return $this->callMethod($method, [clone $this, ...$arguments]);
8183
}
8284

83-
// TODO: throw deprecation, then exception
84-
// when unknown method is called
85-
return $this;
85+
throw new BadMethodCallException(
86+
message: 'Field method "' . $method . '" does not exist'
87+
);
8688
}
8789

8890
/**

src/Panel/Ui/FilePreview/ImageFilePreview.php

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace Kirby\Panel\Ui\FilePreview;
44

55
use Kirby\Cms\File;
6+
use Kirby\Image\Dimensions;
67
use Kirby\Panel\Ui\FilePreview;
78
use Kirby\Toolkit\I18n;
89

@@ -30,17 +31,22 @@ public static function accepts(File $file): bool
3031

3132
public function details(): array
3233
{
33-
return [
34-
...parent::details(),
35-
[
34+
$details = parent::details();
35+
$dimensions = $this->file->dimensions();
36+
37+
if ($dimensions instanceof Dimensions) {
38+
$details[] = [
3639
'title' => I18n::translate('dimensions'),
37-
'text' => $this->file->dimensions() . ' ' . I18n::translate('pixel')
38-
],
39-
[
40+
'text' => $dimensions . ' ' . I18n::translate('pixel')
41+
];
42+
43+
$details[] = [
4044
'title' => I18n::translate('orientation'),
41-
'text' => I18n::translate('orientation.' . $this->file->dimensions()->orientation())
42-
]
43-
];
45+
'text' => I18n::translate('orientation.' . $dimensions->orientation())
46+
];
47+
}
48+
49+
return $details;
4450
}
4551

4652
public function props(): array

tests/Cms/Collections/CollectionTest.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,14 @@ public function uuid(): string|Field
4040
}
4141
}
4242

43+
class MockObjectWith__CallException
44+
{
45+
public function __call($name, $arguments)
46+
{
47+
throw new Exception('Test exception');
48+
}
49+
}
50+
4351
#[CoversClass(Collection::class)]
4452
class CollectionTest extends TestCase
4553
{
@@ -145,6 +153,14 @@ public function testAppend(): void
145153
$this->assertSame([$a, $b, $c, $d, 'a simple string'], $collection->values());
146154
}
147155

156+
public function testAppendWith__CallException(): void
157+
{
158+
$collection = new Collection();
159+
$obj = new MockObjectWith__CallException();
160+
$collection = $collection->append($obj);
161+
$this->assertSame([0], $collection->keys());
162+
}
163+
148164
public function testFindByUuid(): void
149165
{
150166
$collection = new Collection([

tests/Content/FieldTest.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use Kirby\Data\Data;
1515
use Kirby\Data\Json;
1616
use Kirby\Data\Yaml;
17+
use Kirby\Exception\BadMethodCallException;
1718
use Kirby\Exception\InvalidArgumentException;
1819
use Kirby\Filesystem\Dir;
1920
use Kirby\Image\QrCode;
@@ -68,10 +69,11 @@ public function test__call(): void
6869

6970
public function test__callNonExistingMethod(): void
7071
{
71-
$field = $this->field('value');
72-
$result = $field->methodDoesNotExist();
72+
$this->expectException(BadMethodCallException::class);
73+
$this->expectExceptionMessage('Field method "methoddoesnotexist" does not exist');
7374

74-
$this->assertSame($field, $result);
75+
$field = $this->field('value');
76+
$field->methodDoesNotExist();
7577
}
7678

7779
public function test__debugInfo(): void

tests/Toolkit/VTest.php

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -600,18 +600,18 @@ public function testSize(): void
600600
$this->assertFalse(V::size(7, 5, '<'));
601601

602602
$field = new Field(null, 'foo', 2);
603-
$this->assertTrue(V::size($field->foo(), 2));
604-
$this->assertTrue(V::size($field->foo(), 3, '<'));
605-
$this->assertTrue(V::size($field->foo(), 1, '>'));
606-
$this->assertFalse(V::size($field->foo(), 1, '<'));
607-
$this->assertFalse(V::size($field->foo(), 3, '>'));
603+
$this->assertTrue(V::size($field, 2));
604+
$this->assertTrue(V::size($field, 3, '<'));
605+
$this->assertTrue(V::size($field, 1, '>'));
606+
$this->assertFalse(V::size($field, 1, '<'));
607+
$this->assertFalse(V::size($field, 3, '>'));
608608

609609
$field = new Field(null, 'foo', 'hello');
610-
$this->assertTrue(V::size($field->foo(), 5));
611-
$this->assertTrue(V::size($field->foo(), 6, '<'));
612-
$this->assertTrue(V::size($field->foo(), 4, '>'));
613-
$this->assertFalse(V::size($field->foo(), 4, '<'));
614-
$this->assertFalse(V::size($field->foo(), 6, '>'));
610+
$this->assertTrue(V::size($field, 5));
611+
$this->assertTrue(V::size($field, 6, '<'));
612+
$this->assertTrue(V::size($field, 4, '>'));
613+
$this->assertFalse(V::size($field, 4, '<'));
614+
$this->assertFalse(V::size($field, 6, '>'));
615615
}
616616

617617
public function testSizeInvalid(): void

0 commit comments

Comments
 (0)