refact!: Split `Uuid::for()` and `Uuid::from()` #7544

distantnative · 2025-08-15T11:36:16Z

Description

The motivation here is to break up some of the magic monster methods into separate, dedicated methods.

Thinking also about moving all permalink logic into its own Uuid\Permalink class.

Changelog

♻️ Refactored

New Kirby\Uuid\Uuid::from(string $uuid) method for creating an Uuid object from a UUID string. Kirby\Uuid\Uuid::for() remains to create a Uuid object for a model object.

🚨 Breaking changes

Kirby\Uuid\Uuid::for() cannot be called any longer with a string. Use Kirby\Uuid\Uuid::from(string $uuid) or Kirby\Uuid\Uuid::fromPermalink(string $permalink) instead.

For review team

Add changes & docs to release notes draft in Notion

lukasbestle

The for and from names are great here!

lukasbestle · 2025-08-17T15:12:27Z

src/Uuid/FieldUuid.php

 	 * - parent UUID including scheme
 	 * - field name
 	 * - UUID id string for model
 	 */
 	public function value(): array
 	{
+		/**
+		 * @var \Kirby\Cms\Site|\Kirby\Cms\Page|\Kirby\Cms\User $model


Question: Couldn't it also be a file?

You are right. It seems I was rather thinking about the result of $model->parent() here.

lukasbestle · 2025-08-17T15:14:38Z

src/Uuid/Uuid.php

+				// 'block'  => new BlockUuid(uuid: $seed, context: $context),
+				// 'struct' => new StructureUuid(uuid: $seed, context: $context),
+				default  => throw new InvalidArgumentException(
+					message: 'Invalid UUID URI "' . $uri . '" in ' . $uuid


Suggested change

message: 'Invalid UUID URI "' . $uri . '" in ' . $uuid

message: 'Invalid UUID type "' . $uri . '" in ' . $uuid

Probably makes sense to rename the variable as well. Not sure why we used $uri here.

lukasbestle · 2025-08-17T15:16:48Z

src/Uuid/Uuid.php

+	 */
+	final public static function from(
+		string $uuid,
+		string|array|null $scheme = null,


Suggestion: Uuid::is() uses the name $type, I'd use the same here for consistency:

Suggested change

string|array|null $scheme = null,

string|array|null $type = null,

lukasbestle · 2025-08-17T15:21:52Z

config/tags.php

+			if ($uuid = Uuid::from($tag->value, ['page', 'file'])) {
+				$tag->value = $uuid->toUrl();


Issue: This changes behavior. Before, a valid but non-existent UUID would lead to an empty value, which would be handled below. Now, the original UUID URI would stay in the value unconverted.

I think here we could solve it by just turning it into a one-liner:

$tag->value = Uuid::from($tag->value, ['page', 'file'])?->toUrl();

as this would also lead to $tag->value being null afterwards if it isn't a valid UUID.

Not quite. Then the value will also be null for all non-UUID links (links that don't match the page://something or file://something patterns).

lukasbestle · 2025-08-17T15:24:45Z

src/Cms/HasFiles.php

+		if ($uuid = Uuid::from($filename, 'file', $this->$in())) {
+			return $uuid->model();


Issue: Same here, but not as critical. Here, the valid but non-existent UUID will make Kirby execute all of the other checks instead of exiting early (but before with an error like Call to function model() on null).

That raises maybe a question: If something looks like a UUID but cannot be resolved to a model, would we rather have it throwing some form of error or just ignore it and try to resolve it with the other checks?

I think we should throw an error. The UUID pattern is very unique (file://something) and it is quite unlikely that something else was meant. Especially not a filename as filenames cannot contain two slashes sequentially.

lukasbestle · 2025-08-17T15:29:27Z

src/Text/KirbyTag.php

-			return $file;
-		}
+			if (method_exists($parent, 'file') === true) {
+				return $parent->file($path);


Issue: If called on $parent = Page, this will return null if that page doesn't have the file $path. The old code continued to the very last line of the method and checked for site files of that name.

distantnative self-assigned this Aug 15, 2025

distantnative force-pushed the v6/refact/uuids-uuid-from branch from 74daedf to 74c6e4c Compare August 15, 2025 11:51

distantnative changed the title ~~V6/refact/uuids UUID from~~ refact!: Split Uuid::for() and Uuid::from() Aug 15, 2025

distantnative force-pushed the v6/refact/uuids-uuid-from branch from 74c6e4c to 415ebac Compare August 15, 2025 18:03

distantnative marked this pull request as ready for review August 15, 2025 18:15

distantnative requested a review from a team August 15, 2025 18:15

distantnative marked this pull request as draft August 15, 2025 18:16

distantnative force-pushed the v6/refact/uuids-uuid-from branch 2 times, most recently from 0638184 to d43bc60 Compare August 15, 2025 18:35

distantnative changed the base branch from v6/develop to v6/refact/uuids-permalink-class August 15, 2025 18:35

distantnative force-pushed the v6/refact/uuids-permalink-class branch from f895088 to 1a44f8b Compare August 15, 2025 18:41

distantnative force-pushed the v6/refact/uuids-uuid-from branch from d43bc60 to 953751b Compare August 15, 2025 18:43

distantnative force-pushed the v6/refact/uuids-permalink-class branch 3 times, most recently from 5d5764a to f4edc38 Compare August 15, 2025 19:08

distantnative force-pushed the v6/refact/uuids-uuid-from branch from 953751b to ba7e2a6 Compare August 15, 2025 19:09

distantnative force-pushed the v6/refact/uuids-permalink-class branch from f4edc38 to 7cd7158 Compare August 16, 2025 18:52

distantnative force-pushed the v6/refact/uuids-uuid-from branch from ba7e2a6 to c7138d8 Compare August 16, 2025 18:58

lukasbestle reviewed Aug 17, 2025

View reviewed changes

distantnative force-pushed the v6/refact/uuids-permalink-class branch from 34ebf2e to 999539b Compare August 18, 2025 19:49

Base automatically changed from v6/refact/uuids-permalink-class to v6/develop August 19, 2025 09:55

refact!: Split Uuid::for() and Uuid::from()

ad862b2

distantnative force-pushed the v6/refact/uuids-uuid-from branch from c7138d8 to ad862b2 Compare August 19, 2025 09:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

refact!: Split `Uuid::for()` and `Uuid::from()` #7544

refact!: Split `Uuid::for()` and `Uuid::from()` #7544

Uh oh!

distantnative commented Aug 15, 2025 •

edited

Loading

Uh oh!

lukasbestle left a comment

Uh oh!

lukasbestle Aug 17, 2025

Uh oh!

distantnative Aug 17, 2025

Uh oh!

lukasbestle Aug 17, 2025

Uh oh!

lukasbestle Aug 17, 2025

Uh oh!

lukasbestle Aug 17, 2025

Uh oh!

distantnative Aug 17, 2025

Uh oh!

lukasbestle Aug 17, 2025

Uh oh!

lukasbestle Aug 17, 2025

Uh oh!

distantnative Aug 17, 2025

Uh oh!

lukasbestle Aug 17, 2025

Uh oh!

lukasbestle Aug 17, 2025

Uh oh!

Uh oh!

	message: 'Invalid UUID URI "' . $uri . '" in ' . $uuid
	message: 'Invalid UUID type "' . $uri . '" in ' . $uuid

	string\|array\|null $scheme = null,
	string\|array\|null $type = null,

		if ($uuid = Uuid::from($tag->value, ['page', 'file'])) {
		$tag->value = $uuid->toUrl();

		if ($uuid = Uuid::from($filename, 'file', $this->$in())) {
		return $uuid->model();

Uh oh!

refact!: Split Uuid::for() and Uuid::from() #7544

Are you sure you want to change the base?

refact!: Split Uuid::for() and Uuid::from() #7544

Uh oh!

Conversation

distantnative commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changelog

♻️ Refactored

🚨 Breaking changes

For review team

Uh oh!

lukasbestle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

refact!: Split `Uuid::for()` and `Uuid::from()` #7544

refact!: Split `Uuid::for()` and `Uuid::from()` #7544

distantnative commented Aug 15, 2025 •

edited

Loading