Include UUID with each URL metric

[westonruter]

When working with URL metrics, I found it difficult when debugging to not have a stable identifier as part of a given URL metric instance. This adds a uuid field to the URL metric to serve as this stable identifier.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>
Co-authored-by: felixarntz <flixos90@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[adamsilverstein]

singular

[felixarntz]

@westonruter LGTM, just one nit-pick.

[felixarntz]

isset() is faster than array_key_exists(), this has been subject of several core changes. I'd argue anyway that it would be safer to use the following here to ensure this is never empty:

Suggested change

	if ( ! array_key_exists( 'uuid', $data ) ) {
	if ( ! isset( $data['uuid'] ) || ! $data['uuid'] ) {

[westonruter]

Sounds good, but what about #1219?

[westonruter]

I guess we should reduce the scope of that issue to remove use of empty() only.

[westonruter]

Also, the ! $data['uuid'] part is flagged by PHPStan's strict rules:

Only booleans are allowed in a negated boolean, mixed given.

So I'll just commit the ! isset( $data['uuid'] ) part. The JSON Schema will handle validation of uuid. It should be absent for everyone right now, resulting in a new uuid being assigned to all existing URL metrics whenever a new URL metric is captured and added to an od_url_metrics post.

Done in 7a1b892

[felixarntz]

+1, I don't think there's any problem with isset() when used with array keys or object properties. In other words, isset( $arr['something'] ) is perfectly fine, but isset( $something ) is not.

It's a good idea to remove empty() usages and, if reasonable to implement, also warn about using isset() when it's not with array keys or object properties.

[felixarntz]

Only booleans are allowed in a negated boolean, mixed given.

On another note, this is the only thing that PHPStan complains about that I don't like. I don't like writing '' === $value just as little as I like writing false === $value, and I don't see any benefit of preventing ! on a string. What is the benefit? 🤔

[westonruter]

While I agree it can be annoying at times, I think the explicitness is good for type safety and making sure that you're always working with the data you expect. Otherwise, I had to ask Gemini for advice: https://g.co/gemini/share/270a3e76fcdd

@swissspidy may have more thoughts.