Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Complete implementation of HTTPService #295

Open
wants to merge 5 commits into
base: development
Choose a base branch
from
Open

Complete implementation of HTTPService #295

wants to merge 5 commits into from

Conversation

arcadien
Copy link

@arcadien arcadien commented Aug 9, 2018

  • Enhance implementation of GET and POST method in HttpService
  • Implement management of file upload using POST/multipart, following the Servlet(c) API
  • Allow to provide options to CivetWeb when starting a ServletContainer
  • Allow to run GTest in a HttpService-enabled environment
  • Implement unit/functional tests for HttpService

@arcadien arcadien force-pushed the 287-implement-http-post-multipart branch 10 times, most recently from 9e21212 to 3da36b9 Compare August 14, 2018 09:07
@arcadien
Copy link
Author

@saschazelzer it look we are not using the same clang-format version, since our 'format-source.sh' run has produced difference in source format. We are using clang-format 6.0.0.

@arcadien arcadien force-pushed the 287-implement-http-post-multipart branch 4 times, most recently from 4a51b75 to ce4846d Compare August 14, 2018 13:08
@saschazelzer
Copy link
Member

@arcadien This is strange, I am also using clang-format 6.0.0. Anyway, I don't care too much. You could just format the files you touched (or not format them at all) and we are going to re-format everything from time to time. This should not be a major source of pain. If you want help with the CI test errors, you could share what your current status is and we could try and help out.

BTW, many thanks for working on this, it is much appreciated.

@arcadien arcadien force-pushed the 287-implement-http-post-multipart branch from 2523bf7 to f98f9c9 Compare August 17, 2018 10:16
@codecov-io
Copy link

codecov-io commented Aug 17, 2018 •
edited

Codecov Report

Merging #295 into development will decrease coverage by 5.03%.
The diff coverage is 80.51%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #295      +/-   ##
===============================================
- Coverage        89.52%   84.49%   -5.04%     
===============================================
  Files               89      108      +19     
  Lines             6274     7416    +1142     
===============================================
+ Hits              5617     6266     +649     
- Misses             657     1150     +493
Impacted Files Coverage Δ
httpservice/src/ServletContext.cpp 45.45% <ø> (ø)
httpservice/src/ServletConfig.cpp 58.82% <ø> (ø)
...include/cppmicroservices/httpservice/HttpServlet.h 100% <ø> (ø)
framework/include/cppmicroservices/BundleContext.h 96.15% <ø> (ø) ⬆️
httpservice/src/HttpServlet.cpp 30.06% <0%> (ø)
...cppmicroservices/httpservice/IHttpServletRequest.h 100% <100%> (ø)
...de/cppmicroservices/httpservice/IHttpServletPart.h 100% <100%> (ø)
...ude/cppmicroservices/httpservice/IServletContext.h 100% <100%> (ø)
...ppmicroservices/httpservice/IHttpServletResponse.h 100% <100%> (ø)
httpservice/src/HttpServletPartPrivate.h 100% <100%> (ø)
... and 26 more

@arcadien
Copy link
Author

@saschazelzer we will remove untouched files where syntax has been broken to have a more consistent pull request. Also, we will improve the test coverage on some HttpService components so that the total coverage does not decrease.
We may need some help in debugging errors with Clang builds we don't really understand (and we haven't Clang in our own buildchain).

@arcadien arcadien force-pushed the 287-implement-http-post-multipart branch from f98f9c9 to 97268d8 Compare August 17, 2018 13:55
@arcadien arcadien force-pushed the 287-implement-http-post-multipart branch 2 times, most recently from 5166abb to 93cde3a Compare September 21, 2018 07:43
@arcadien arcadien force-pushed the 287-implement-http-post-multipart branch 5 times, most recently from 6b5efaf to 70d6357 Compare December 5, 2018 16:29
@arcadien
Copy link
Author

arcadien commented Dec 5, 2018

I will do my best for coverage enhancement here and keep the branch sync with your development tree.
It is quite hard to find time since it works for us as is.

@arcadien arcadien force-pushed the 287-implement-http-post-multipart branch 2 times, most recently from 89851ba to fc302dc Compare December 6, 2018 09:15
@arcadien arcadien force-pushed the 287-implement-http-post-multipart branch from fc302dc to d000885 Compare December 6, 2018 09:39
@arcadien arcadien force-pushed the 287-implement-http-post-multipart branch 9 times, most recently from f3e5427 to 69eb179 Compare May 24, 2019 15:16
Aurélien Labrosse added 4 commits October 2, 2019 09:21
Bump CivetWeb version to 1.11.0
1479528 
Signed-off-by: Aurélien Labrosse <aurelien@pollen-metrology.com>
Enhancement of HTTP GET, POST and PUT methods implementations
cb5920c 
* Implement POST HTTP method in HttpService (query params and file upload)
* Implement GET HTTP query parameters management following Servlet(c) API
* Implement PUT HTTP method
* Implement HttpRequest#GetBody() to allow raw content processing
* Implement management of file upload using POST/multipart, following the Servlet(c) API
* Allow to specialize (and hence override methods off) HttpServletRequest and HttpServletResponse
  to allow easy unit testing of Servlet implementations
* Allow to provide options to CivetWeb when starting a ServletContainer
* Implement unit/functional tests for HttpService
* Allow to run GTest in a HttpService-enabled environment
* Unit-test various HTTP requests

Signed-off-by: Mael Jannsen <mael.jannsen@pollen-metrology.com>
Signed-off-by: Galil Mori <galil.mori@pollen-metrology.com>
Signed-off-by: Lucas Freyssinet <lucas.freyssinet@pollen-metrology.com>
Signed-off-by: Thibault Friedrich <thibault.friedrich@pollen-metrology.com>
Signed-off-by: Aurélien Labrosse <aurelien@pollen-metrology.com>
feat(framework): Add an API layer on top of Context
d8b6a48 
* Light IContext is added to allow essentially mocking
  of the context. This allow to test modules using services
  without starting a full framework, injecting mock implementations
  in the context.
feat(httpservice): Add API layer on top of HttpService
7e12994 
* HttpService main classes has been reified to interface
  to allow them to be mocked. This allow to test servlets
  implementation without the need of a complete framework
  to be started.
@arcadien arcadien force-pushed the 287-implement-http-post-multipart branch from 69eb179 to 7e12994 Compare October 2, 2019 07:26
@arcadien arcadien changed the title Implementation and test of HTTP post within the HttpService Complete implementation of HTTPService Oct 2, 2019
@arcadien
Copy link
Author

Hello @saschazelzer , long time no activity there. If interest is still here I can upgrade the pull request.

  • Bump Civet to 1.15 (had to to it for Visual Studio pro 2022 compilation)
  • Code move from test utils to first class framework utils (temp file and folder creation)
  • Upgrade of HTTPService itself

Let me know!

@arcadien
Copy link
Author

referring to old discussion, i also switched on clang-format 15.0.6

@jeffdiclemente
Copy link
Member

Hello @saschazelzer , long time no activity there. If interest is still here I can upgrade the pull request.

  • Bump Civet to 1.15 (had to to it for Visual Studio pro 2022 compilation)
  • Code move from test utils to first class framework utils (temp file and folder creation)
  • Upgrade of HTTPService itself

Let me know!

Hi @arcadien , we would be happy to review any changes you have for this PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@arcadien @saschazelzer @codecov-io @jeffdiclemente