Rest refactor

[juan-castrillon]

Initial commit ot eventually solving #57

There are still things missing, but as of right now I've implemented:

• Refactored the RestServer runner, so there is no need for a router anymore, also got rid of waiting method using built in methods from spark
• Created a FromPOST master input, that can be configured with a path and a command and sends the json body from the request as part of the defined command. As of right now test $json just prints the received json to console
• This input blocks until there is a command available in REST(As opposite to the constant exception throwing we had before)
• Created a toGET master output that takes a json body and is configured with a path to create a resource to be consumed
• Both components sync with the rest server, so they only define routes when the server is up
• Both components use FIFO queues to store the commands read/to send.

Right now, is too much of a sketch, names will be changed and the past implementation is still there. Also tests are missing. However, is working (I did manual tests with postman).

I would like to know which feature I am missing. For example the thing we discussed with replacement strings in the input is not clear to me, could you maybe give me an example of how that would look like @alyhasansakr ?

[alyhasansakr]

@juan-castrillon That is the first part of what we agreed to do, the other part is to allow providing JSON elements as replacement strings. So instead of just test $json you should be able to write test $id $name where id and name are elements of the sent JSON string. Of course it is limited to just the basic types (no objects and no arrays), but most use cases are just like that.

[juan-castrillon]

In these last commits:

• I updated Jackson to the latest version because I figured we can use the new polymorphic deduction deserialization. I've tried to implement it, but it requires changes on the raw data classes and given that when implementing the modules this will change,I figured we can wait. However, it will allow us to define classes and use inheritance and make the json parser aware of subclasses just by parameters.
• I did implement polymorphism in Jackson but according to a parameter @IOType, which is optional (If not present, the IO will be parsed to IOConfigData like normal, and now CustomCommand will define an input that has two extra fields command and information
• Now the fromPOST input can be defined in the following way:

  {"type": "GenericPOST","name": "rest_in","address": "/post_test","port": 4897,
        "command" :  "test $name $number",
        "information": ["name", "number"],
        "@IOType" : "CustomCommand"
  }

  Where the parameters in information are the ones that are expected to be parsed from the JSON body.

• The type of the arguments is infered using Jackson library, but as the parameters are passed as part of the command, they are all at the end converted to strings (the values). However, i believe the actual implementation with the swtich case and the options that jackson provide, will eventually help us support objects and arrays if needed. You can see this in UnknownJSONObject.java
• I also modified ConsoleOutput to print more than one argument, just for the sake of testing, I can roll it back

Right now the desired functionality is working. With configuration as shown below, and sending the following request to the path localhost:4897/post_test:

{
  "name": "example",
  "number": 874
}

Configuration:

{
  "ioData": {
      "inputs": [
        {"type": "ConsoleInput","name": "console_in"},
        {"type": "GenericPOST","name": "rest_in","address": "/post_test","port": 4897,
          "command" :  "test $name $number",
          "information": ["name", "number"],
          "@IOType" : "CustomCommand"
        }
      ],
      "outputs": [
        {"type": "ConsoleOutput","name": "console_out"},
        {"type": "GenericGET","name": "get", "address": "/get_test","port": 4897},
        {"type": "MasterUtility","name": "util"}
      ]
    },
  "connections" : [
    {
      "in" : "console_in",
      "out" : "util",
      "commands" : {
        "exit" : "util exit",
        "start_rest_server" : "util runRunner RestServer",
        "start_runner $runner_name" : "util runRunner $runner_name"
      }
    },
    {
      "in" : "console_in",
      "out" : "get",
      "commands" : {
        "get_test" : "toGet resource {\"name\": \"example\",\n\"number\": 874}"
      }
    },
    {
      "in" : "rest_in",
      "out" : "console_out",
      "commands" : {
        "test $name $number" : "console $name $number"
      }
    }
  ]
}

The result on the console is

example 874

Missing are of course the automated test, but before that I wanted to make sure, nothing else is missing 😄

[alyhasansakr]

It looks good, but how do we define more than one resource here? lets say I want another resource on address "/post_test_1" on the same port, do I just write another input?

[juan-castrillon]

Right now, yes, although I will have to modify the code to make sure the rest server is just started once. Is that ok? Or should it just be one input and make possible different paths?

I decided for this way, as it allows to refer to each input separately in the connections, so we dont need to connect all rest inputs to one output but just the one it needs.

[alyhasansakr]

For now that should be enough, but after Modules are implemented we should have another look here.

[juan-castrillon]

It is almost ready for review. New features and things done in these commits include:

• There is a new type of GETOuput that deals with parameterized paths (like path/:client_id) is transparent to the user in the configuration file
• The configuration file can now be defined passing a path as a running argument (.\gradlew run --args=path), if no path is passed it will default to out configuration file
• I tested manually (and changed the automated tests) for MatchMaking and NodeLimit with the new rest
• Added a NodeLimiter that is both input and output to deal with the node limit responsability (before it was in the rest input but had to change as is generic now)
• Modified match maker
• Modified the tests so each work with a different configuration file
• Did automated test for the new REST

Missing are: -Testing correctly the configuring via run argument in the master

• Fixing the exit command which is not working anymore, due to inputs blocking
• Clean master from unused methods (No longer registering nodes in the master for example)
• Fix exception testing as the function we use is deprecated now (I have done this in some of the tests)
• Maybe modify all test so they run with different configurations (That way we also have them in the source code)
• I am still not sure whether a FIFO Queue at the output is the way to go, as as soon as the resource is consumed by some client, it will dissapear

[alyhasansakr]

I don't believe a FIFO at the output is necessary, once a resource is put there it should stay until next modification.

[juan-castrillon]

In the last commits, I did all that was missing in my last comment (Except for cleaning the master, which I figured will do after the review)

• Got rid of the FIFO queue for the output
• Fixed the exception testing in all tests, and added custom master configuration when needed
• Modified the tests of the REST so they dont hang (was happening some times)
• Modified the blocking inputs (REST, Matchmaker, NodeLimit) so they allow to exit the programm and therefore also modified exit functionality

I believe is ready for review

[juan-castrillon]

@alyhasansakr You are probably right but just for the sake of me learning would you mind helping me understand why? After doing some research, The way I see it both ways are ensuring mutual exclusive access to the resource and the only difference I can see is that semaphores would eventually allow more threads to access but a binary semaphore takes that away.

[alyhasansakr]

@juan-castrillon Correct, they are exactly the same in our case. Actually, Semaphores implementation potentially uses synchronized blocks. Semaphores add fairness and multi-access (for non-binary), but this is not the reason. The reason is that you don't need to implement the waiting logic (while loops and stuff) you just need to call acquire and release functions. In short, synchronized blocks is the low level solution while Semaphores is the high level solution.

[juan-castrillon]

Thanks for the help @alyhasansakr. In the last commits I changed the sync blocks (both in the blocking inputs and in the syncronization with the RestServer and IOs) to semaphores. You were right, it simplifies the code.

On a side note I realize that now MasterInput can be an abstract class instead of an interface and the semaphore (and blocking methods) can be abstracted there, however, as we still have components that are both MasterInput and MasterOutput and java doesnt allow multi-inheritance, I say we wait to do this until implementing the modules refactor where that (both I and O) will no longer be the case. What do you say?

[alyhasansakr]

Yes, this changes and more should be done as part of the new modules refactoring.