plugin-fcp-rewrite (with purge-db4o merged already)

### Merge situation

By "with purge-db4o merged already" I mean that I recently merged branch next into this, so you should be able to merge it without conflicts.

### Using plugin-fcp-rewrite

A good entry-point for understanding the code is to read the JavaDoc of:
- Function PluginRespirator.connectToOtherPlugin() which is the entry point which allows plugins to use plugin-fcp-rerwite.
- Interface FredPluginFCPMessageHandler, which plugins implement to handle received messages. It notably specifies the format of the messages.
- Class FCPPluginClient, which plugins use to send messages to each other. In addition to user-focused JavaDoc, it contains a developer/maintainer-focused overview of internal code paths which messages flow through, suitable for people who want to dig into the plugin-fcp-rewrite code.

### Changelog of what plugin-fcp-rewrite improves

- As opposed to the old PluginTalker and PluginReplySender interfaces, this new API is as symmetric as possible:
  - With the old interface, the server could only <i>reply</i> to messages of the client. It could not keep a reference to the client to send messages to it in the future to deploy events, which is absolutely critical for efficient implementation of things such as WOT.
    With this implementation, server and client are free to send messages to each others whenever they like to.
    The only restriction upon this is that the opening and closing of connections is dictated by the client. The server cannot connect to a client on its own.
  - With the new interface, it is now possible for client and server to send messages to *themselves*, i.e. server-to-server and vice versa, while pretending that they came from the remote side. While I must admit that I currently cannot think of a specific use case, it seems probable that one will arise some day. A theoretical example might be that it can be necessary to do something in a new thread - sending a message to yourself will cause it to be processed in a fresh one.

- It is more similar to standard remote procedure call:
  - Each message dialog typically involves an initial message (= a procedure call), and receives a reply message (= return value, indication of success, error code, translated error message).
  - Previously, reply messages were optional, and their format was not standardized. Nothing did even indicate whether a message was a reply. Previously, clients were free to stick error messages anywhere in the SimpleFieldSet. Now we have a standardization of when reply messages happen, and their result format (success/failure, error code, translated error message).
  - The relationship between message and reply is now is now clearly identified by the FCP "Identifier" field. Previously, it would be difficult to impossible to clearly track reply messages because their identification depended upon whether the client properly used the already existing "Identifier" field: This would usually not be the case as the identifier's location in the class hierarchy made it look like a connection identifier, not a message identifier. So clients would use the same identifier for multiple unrelated messages, which made it impossible to determine to what the client is replying. But the documentation of FCP and common sense clearly shows that it should be a message identifier, because only that allows proper tracking of message results (and because connection identifiers should be chosen by the server, not client, to prevent collisions with other clients). plugin-fcp-rewrite makes it very clear that its a per-message identifier, and provides means to use it as such (= implicit automatic random generation per message).

- Because we *can* track reply messages now, we can have an additonal function sendSynchronous(), that is a *blocking* send function which waits for a reply from the remote side and returns it. Previously, there was only send send() which would return immediately, and the reply would arrive as a callback in another thread. My development of two WOT client applications (Freetalk, and the WOT FCPClientReferenceImplementation) and review of several other WOT client appliations has shown that a blocking send is needed because WOT clients already contain at least 4 multiple, possibly bugged wheel-re-inventions of synchronous sending (Freemail, Freetalk, Content Curation, FlogHelper - dunno whether they all sucked but at least the one I wrote for Freetalk did). So its better to do it once and the right way. The advantages are:
  - ability to wait for a reply in stuff such as a GUI, where it makes sense to wait for it: If you use the web interface of a WOT client application to create an identity, you want to immediately get feedback whether the operation succeeded, or maybe failed due to invalid characters in the nickname, etc.
  - ability to not only wait for an "ACK" but actually receive the reply message in the same thread instead of in a message handler thread
  - ability to enforce a proper order of messages. Ordering is important for some algorithms, such as event-notifications of WOT. Events are a record of the past, and thus they wouldn't make sense if their order was changed.
  - The disadvantage of this possibly causing thread congestion and bad code design shall be alleviated by:
    - enforcing the caller to specify a timeout, and having an assert(timeout is smaller than sane maximal value) .
    - designing the thread in sendSynchronous() which waits for the reply to properly handle InterruptedException so the user can interrupt() all of the threads during shutdown.
    - having huge, bold warnings in the JavaDoc which provide examples when to use and when to NOT use synchronous sends.

