Comments

effulgentsia created an issue. See original summary.

effulgentsia’s picture

Status: Active » Postponed
effulgentsia’s picture

Issue summary: View changes
gabesullice’s picture

gabesullice’s picture

Title: [PP-1] Add PrimaryData, IncludedData, and RelationshipData specializations of Data » [PP-1] Add ResourceObjectData, IncludedData, and RelationshipData specializations of Data

s/PrimaryData/ResourceObjectData/ because on relationship responses, the relationship data is the primary data.

gabesullice’s picture

StatusFileSize
new1 KB
new13.23 KB
new46.57 KB
wim leers’s picture

+++ b/src/JsonApiResource/Data.php
@@ -10,7 +10,7 @@ use Drupal\jsonapi\Exception\EntityAccessDeniedHttpException;
-class Data implements \IteratorAggregate, \Countable {
+abstract class Data implements \IteratorAggregate, \Countable {

+++ b/src/JsonApiResource/IncludedData.php
@@ -0,0 +1,28 @@
+class IncludedData extends Data {
...
+  public function __construct($data) {
+    assert(Inspector::assertAllObjects($data, ResourceObject::class, EntityAccessDeniedHttpException::class));

+++ b/src/JsonApiResource/RelationshipData.php
@@ -0,0 +1,29 @@
+class RelationshipData extends Data {
...
+  public function __construct(array $data, $cardinality = -1) {
+    assert(Inspector::assertAllObjects($data, ResourceIdentifier::class));

+++ b/src/JsonApiResource/ResourceObjectData.php
@@ -0,0 +1,30 @@
+class ResourceObjectData extends Data {
...
+  public function __construct($data, int $cardinality = -1) {
+    assert(Inspector::assertAllObjects($data, ResourceObject::class, EntityAccessDeniedHttpException::class));

I do like the extra type safety this gets us! The fact that they're all still @internal means this is solely refactoring of implementation details that gets us type safety.

(We can nail the name later, before making it non-internal.)

This looks wonderful! Just one question: why not make each concrete Data subclass final?

RTBC as far as I'm concerned.

gabesullice’s picture

Status: Postponed » Reviewed & tested by the community

#3032679: Clean-up: rename EntityCollection to Data landed. Per #7, RTBC.

Re-uploading the #6 do-not-test patch. Will commit if tests pass.

gabesullice’s picture

StatusFileSize
new13.23 KB

Heh, forgot to upload.

gabesullice’s picture

Title: [PP-1] Add ResourceObjectData, IncludedData, and RelationshipData specializations of Data » Add ResourceObjectData, IncludedData, and RelationshipData specializations of Data

  • gabesullice committed 96d4640 on 8.x-2.x
    Issue #3033123 by gabesullice, effulgentsia, Wim Leers: Add...
gabesullice’s picture

Status: Reviewed & tested by the community » Fixed

🎉

effulgentsia’s picture

wim leers’s picture

  • Wim Leers committed 3e062ac on 8.x-2.x
    Issue #3035676 by Wim Leers: Follow-up for #3033123: PHP 5.5 + Drupal 8....

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.