-
Notifications
You must be signed in to change notification settings - Fork 77
added popo serializer/deserializer and Service shorthand #66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -161,3 +161,31 @@ function elementList(Reader $reader, $namespace = null) { | |
| return $values; | ||
|
|
||
| } | ||
|
|
||
| /** | ||
| * @param Reader $reader | ||
| * @param object $valueObject | ||
| * @param string $namespace | ||
| * @return null|$valueObject | ||
| */ | ||
| function valueObject(Reader $reader, $valueObject, $namespace) { | ||
| if ($reader->isEmptyElement) { | ||
| $reader->next(); | ||
| return null; | ||
| } | ||
|
|
||
| $reader->read(); | ||
| do { | ||
|
|
||
| if ($reader->nodeType === Reader::ELEMENT && $reader->namespaceURI == $namespace) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Future idea: add the ability to parse annotations and let people create classes like this: class MyVO {
/**
* @map {http://namespace}elem
*/
public $foo;
}
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I also thought about such things... I dont like annotations that much because they add a magic layer which needs to be documented very properly. Also it mixes the serialize/deserialize logic into the ValueObjects which is the opposite direction this PR tries to aim for. Maybe this will improve in php-src future in case annotations get a 1st class citizen in php.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having annotations also opens the door for mapping xml attributes somewhere. Atm we dont support them at all in this popo cases
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this function use use
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its one indirection and another short living temp. Structure.. In case you prefer it I am fine with it. We can restore to what we have right now in case it gets a perf bottleneck
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Absolutely. Thanks for your input.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually... I changed my mind! I thought using
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okidoki, you have 10 hours to change your mind again :-)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lol
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. time is up. pushed without the additional indirection ;) |
||
|
|
||
| $valueObject->{$reader->localName} = $reader->parseCurrentElement()['value']; | ||
| } else { | ||
| $reader->read(); | ||
| } | ||
| } while ($reader->nodeType !== Reader::END_ELEMENT); | ||
|
|
||
| $reader->read(); | ||
|
|
||
| return $valueObject; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| <?php | ||
|
|
||
| namespace Sabre\Xml\Serializer; | ||
|
|
||
| use Sabre\Xml\Writer; | ||
|
|
||
| /** | ||
| * This file provides a number of 'serializer' helper functions. | ||
| * These can be used to easily specify custom serializers for specific | ||
| * PHP objects/values. | ||
| */ | ||
|
|
||
| /** | ||
| * @param Writer $writer | ||
| * @param object $valueObject | ||
| * @param string $namespace | ||
| */ | ||
| function valueObject(Writer $writer, $valueObject, $namespace) { | ||
| foreach (get_object_vars($valueObject) as $key => $val) { | ||
| $writer->writeElement('{' . $namespace . '}' . $key, $val); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dropped the
$this->namespaceMap[$namespace]=nullexpression here, because it had side effects.I lead to all elements of this namespaces beeing represented by its localName instead of clark-notated (also for this not "registered" via
mapValueObject).After dropping the line we are still green, so nothing important ;).
see https://github.com/staabm/sabre-xml/commit/09836bbd21d093a7b23c792d31b9019730f13b49#diff-de76feda74829916defbb14d089c7c08R208 if you dont know what the hell I am talking about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
strike that. after I recognized that I looked at the wrong repos on travis I realised the line is still necessary.
therefore the question... what do you think about this sideeffect? any idea how to get arround it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused because this thread is on an 'outdated diff' discussing a line that wasn't in that diff... can you point me in the right direction?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the Line in the current diff https://github.com/fruux/sabre-xml/pull/66/files#diff-de76feda74829916defbb14d089c7c08R208