[OJB-developers] RequestProcessor reworked ...

[<Joa...@tp...>]

hy,

okay, this mail grew while I worked on the C/S-model (I started it, when I
thought it was finished, but finished it just 2 hours later), so it might
be a bit confuse and too long to be usefull, so i'll just put an "executive
summary" right at the top ;-)

--- begin executive summary block ---
I did some major refactoring to RequestProcessor, extracting a new class
ConnectionHandler that does all the socket-stuff. This will allow me to
reuse RequestProcessor in my soon-to-come PersistenceBrokerServlet. While
doing this I also cleaned up the RequestProcessor and gave it a much
cleaner structure (IMHO). Of course this also required some modification to
the PersistenceBrokerServer, but they were minimal (no changes to the
PersistenceBrokerClient were necessary, but will be, when I add support for
the Servlet).
The result works great for me. All JUnit tests pass (except for the
mandatory failed test using MySQL) using C/S. The only drawback I see is
that srvPing lost a minimal amount of functionality, but that seems to be a
diagnostic function.
Just unzip the attached server.zip to src\ojb\broker\server to try the new
classes.
--- end executive summary block ---

For those, who are willing to follow me through my odyssey here are the
gory details:

--- begin gory details block ---
right now I'm deeply into the C/S-model of ObjectBridge. But I'm having a
problem. The JUnit tests worked well in singlevm mode on mysql (except for
the one Test that failes 'cause MySQL handles CLOBs in a non-standard way),
but as soon as I started it in Client/Server mode it started to act up.

First of all I searched 2 hours for an error until I noticed that my
experimental support for global UIDs in SequenceManagerHighLowImpl (which
was a very, very ugly hack) wreaked havoc, but after this was fixed I still
had problems. After some further investigation (and some prayer that the
cause-exception handling of 1.4 will become more common) I found out that
the server choked on .store() outside of transactions (which I think is
legal, at least in the JUnit tests in singlevm mode it works).

The problem was the code

            broker.store(arg1, (ObjectModification) arg2);
            ((PersistenceBrokerImpl) broker).checkPoint();

i changed it to

            broker.store(arg1, (ObjectModification) arg2);
            if (broker.isInTransaction()) {
                ((PersistenceBrokerImpl) broker).checkPoint();
            }

which at least solved the symptoms. Is the checkPoint really necessary?

Later on I found one more problem: When the Client executed a
beginTransaction after already haveing done some talking with the server
(outside of an Transaction) the Server chocked, 'cause it thought that the
fact that the client already got an brokerId automatically ment that it
already had a running transaction (which is wrong, AFAICS).

After I got the default C/S-System to run I refactored the RequestProcessor
to take out the Socket-Handling code to allow me to reuse it for my
soon-to-come PersistenceBrokerServlet. The Socket Handling Code became a
new class ConnectionHandler (I'd have made it a private inner class of
PersistenceBrokerServer, but I know that most other ppl don't like this
very much).

 Then I started refactoring dispatch(). Most of the methods like store(),
open(), getUniqeId() have only two lines difference (the common code beeing
the brokerId-is-still--1-code and the exception handling). I moved both the
brokerId-handling code and the exception handlig out of the specific
functions and as the remaining functions all had only one or two lines left
I inlined them into the call, saving some hundered lines of codes. I know
this is a matter of taste, but I prefer it this way, I'd like to hear your
oppinion on this one.

Basically the major improvement (IMHO) was that I changed the broker
allocation scheme. Now if the client provides a brokerId this broker is
always used (which opens a huge security hole, but this was not different
before). At the end of the-function-formerly-known-as-dispatched (which I
renamed to execute, to reflect its new functionality) the broker is
released, unless it has a running transaction (which only happens, when the
client has requested a beginTransaction). This seems to work fairly well.
At least I now pass all JUnit Test while in Client/Server mode (except of
course PersistenceBrokerTest.testEscaping which MySQL can't pass).

before I forget it: one functionality was lost and I don't know yet what to
do about it: srvPing now no longer can use  connection.getLocalAddress() as
it is handled in RequestProcessor which no longer knows about the socket.

Unfortunately I can't make a reasonable diff, due to the newline-problem in
CVS (it allways produces a diff that is twice the size of the files to be
diffed), so I'll just include a .zip with the two changed files
(PersistenceBrokerServer and RequestProcessor) and the new File
(ConnectionHandler).
--- end gory details block ---

btw, shouldn't static finals (as in PBMethodIndex) be written in
all-uppercase (i.e. ABORT_TRANSACTION instead of abortTransaction)?

regards
Joachim Sauer

P.S.: You don't need to send the message to both the list and me, I'm
allready subscribed ;-)


(See attached file: server.zip)