limweb / sabreamf

Automatically exported from code.google.com/p/sabreamf
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

Feature request: AMFEXT #4

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Would it be possible to support the PHP extension AMFEXT [1]?

The slower speed is the only thing that stops me from using it. It's about
100 to 150ms slower than AMFPHP when I transfer 500 simple objects/arrays.
See the sample PHP code below.

[1] http://www.teslacore.it/wiki/index.php?title=AMFEXT

<?php

class Student
{
    public $ID;
    public $name;
    public $phone;

    function __construct($ID, $name, $phone)
    {
        $this->ID = $ID;
        $this->name= $name;
        $this->phone = $phone;
    }
}

class StudentService
{
    function getStudents()
    {
        $ret = array();

        for ($i = 0; $i < 500; $i++)
        {
            $data = array(
                'ID' => $i,
                'name' => 'John Doe',
                'phone' => '999-999999'
            );

            array_push($ret, $data);
        }

        return new SabreAMF_ArrayCollection($ret);
    }
}

?>

Original issue reported on code.google.com by gertschi83@gmail.com on 11 Mar 2008 at 9:37

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
I'll definitely have a look at this, been looking at it before, and the api is 
not
great.. Will have to make some wrappers around it to make it work a bit nicer..

Original comment by evert...@gmail.com on 12 Mar 2008 at 4:51

GoogleCodeExporter commented 9 years ago
I'm also interested in this.

I have hacked it in there (just into the Message.php, using amf_encode and 
amf_decode
instead of writeAMFData and readAMFData), in the attached diff (against SVN 
r194). 
But I don't have any unit tests - so while this works for my little sample 
apps, I
don't know whether this will be reliable for all cases.

Original comment by grant....@gmail.com on 15 Apr 2008 at 2:16

Attachments:

GoogleCodeExporter commented 9 years ago
Hi Grant,

great work.. This will definitely help me implement AMFEXT support.. 

This will work for the basic features, but it doesn't really deal with 
classmappings
and the other more advanced things.. I'd like to keep compatibility with the 
PHP-way,
so I'd need all that..

I don't currently have testcases, but really want to get that up and running as
well.. I have a few things planned for a 2.0 release, and full-on AMFEXT 
support is
one of them..

Original comment by evert...@gmail.com on 15 Apr 2008 at 5:52

GoogleCodeExporter commented 9 years ago
Hi

I've used the patch from grant.cox and extended it a bit to support of the 
objects.
Unfortunately, amfext has to be patched too (the patch for amf.c agaings the 
latest
version from pecl cvs repository). It did not support serializing of 
externalized
classes and had some other bugs (for example
http://pecl.php.net/bugs/bug.php?id=12668, but it's not the only one)

Original comment by alex.dem...@gmail.com on 23 Jul 2008 at 3:18

Attachments:

GoogleCodeExporter commented 9 years ago
Alex - great, thanks for that.  I have a couple of questions though:

AMFEXT 0.9.2 was released, apparently in response to that bug you linked to 
above
(cool, hey!) - do you know if this has also included fixes for the other bugs 
you
mentioned?

With your samberamf.path file - I think you want "self::$amfEndian" on line 172 
(of
the patch).  Also, are the headers always AMF0, so the $amfVersion isn't 
required?

Finally, I think that line 204 of the patch should have "&& $body['data'] 
instanceof"
- otherwise I would assume it compares the boolean output of is_object in the 
instanceof.

I'll implement your patch into our application, and post back if I find any 
issues
(highly unlikely - we just transfer anonymous objects / assoc arrays).

Original comment by grant....@gmail.com on 24 Jul 2008 at 12:27

GoogleCodeExporter commented 9 years ago
Pretty good news, I wanna give this a test run myself

Original comment by evert...@gmail.com on 24 Jul 2008 at 12:37

GoogleCodeExporter commented 9 years ago
about "line 172" (amfVersion):
first of all, amfext_decode doesn't not need this flag actually (i see it in 
amf.c). 
AMF3 Encoding is defined by the 0x11 flag SOMEWHERE (means - smth before could 
be 
still AMF0) in the input stream. If the parser sees it, at this moment it must 
switch to the AMF3 decoding procedure. 

So, back to the context headers. The names of the headers are the strings, not 
a 
string type (sounds strange) from AMF Spec point of view. So, they are encoded 
as 
<16bit length + the string itself>, while the message itself is AMF0. The 
actual 
value of the header could be AMF0 or AMF3, which is defined by 0x11 flag in 
input 
stream. And this will be handled correctly by amfext_decode or SabreAMF (look 
inside 
of AMF0/Deserializer.php)

Now about the message body. SabreAMF actually starts always from AMF0. If it 
sees 
during the parsing procedure any AMF3 "pieces", the oneremembers the encoding 
type 
only for the later response encoding procedure. SabreAMF knows than, that 
client 
understands AMF3 and can use it to save a lot of bytes by using the references. 
But, 
again, it still AMF0 at the very first moment until the parser encounters 0x11 
- 
because $deserializer is AMF0/Deserializer.php

