POC: Data objects

[joelbutcher]

Warning

This is a Proof of Concept. Do not merge.

This PR serves as a proof of concept to add data objects back into Cachet core. Data objects provide developers with a better type system than arrays, making it clearer what data should be used where.

Initialisation

Data objects may be initialised four ways. Firstly, you may use the traditional new MyClass syntax:

$data = new ComponentData(
    name: 'My Component',
    description: 'My component description',
);

dd($data->name);
// 'My Component'

Secondly, from arrays via the fromArray builder method:

$data = ComponentData::fromArray([
    'name' => 'My Component',
    'description' => 'My component description',
);

dd($data->name);
// 'My Component'

Thirdly, from a JSON string, via the fromJson builder method:

$data = ComponentData::fromJson('{"name": "My Component",  "description": "My component description"}');


dd($data->name);
// 'My Component'

Finally, from a form request using the fromRequest builder method:

public function store(CreateComponentRequest $request)
{
    $data = ComponentData::fromRequest($request);

    dd($data->name);
    // 'My Component'

    // ...
}

Casting

As an array

Data objects can be cast to an array. To do this, the abstract Data class uses PHP's reflection API to retrieve all public readonly properties and traverse them into an array of key / value pairs. Array keys are converted to snake_case keys using Laravels Str helper.

As a string / JSON

The base Data class implements both the \Stringable and \JsonSerializable contracts. As such, all data objects may be cast to JSON string either by traditional string casting (string) $myData, via strval, or by calling jsonSerialize.

Architecture

Where possible, I've tried to abstract most of the functionality into a single class that data objects extend. This is checked in a new DataTest.php architecture test:

test('data objects test')
    ->expect('Cachet\Data')
    ->toBeClasses()
    ->toBeFinal()
    ->ignoring(Data::class)
    ->toExtend(Data::class);

---

Summary

In summary, data objects provide:

1. Immutability
2. Strong typing
3. fromArray, fromJson, and fromRequest builder methods
4. Casting to array and JSON strings.

[joelbutcher]

@jbrooksuk I've deliberately opened this as a draft PR as I didn't want to spend hours on this PoC if it's not going to be accepted. Above, I've outlined my approach and why I think this is a strong case for using data objects in cachet/core again.

[jbrooksuk]

@joelbutcher I can definitely see the upside of this change, not just the IDE completion (which is a big win for development).

How do we handle a situation where we may accept a payload value like notifications, but actually need to store that as notify?

[joelbutcher]

@jbrooksuk If the column on the model is not named the same as the validated array key, then that will always cause issues. But that problem is pre-existing, because we're passing the result of $request->validated() to Model::create(...) and Model::update(...).

The easiest way is to resolve this is to map properties via an array key / value pairing, though this could lead to obscurity within the data objects themselves (when do you map from / to for e.g.).

What would you want to be able to achieve in that scenario? Is it hiding the database table structure by using different parameter names in the request validation versus column names in the database?

[joelbutcher]

One approach could be to have some form of "interceptor" on the model that maps the attributes from Data::toArray() to values that are accepted by the model itself. You wouldn't need to do this for all attributes, just those that differ.

[joelbutcher]

Not too happy about the use of array_filter here. This assumes that you shouldn't be allowed to update a columns value to null. Might have to think about this a bit more…

Removing array_filter means that columns that aren't passed in the request (e.g. status) are returned as 'status' => null when calling $data->toArray(). This doesn't necessarily indicate that the value we want to set in the DB is null, it just indicates that the data was not sent.

This raises the discussion around the appropriateness of PUT versus PATCH requests for the API... @jbrooksuk correct me if I'm wrong, but a PUT request should send all attributes, even if they've not changed and PATCH only sends the ones we want to update?

[jbrooksuk]

@jbrooksuk What would you want to be able to achieve in that scenario? Is it hiding the database table structure by using different parameter names in the request validation versus column names in the database?

Yeah, basically. It's more for a nicer API than what the DB was giving. We could rename the columns if it came to it.

This raises the discussion around the appropriateness of PUT versus PATCH requests for the API... @jbrooksuk correct me if I'm wrong, but a PUT request should send all attributes, even if they've not changed and PATCH only sends the ones we want to update?

You're right. We've only ever support PUT endpoints though. We could support PATCH too, I guess. Though it does add another endpoint to support.

[joelbutcher]

@jbrooksuk Not had a lot of time to dedicate to this. Hoping to pick this back up this week!

[jbrooksuk]

@joelbutcher you know, I think Data Objects may be more useful when passing incidents (and completed schedules) to the timeline view. Since incidents and schedules are not the same, but both need to be collected and displayed, this may simplify a lot of the logic.

#93

[joelbutcher]

@jbrooksuk I've revisited this with a different approach but it requires L11, so if we could get that sorted, I can progress here

[jbrooksuk]

@joelbutcher we're now running Laravrl 11.x. I'm looking forward to what you're working on here.