- Interface and class names are easier to understand:
  - FredPluginFCP (old) vs FredPluginFCPMessageHandler.ServerSideMessageHandler (new)
  - FredPluginTalker (old) vs FredPluginFCPMessageHandler.ClientSideMessageHandler (new) [For symmetry, this is actually the same interface as ServerSideMessageHandler - due to the parent interface FredPluginFCPMessageHandler. The child interface merely exists to javadoc the different rules of behavior allowed for servers / clients.]
  - PluginTalker (old) vs FCPPluginClient (new)
  - PluginReplySender (old) vs FCPPluginClient (new). Notice that the old version duplicates the above PluginTalker as a representation of the client. This is the aforementioned non-beautiful asymmetry of the old code: To the client application, the client was a PluginTalker, to the server application, it was a PluginReplySender. Now there is just one class - FCPPluginClient - which is used by both server and client.

- The public API, that is anything which shall be used by server / client plugin implementations, has very extensive JavaDoc which specifies how it can and must be used. This will prevent misuse as it has happened with the previous API as explained above. The previous API was undocumented. Thats another reason why it was chosen to rewrite the stuff instead of extending it: Backwards compatibility would have been very difficult to do without documentation which specifies what exactly we have to be compatible to.

- Also, even the internal part of the new code is documented very extensively to ensure that it will be easy to extend. This was indicated by the fact that I had to re-read the code of the old API every few months to understand how it works even though I had worked rather often with it. It was a strange architecture. Now there is a large overview of the internal paths through which messages flow at class FCPPluginClient.

- It should be mostly backwards compatible with regards to external libraries: What was changed about the on-network format of FCP is only addition of new fields. The existing fields still exist, albeit their semantics has changed a bit (which shouldn't break stuff, its mostly the "Identifier" field which has been explained above to have been wrongly used in the past anyway). Backwards compatibility should be as follows:
  - Server plugin which only implements the old API + Client which only implements the old API = Should keep working.
  - Server plugin which only implements the new API + Client which only implements the old API = Should keep working as long as the client is connected by network, not running as a plugin. "By network" = External libraries will keep working. The incompatibility between old clients which are running as *plugin* instead of being connected by network and new server plugins is due to the fact that messages between plugins flow through a "shortcut" in both the old API, and the new API - they won't hit the network, but instead be passed through Java functions directly. Those shortcut interfaces were poorly specified in the old API, and it would have been more work to adapt them to be able to communicate with the new code than to fix existing plugins. We have to touch all plugins to compile-fix them againt the fred API changes in purge-db4o anyway. If purge-db4o may break fred Java API, I also may :) Also notice that old plugins *will* still compile and load fine, they just won't be able to connect to new ones such as WOT. Will deal with fixing the affected official plugin FlogHelper soon.

- It is possible to specify the thread priority of each individual message! For example this could be used to specify a higher priority for FCP functions which are typically used by the UI.


Unrelated, bonus code in the pull request - had to write this for being able to adapt WOT to plugin-fcp-rewrite nicely:
- General code quality improvements were made to ease the creation of full nodes within unit tests. This was done so I can test WOT FCP within a unit test. An example of how to create a node in unit tests is at https://github.com/freenet/plugin-WebOfTrust/blob/e62fb5824381dcb27abd38a0c65888a13b73e2e3/test/plugins/WebOfTrust/AbstractFullNodeTest.java ... Some of the aforementioned changes are:
  - It's now possible to specify the random number generator of those fake unit test nodes to be a DummyRandomSource. This is a lot faster than the standard Yarrow because it does not wait for entropy, and it allows specification of a seed for determinism and being able to log the seed of unit tests so their results can be reproduced.
  - It's now easily possible to specify the directory where the node will store its files, so you can tell JUnit to delete them afterwards. JUnit 4 is needed for this AFAIK.

### Testing situation

The following aspects of plugin-fcp-rewrite have been tested *before* merging purge-db4o into it:
- Whether communication between a plugin-fcp-rewrite server plugin and a plugin-fcp-rewrite client plugin works. This has been tested very throroughly with the WOT adaption to plugin-fcp-rewrite and its reference FCP client.
- Whether communication between a plugin-fcp-rewrite server and a networked pre-plugin-fcp-rewrite client works. This has been tested briefly using jSite.
- Whether communication between a pre-plugin-fcp-rewrite server plugin and a pre-plugin-fcp-rewrite client plugin works. This has been tested briefly using old WOT and Freetalk.

In other words: All combinations of new server / old server / new client / old client which have above been promised to be compatible have been tested.

**Notice:** plugin-fcp-rewrite was NOT tested again *after* merging purge-db4o into it: AFAIK, none of the plugins which offer FCP have been ported to purge-db4o yet, so there is nothing to test plugin-fcp-rewrite with. I will very soon start adapting WOT to purge-db4o, and then test plugin-fcp-rewrite again. Nevertheless, it would be nice if you could merge the pull request already: The merge was done carefully, post-merge compiler issues have been fixed, and the code compiles fine now. And as WOT is an official plugin, its unlikely that purge-db4o will be deployed before it has been fixed to work with it, so whats needed to test the purge-db4o adaption of plugin-fcp-rewrite needs to be finished before purge-db4o can be deployed anyway.
What is the advantage of absolute symmetry here? It strikes me as both
actively confusing and pointless - why would something need to involve FCP
in sending a message to itself? It may be shiny from an API standpoint but
I'm not convinced it's worthwhile.

On Thu, Dec 11, 2014, 8:52 PM xor-freenet <notifications@github.com> wrote:

> In src/freenet/clients/fcp/FCPPluginClient.java
> <https://github.com/freenet/fred/pull/319#discussion-diff-21724749>:
>
> > +     *             should then discard it and obtain a fresh one.
> > +     *
> > +     *             <p><b>ATTENTION:</b> If this is not thrown, that does NOT mean that the
> > +     *             connection is alive. Messages are sent asynchronously, so it can happen that a
> > +     *             closed connection is not detected before this function returns.<br/>
> > +     *             The only way of knowing that a send succeeded is by receiving a reply message
> > +     *             in your {@link FredPluginFCPMessageHandler}.<br>
> > +     *             If you need to know whether the send succeeded on the same thread which shall
> > +     *             call the send function, you can also use {@link #sendSynchronous(SendDirection,
> > +     *             FCPPluginMessage, long)} which will return the reply right away.</p>
> > +     * @see #sendSynchronous(SendDirection, FCPPluginMessage, long)
> > +     *          You may instead use the blocking sendSynchronous() if your thread needs to know
> > +     *          whether messages arrived, to ensure a certain order of arrival, or to know
> > +     *          the reply to a message.
> > +     */
> > +    public void send(final SendDirection direction, FCPPluginMessage message) throws IOException {
>
>  Both ends are called the client? That there's some confusing naming!
>
> Hmm. How about renaming FCPPluginClient to FCPPluginConnection? I also
> like that more I think.
> "Connection" is more what it is, and doesn't imply that it belongs to the
> client.
>
> I propose the following:
>
> Rename this class to FCPPluginConnector or something similar
> Make send(SendDirection, FCPPluginMessage) private
> Have this class implement both of the following interfaces:
> public interface FCPPluginClientConnector {
>
> public interface FCPPluginServerConnector {
>
>
> Only provide the ServerConnector to the server, only provide the
> ClientConnector to the client, and use the raw FCPPluginConnector only
> internally.
>
>  Only providing one to each would complexify the code, and be
> non-symmetrical. Right now we have a symmetrical interface
> FredPluginFCPMessageHandler which is implemented by both the server and the
> client. The send() function thus can deal with both client and server, it
> doesn't care about whether its sending to server or client.
>
> I suggest the following combination of operhiem1's general complaint of
> the poor name and bertm's suggestion to introduce convenience-functions
> without the enum:
>
>    - Rename FCPPluginClient to FCPPluginConnection
>    - Add the convenience-functions sendToServer(), sendToClient(),
>    sendSynchronousToServer(), sendSynchronousToClient() but keep everything
>    else as is.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/freenet/fred/pull/319/files#r21724749>.
>