Summarize: $amfVersion is not required for the decoding procedure. The 
corrected 
patch is in attachment. Sorry about that, but i had to look more exactly at amf 
specs (both amf0 and amf3) to undestand this. 

There is still the small issue: if the first bytes of an amf message tells AMF0 
is 
coming, but the body will contain any AMF3 encodings, SabreAMF will response 
correctly with AMF3, but amfext version - not. There is no way to catch this 
situation - amfext doesn't return/set any flags, telling that it found AMF3 
inside.

about "line 204"
you're right. my fault :) I had older version of SabreAMF and did not merge 
correctly my changes in the new one.

about amfext patch:
unfortunately, not. I didn't send him my changes yet. I wanted, but... he 
didn't 
response for my messages and i thought the PECL module is forgotten or 
something 
like this... I will open the new bug in PECL about the required modifications...

Original comment by alex.dem...@gmail.com on 24 Jul 2008 at 10:57

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks for the explanation Alex, that's interesting to know.

I was thinking that this lack of identification of whether the request was AMF0 
or
AMF3 was quite an oversight by the original author - until I saw that the
amf_decode() definition is meant to have the second parameter (for the flags) 
passed
as reference! (see amfext-0.9.2/docs/amfextdoc.php).  Sure enough, pass the 
flags
parameter by reference and it will be updated if there was AMF3 data decoded.

Also, we can't pass in $this->encoding as a flag to amf_encode - as the 
definitions
for SabreAMF_Const::AMF3 and AMF_AMF3 are not the same (3 vs 1).

So, I've attached another diff, which adds these two parts.

Original comment by grant....@gmail.com on 25 Jul 2008 at 3:00

Attachments:

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
heh, you're right :) I simply didn't see it. Lucky, SabreAMF_Const::AMF3 is 
'11' 
(bits) and AMF_AMF3 is '1' so the logical & gives in both cases 1. And because  
AMF_BIG_ENDIAN is '10', $this->enconding was updated with '111', which is  
SabreAMF_Const::AMF3 :)))

Small correction to your patch:
$amf_flags = $this->amfEndian;
$body['data'] = amf_decode($stream->rawData, $amf_flags, $stream->cursor, array
($this, '_amf_decode_callback'));

there was wrong var name and '&' is not neccessary in the function's call 
statement

Original comment by alex.dem...@gmail.com on 25 Jul 2008 at 6:51

GoogleCodeExporter commented 9 years ago
Hey guys, 

Would you be interested in SVN access to this project? I'd love to see this 
developed
in an SVN branch 

Original comment by evert...@gmail.com on 31 Jul 2008 at 5:38

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
I'm not sure that I personally would have much to add - the problem is that my 
use of
AMF is fairly limited - we use it (using AMFPHP / SabreAMF) in a number of 
projects,
but really just use it to transfer simple data types (arrays and anonymous 
objects).
 Our application handles pagination and data types, so we don't use the more
interesting AMF objects (ArrayCollection, XML, Dates).

Actually, perhaps a test suite would be a good idea - something that does test 
all of
these cases out.  I've had a look before at other AMF en/decoders and found very
little in this space (although PyAMF from memory does have some good tests), 
and it'd
certainly be useful when looking at things like integrating AMFEXT.

I'll have a look into this and report back.  For the decoding we can easily use
Charles request dumps, not sure about the encoding (I guess you have to look at 
Flash
Media Server to get an authoritative encoding?).

Original comment by grant....@gmail.com on 1 Aug 2008 at 12:31

GoogleCodeExporter commented 9 years ago
PyAMF does have a quite extensive test suite.. its pretty impressive 

Original comment by evert...@gmail.com on 1 Aug 2008 at 12:45

GoogleCodeExporter commented 9 years ago
But yea, be sure to let me know about getting SVN access. There's no commitment 
what
so ever, I think it would simply be helpful for tracking progress and it'll 
allow
myself to make alterations as I'm changing the API.

Original comment by evert...@gmail.com on 1 Aug 2008 at 3:37

GoogleCodeExporter commented 9 years ago
yes, this would be interesting. I use classes (VOs) actively in my projects and 
without AMFEXT the conversion is pretty slow on big arrays...

P.s.: sorry for the delay with an answer. Was in vacation...

Original comment by alex.dem...@gmail.com on 12 Aug 2008 at 8:15

GoogleCodeExporter commented 9 years ago
Has there been any additional progress made on this front? I'm really 
interested in
getting SabreAMF to work with AMFEXT as well.

Original comment by mike.ben...@gmail.com on 18 Sep 2009 at 6:24

GoogleCodeExporter commented 9 years ago
I have personally simply lost interest in adding any new features. A feature as 
big
as this will definitely take some time I just can't afford at this moment. 

I've committed to fix bugs as they come along, as a big thank you for the people
using it; but the addition of AMFEXT is just too big for me right now.

Original comment by evert...@gmail.com on 22 Sep 2009 at 7:15