limweb / sabreamf

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

Expose additional request information to the callback function. #15

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
With our project we need to carry out a lot of async calls to the server,
so the server needs to utilize shared memory to share data between these
calls in many cases.

As an example, our Flex client will often request the server to carry out a
lengthly process in one connection, then constantly check the progress of
it in another connection to report back to the user.

Luckily AMF3 attaches a GUID to each request/message, which is ideally
suited for this situation as the both the client and server know the
messageId and thus can be used as a basis for a unique lookup key. However
SabreAMF doesn't expose that information through the invokeService callback
function.

As such I propose adding an additional "$extras" argument to invokeService
where we can pass any additional information that we need to now, and in
the future. I did some quick testing and it appears as though
call_user_func_array() will ignore additional arguments, so this change
should be 100% backwards compatible.

In the attached patch for review, my question is if I should restrict what
data is passed along as the "$extras" array, should we send along as much
information as possible now, or should we take an entirely different
approach to exposing this information?

Original issue reported on code.google.com by mike.ben...@gmail.com on 23 Sep 2009 at 7:48

Attachments:

GoogleCodeExporter commented 9 years ago
Hi Mike,

Since this is a pretty easy change, and right near the edge of the system
(callbackserver), I would suggest just subclassing the callbackserver, and 
overriding
this function. I don't really see a need to add this for everybody..

Original comment by evert...@gmail.com on 23 Sep 2009 at 8:36

GoogleCodeExporter commented 9 years ago
We would need to overload the Exec() and invokeService() functions to do this 
though,
and that contains a significant portion (over 60%) of the code in the CallBack 
server
class. 

It would be much easier to maintain our own fork with the attached 3 line patch.

I was trying to see if there was a way to access the SabreAMF object and obtain 
the
information directly from the callback function, but I wasn't able to come up 
with
anything as clean as the attached patch. 

I mean we could refactor Exec() slightly to put another function between it and
invokeService, to it could then be easily overloaded, but we still need to 
expose
that data at some point. This method seems like it would take more code and be 
slower
than the attached patch.

Do you have any other ideas perhaps?

There is actually quite a bit of information contained in the AMF protocol that 
is
exposed and easily accessible from Flex that can be incredibly useful. 
messageId is
just one of those values that is most useful to us at this point, but it just 
seems
to make sense that if Flex can access this information that the server should 
be able
to as well. 

Original comment by mike.ben...@gmail.com on 23 Sep 2009 at 9:16

GoogleCodeExporter commented 9 years ago
I would say just fork the class.. 

I understand where you're coming from, but I'd really like to try to keep 
changes to
an absolute minimum. 

Original comment by evert...@gmail.com on 23 Sep 2009 at 9:41

GoogleCodeExporter commented 9 years ago
Are you proposing that SabreAMF is effectively "dead" when it comes to 
additional
features? 

Do you suggest that we fork SabreAMF into an entirely new project in order to 
further
enhance and improve it? The patches we have submitted so far are just the tip 
of the
iceberg, we have many more already completed and are starting to look at other 
more
intrusive things like supporting AMFEXT and such. We very much consider this to 
be a
long-term on-going project.

We would much rather work with you and contribute these changes back to 
SabreAMF, but
I can understand if this is not your plan for it.

Original comment by mike.ben...@gmail.com on 23 Sep 2009 at 10:10

GoogleCodeExporter commented 9 years ago
I was actually really just talking about that one class.

I must say I have interest in a possible 2.0 version, but I don't see this 
happening
within the next 6 months. As of right now I only want to fix bugs, and not add 
on any
new features.

If you strongly feel the need to fork the current codebase to add things like 
AMFEXT,
I would be in no way offended as I understand how annoying this must be. I 
personally
just feel I can't spend the time working on this project unfunded in the 
short-term. 

I do feel strongly about keeping the current codebase as-is, and staying in 
control
of it. I'm hesitant making even small changes, because I don't have the time and
resources to do any real testing.

Original comment by evert...@gmail.com on 23 Sep 2009 at 10:39