Skip to content
This repository was archived by the owner on Apr 5, 2020. It is now read-only.

Conversation

@johannes85
Copy link
Contributor

If an object has private fields which can be accessed via getter functions, the marshalling (with RestMarshalling) of those fields works, but if the parent object also has such fields, it won't work.

Example:

class Parent extends Object {
  private $foo = 'foo';

  public function getFoo() {
    return $this->foo;
  }
}

class Child extends Parent {
  private $bar= 'bar';

  public function getBar() {
    return $this->bar;
  }
}

Produces:

[
  bar => 'bar'
]

But is should produce:

[
  foo => 'foo',
  bar => 'bar'
]

This pull request implements this behavior, which can be enabled by adding the #[@recursive] annotation to the child class.

@thekid
Copy link
Member

thekid commented May 2, 2017

👍 without the annotations; plus requires v9.0.0 because of the possible BC breaks included Update: See below!


Also, because I mentioned it today, the idea of introducing annotations for this, much like JSON struct field tags in GoLang.

type Identity struct {
  Id int64 `json:"id"`
}

(see https://golang.org/pkg/encoding/json/ and https://play.golang.org/p/ZDb6bVGiO6)

We could get this:

class Identity implements \lang\Struct {
  #[@name('id')]
  private $id;
}

...and then wouldn't have to rely on fuzzy getter/setter lookup semantics. This could be a part of XP9...

@thekid
Copy link
Member

thekid commented May 2, 2017

Hrm, implementing this myself (including a fix for your recursion implementation which would call getters for protected members twice or more) I stumbled upon the fact that private members may exist parallel to a parent class' private member:

$ xp -d 'class P { private $p= "P"; } class C extends P { private $p= "C"; } return new C()'
object(C)#29 (2) {
  ["p":"C":private]=>
  string(1) "C"
  ["p":"P":private]=>
  string(1) "P"
}

If we implement this with recursion, which field's value should we fetch and serialize? Whatever answer you select will never completely make you happy. I'm not sure this is really solveable in a "right" way except for leaving it as-is.

I suggest using protected in your case. It's clear what should happen and it also has the benefit that we don't need to change this library.

/cc @mikey179

@thekid thekid added the question label May 2, 2017
@johannes85
Copy link
Contributor Author

I think setting the members to protected would be an ugly hack, which is difficult to understand for someone not involved in this Problem. So, I have to add comments explaining the hack, which is even more ugly.

If there are two protected members, the getter should return the value of the child, which I think is the correct behavior. I think that the only problematic cases are the ones, when you mix private and public/protected members with the same name.

@thekid
Copy link
Member

thekid commented May 6, 2017

Another option would be to write specialized marshallers for your types, e.g. by adding a marker interface (or maybe you already have some sort of parent class). For this, we only need to make marshalling accessible, e.g. by adding a marshalling() method to webservices.rest.Endpoint.

Here's a full example of how implementing marshalling for value objects implementing the getX() / setX() paradigm would work:

interface ValueObject {
}

class ValueObjectMarshaling implements TypeMarshaller {

  public function marshal($value, $marshalling) {
    $type= typeof($value);
    $fields= [];
    do {
      foreach ($type->getFields() as $field) {
        if ($field->getModifiers() & MODIFIER_STATIC) continue;
        $fields[$field->getName()]= true;
      }
    } while (null !== ($type= $type->getParentclass()));

    $result= [];
    foreach ($fields as $key => $val) {
      $result[$key]= $marshalling->marshal($type->getMethod('get'.$key)->invoke($value));
    }
    return $result;
  }

  public function unmarshal(Type $target, $value, $marshalling) {
    $instance= $target->newInstance();
    foreach ($value as $key => $val) {
      $method= $target->getMethod('set'.$key);
      $type= $method->getParameter(0)->getType();
      $method->invoke($instance, [$marshalling->unmarshal($type, $val)]);
    }
    return $instance;
  }
}

class Identity implements ValueObject {
  private $id;

  public function setId(int $id): self { $this->id= $id; return $this; }
  public function getId(): int { return $this->id; }
}

$api= (new Endpoint($url))->marshalling(ValueObject::class, new ValueObjectMarshalling());

@thekid
Copy link
Member

thekid commented May 6, 2017

How do other PHP libraries handle this, by the way?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants