added popo serializer/deserializer and Service shorthand

[staabm]

added popo mapping on the Service class.

see enclosed test how it is expected to work.
its more or less the same as initially discussed in #61

After we finished the impl. details I will iterate over the phpdocs a bit.

[evert]

I'm kind of wonderering if we should go with VO or even ValueObject instead of Popo. I got what it meant because of Pojo, but I'm not certain how well known that terminology is in PHP... What do you think?

[staabm]

I am used to the term popo from symfony or propel, but I am also ok with using a more 'pattern-oriented' term like ValueObject.

I guess both terms are only known to experienced php programmers, so we can decide whatever we like best.

[evert]

I think I would lean towards ValueObject, it also sounds a bit better and I feel it requires a bit less explanation...

[staabm]

Ok, so how should the method be named?

mapVo or mapValueObject? I prefer the latter

[evert]

I agree.. was just gonna say that another benefit of using the term VO means that we can fully spell it out (as opposed to PlainOldPhpObject ;))

[staabm]

Will update tomorrow

[staabm]

here we go

[staabm]

I dropped the $this->namespaceMap[$namespace]=null expression 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.

[staabm]

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?

[evert]

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?

[staabm]

This is the Line in the current diff https://github.com/fruux/sabre-xml/pull/66/files#diff-de76feda74829916defbb14d089c7c08R208

[evert]

On the first read this looks pretty great! No comments

[staabm]

This line has a side effect on all elements of the given namespace not only the one named root-element

[evert]

oh yea that does seem like a big problem! Why is it in there in the first place?

[staabm]

Otherwise we would get some aliases in the xml

[evert]

Well I think I would just want to encourage people to always (manually) set their preferred namespace map.

I would also suggest to always use some prefix, and not default to no prefix at all. The fallback with aliases is specifically added because it generates valid (but somewhat ugly) xml, but you, as the xml author, need to pick your preferred namespace prefix (even if that prefix is empty).

So yea this is definitely by design.

[evert]

We could add a new feature that sets the default namespace to null, but only for the root element. So this needs to be detected, and happen outside of of the serializer functions

[staabm]

How do I need to define the namespace, so the xml is mapped to something which I defined myself?

[evert]

as a user of Sabre\Xml\Service, you would set:

$server->namespaceMap['uri'] = 'prefix';

This is how sabre/dav does it:

https://github.com/fruux/sabre-dav/blob/master/lib/DAV/Xml/Service.php

[evert]

So yea, I think the last two tasks are to remove that $this->namespaceMap[$namespace] = null; line and add the missing docblocks =)

[staabm]

After removing the line it wont generate the xml I want it to render.

But I guess the difference is only relevant for the human eye, not a xml parser, right?
This assert https://github.com/fruux/sabre-xml/pull/66/files#diff-4f076a05f0ee0e3c70075dfa9cbe8873R233 would no longer be true.

[evert]

Just add the line to the unittest then ;) You can set the namespace prefixes from outside the Service object, and that's also exactly how you are supposed to do it.

[staabm]

Uff thats too easy... :-))

[staabm]

@evert not sure you got the last ping.. This is ready to land

[evert]

The thing I was still waiting for is better php docblocks. But I can merge this now and I'd be happy to address that before a release. Really great work 👍

[staabm]

Ohh sorry missed the docs :-)