Skip to content
This repository has been archived by the owner on Oct 29, 2019. It is now read-only.

Merge 3.0 and add new DB management and REST V2 #292

Merged
merged 63 commits into from Jul 15, 2019
Merged

Merge 3.0 and add new DB management and REST V2 #292

merged 63 commits into from Jul 15, 2019

Conversation

marakiis
Copy link
Contributor

Merge fixes from 3.0
Add new DB management DAO and POJO
Add Rest v2

@bcarlin Do you have an insight on the "merges" commits (2015 to early 2018)

@marakiis marakiis added this to the 3.1.0 milestone Jun 14, 2019
Copy link
Contributor

@fredericBregier fredericBregier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly 4 kinds of questions on your perfect job!

  • related to DbSession no more used so to remove probably from all the method's signature
  • related to replacement of PreparedStatement with dynamic call each time (and not using therefore the "next" of results)
  • one related to backward compatibility (fromSsl in local packet)
  • one maybe to delay related on removing the wait for localChannel

Copy link
Contributor

@fredericBregier fredericBregier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding one item so

Copy link
Member

@bcarlin bcarlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just some minor changes

maven-version-rules.xml Show resolved Hide resolved
maven-version-rules.xml Show resolved Hide resolved
src/main/java/messages_en.properties Outdated Show resolved Hide resolved
src/main/java/messages_fr.properties Show resolved Hide resolved
src/main/java/org/waarp/openr66/context/R66Session.java Outdated Show resolved Hide resolved
@bcarlin bcarlin mentioned this pull request Jun 19, 2019
@fredericBregier
Copy link
Contributor

fredericBregier commented Jun 19, 2019

I would like also that you do some cleaning in the history:

  • from Commits on May 14, 2019 to last one: making one commit for all (since it was the very beginning of this great work) (git rebase -i HEAD~28 or +1 or more if you add mine or more of yours)
    Keeping a clear history is important too... ;-)

Note: I update my MR to your branch in order to clean it

@bcarlin
Copy link
Member

bcarlin commented Jun 26, 2019

  • from Commits on May 14, 2019 to last one: making one commit for all (since it was the very beginning of this great work) (git rebase -i HEAD~28 or +1 or more if you add mine or more of yours)
    Keeping a clear history is important too... ;-)

Actually, this PR has gotten out of control, it is far too large. It covers 3 topics : a port of the fixes of the branch 3.0 to the branch wNetty41; the addition of a new REST API; and the rewrite of the database management layer. These should have been 3 different PRs.
I was not careful enough to block it from the very start.

My proposition now is the following :
From now on, the only changes to be made here are :

  • fixes asked in the review (things that are not still resolved);
  • fixes done in order to make all the full test suite pass ;

Any other change will have to be made in its own PR afterwards.

Edit: If I'm not mistaken, there are not any unresolved points anymore, so when the tests are green, this PR will be considered done

Once there are no more changes to be made (the two points above), a selective squash will be made to simplify history in a few topic based commits :

  • fixes ported from 3.0
  • DB layer rewrite
  • REST API v2

I think quashing all into a single commit would be a mistake here (as we are now), because there are too many different things in here.

Copy link
Contributor

@fredericBregier fredericBregier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some changes still needed. I refer to my proposal (which is no more usable as is in your PR) to complete this final review.

Also, try to keep as minimal as possible the number of added commits (yours from may, not before, even if already not clean before) using a squash (as @bcarlin wrote to 3 commits if possible).

Bruno Carlin and others added 17 commits July 1, 2019 10:34
In EXEC* tasks on windows, if slashes are used for the executable, the resulted
command executed is:

  EXECUTABLE EXECUTABLE ARGUMENTS

with the first EXECUTABLE having antislashes and the second one slashes.

This patch returns the command to its normal form :

  EXECUTABLE ARGUMENTS
Split globalchannel limiter into global limiter and channel limiter
Add two new packages:
 - POJO to store the Data object and use them in the application
 - DAO to access the data persitance layer

