Support renaming properties in flattened sub-objects

[Crell]

Description

The tracking logic inside ObjectImporter was slightly buggy, and didn't support property renaming on objects that are being flattened/collected. Now it works.

While technically a bug fix, it enables much more convenient value objects so it's also an enhancement.

This also introduces a flattenPrefix directive, which lets flattened fields get a prefix defined by the parent class. That helps to avoid name collisions.

Motivation and context

Resolves #42.

How has this been tested?

Additional tests included.

Types of changes

What types of changes does your code introduce? Put an x in all the boxes that apply:

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

[agustingomes]

This way it (also applicable for Age ) forces properties to have a specific name, which depending on the context may be ambiguous, or even worse, it may be necessary to have, let's imagine, 2 email objects.

Ideally the property name would be dictated by the parent object, People in this case, or as a fallback, the serializedName to be infered there.

Another way to put it, is to consider this scenario:

<?php

declare(strict_types=1);

namespace Crell\Serde\Records\ValueObjects;

use Crell\Serde\Attributes\Field;

class Invite
{
    public function __construct(
        #[Field(flatten: true)]
        public Email $invitingEmail,
        #[Field(flatten: true)]
        public Email $invitedEmail,
    ) {}
}

Forcing the serializedName on the value object makes this object impossible to deserialize/serialize, as far as I understand.

Does my perspective on this make sense?

[Crell]

Hm, yes, it does break down if the value object is used twice. However, I'm not really sure how to have an alternate name injected from the parent.

Part of the challenge is that there are certain approaches that would only work for a single-property value object, but there's nothing inherent in value objects that makes them single-value only. (You could have a phone number class that stores the area code separately internally, for whatever reason.) So any solution would need to handle that, too.

Which suggests, maybe using renameWith: new Prefix(...) on the parent class. But then the question is how that interacts with the names in the value object. I'm not sure what interaction there makes sense without some completely separate pathway.

[cnastasi]

I'm delighted to see this PR.

But as already noted before, setting the field's name into the child object might be troublesome. You can't say how the value object will be used.

For example, considering the type Age, a field of that type called age could exist, but also a minimumAge or whatever. Also, multiple fields could use the same type, so different names are needed.

It is the parent class, then, that should determine the name.

What about a new attribute, as I put on the issue?

#[Attribute(Attribute::TARGET_CLASS)]
readonly class ValueObject
{
    public function __construct(public string $field = 'value') {}
}

In this way, Serde knows how to serialize/hydrate. But this adds an extra dependency, and I prefer Vanilla PHP when possible.

As an alternative, why not use a flag like simple?

readonly class Person
{
    public function __construct(
        #[Field(flatten: true, simple: true)]
        public Name $name,
        #[Field(flatten: true, simple: 'age')]
        public Age $age
    ) {}
}

I imagine two possible values:

• true when the child value object uses the default field value
• string when the child uses a custom field name

Or better, why not merge flatten and simple behaviours in a single property called simplify?

readonly class Person
{
    public function __construct(
        #[Field(simplify: true)]
        public Name $name,
        #[Field(simplify: 'age')]
        public Age $age
    ) {}
}

The field name or the serializedName might be used as a name.

[Crell]

OK, I think I figured out an approach that will work. The PR now introduces a flattenPrefix directive, which is applied recursively. See the new tests for how it works. That should cover the needs in #42, for both single- and multi-value value objects.

Unless I hear a bug report in the next day or two, I will probably merge this and tag a 1.1 release on Tuesday or Wednesday.

@agustingomes @cnastasi Please confirm this solves your issue(s).

[agustingomes]

Thank you for working on this @Crell . I won't be able to verify it before Thursday, but if it solves the initial report from @cnastasi i'm sure it will work for my use case as well.

[cnastasi]

Hi @Crell, thanks for your work and effort, really appreciate it.

I just downloaded the branch and ran a few tests on my own.

Tell me if I'm wrong, but it covers only the cases when the name of the flattened field has the "serializedName" specified by the child as a part of the target field name.

Example: age -> min_age & max_age.

But what if:

• Do I need minAge instead of min_age? (camelCase vs snake_case)
• Do I need a completely custom name? age -> foo & bar (ex. words in other languages that uses types wrote in English)
• Do I need the same name of the parent field?

Did I miss how to implement those cases, maybe?

What do you think?

[Crell]

The way the logic works is "whatever the Age class says this property should be called, add this string as a prefix."

So you could have a flattenPrefix of "min", and then on the property use serializedName: 'Age', or renameWith: Cases:Capitalized (if the property is named age). That should give you a minAge field in the output.

You can fully rename the field if you want. As shown in the examples, the actual property name is value. But in the output, it is "min_age", because of the combination of flattenPrefix and serializedName.

I don't know what you mean by "same name of the parent field." The name of the property in the parent is currently irrelevant, I believe. The prefix can match it or not, as you prefer.

[Crell]

This seems like it resolves the issue, so I'm going to tag a new release shortly. Thanks all! As a fan of value objects I'm glad to see I'm not alone. 😄