Package are populated with the needed class to function with a standard
SQL database.
ppantellini and others added 28 commits July 1, 2019 15:16
- added server status, logs export, and config export to server commands handler
- added ServerStatus POJO to represent general server status
- added more BadRequest and InternalError exceptions builders for common errors
- replaced 'allow' static fields in http handlers with dynamic `options` method in RestUtils
- replaced 'HOST_ID' constants in http handler by a single 'HOST_ID' constant in RestUtils
- changed OpenRestR66 exceptions now extend `Exception` instead of 'Error'
- fixed all unhandled OpenRestR66 exceptions are now handled
- renamed Database classes as TestDatabase and marked them Deprecated (to be replaced by database requests)
- added missing Javadoc to RestUtils methods
- query parameters are now method parameters annotated with '@QueryParameter'
- removed now deprecated RestUtils method 'extractParameters'
- removed support for PATCH method since support was removed from Netty-Http
- removed all Filters classes, replaced by method parameters
- moved Order enums from Filter classes to their corresponding data classes
- changed RestServiceInitializer.init method to use a RestConfiguration object
- changed init method to remove compression from the network pipeline which causes problems with Java 6
- added Content and Accept type verification
- added Basic authentication
- removed all database POJO utility classes and moved their code to their respective Rest handler
- moved all default BadRequest and InternalError common exception builder messages to the new RestResponses class that lists all error responses
- added 'NonWritable' annotation for field that should not be present in the request body but should be in the response body
- separated fields declaration and initialization to be able to check NonWritable fields for unauthorized writes by the ObjectMapper
- fixed a bug in routing error handler message caused by an incorrect content-length value
- changed the types of all query parameters to String to prevent unwanted error by the router when a value of invalid type is entered, parameter type verification is now done in the handler
- removed all 'data' subpackages to account for the removal of the utility classes
- changed request behavior : selection is now done by the database itself instead of the handler, as well as the verification of field values (negative port, ...)
- rebased on branch 'wNetty45'
- added support for new DAOFactory and POJOs
- renamed all Rest data POJOs to differentiate them with database POJOs
- added a method to all Rest POJOs to convert them to their equivalent database POJOs
- added a constructor to all Rest POJOs to create an instance from a corresponding database POJO instance
- added a static method to transfer, rule and host Rest POJOs to generate a List from a List of database objects
- removed 'testdatabases' package
- added 'NotEmpty' annotation for String fields
- added 'Or' and 'Bounds' annotations for numerical fields
- changed 'checkEntry' method to check for 'NotEmpty' and 'Or' annotations conformity
- moved default fields value affectation to their declaration
- added permission check when making a request with authentication
- added `AbstractRestHttpHandler` abstract class extending `AbstractHttpHandler` with a `isAuthorized` method that checks if a user is authorized to make a specific request on the handler
- changed all handlers to now extend `AbstractRestHttpHandler` to be able to use the `isAuthorized` method
- fixed the `checkEntry' method to also check fields attribute recursively
- fixed malformed error message sent when a 404 error is returned by the router
- moved the default 404 and 405 error messages from `RestRoutingErrorHandler` to `RestResponses`
- added : response messages are now defined in English and French in the 'RestMessages' ResourceBundle
- changed : 'RestResponse' is now an abstract class for response message formatters
- added : all classes in the 'error' package which are formatters for their respective response messages
- added : response message language is extracted from the 'Accept-Language' header of the request
- changed : 'OpenR66RestBadRequestException' and 'OpenR66RestInternalErrorException' now take their respective message formatter as parameters
- removes : 'OpenR66RestInvalidEntryException' has been replaced by 'OpenR66RestBadRequestException'
- added hmac authentication
- added privilege checking
- fixed error messages
Fixed time out bug when making multiple requests

{Rest} Improved error handling

- fixed unhandled exceptions during the creation of error messages
- replaced occurences of 'String.format' by 'ObjectMapper' for JSON serialization
- moved all 'RestUtils' constants to 'RestConstants' class
- media type handling moved to each entry point for better granularity

{Rest} Removed 'OpenR66InternalErrorException'

{Rest} Added server log export

{Rest} Added request signature checking

{Rest} Remade the error handling system

{Rest} Move permission check to individual methods

{Rest} Added all missing features

- Added SSL support
- Added CRUD support
- Simplified content type checking with method annotations
- Simplified privilege checking with method annotations

{Rest} Add ability to run v1 & v2 concurrently

{Rest} Add CRUD for each subpath of ServerHandler

{Rest} Fix query parameter type errors & JavaDoc

{Rest} ADd URI in header on object creation

{Rest} Fix code formatting

Fix getSequence

{Rest} Fix missing header in entry creation reply

{Rest} Added missing `close()` on DAO

{Rest} Fixed REST authentication

{Rest} Fixed request signature checking

{Rest} Better content type handling

Temp commit

{Rest} Optimized imports

{Rest} Added missing elements from API spec

{Rest} Fixed error when requesting server restart

{Rest} PUT method now updates instead of replacing

{Rest} Changed exceptions into unchecked exception

{Rest} More accurate JSON parsing

{Rest} Better XML parsing

{Rest} Simplified code with static imports
{Rest} Integrated DAO changes

{Rest} Replaced RestLimit with ObjectNode

{Rest} Replaced RestHost with ObjectNode

{Rest} Replaced RestHost with ObjectNode

{Rest} Replaced RestRule with ObjectNode

{Rest} Replaced ServerStatus with Monitoring

{Rest} Fixed transfer log export

{Rest} Removed deprecated elements

{Rest} Reorganize code
{Rest} Fixed OPTIONS request handling

{Rest} Add CORS support
…b,\n change auto-upgrade default value to false
Correct some requests on monitoring interface
Fix some packaging issues and revert some R66session suppression
@bcarlin bcarlin merged commit cda58ea into waarp:wNetty41 Jul 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants