Skip to content
This repository has been archived by the owner on Jun 12, 2018. It is now read-only.

Create common, polymorphic base class for HTTP/HTTPSServer #208

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ttislerdg
Copy link

Addresses Issue #168

Removed templating on ServerBase and inner classes.
Created virtual methods for those using sockets
Subclassed Response and Connection for HTTP/HTTPS

All code and functionality should be identical, just reorganized to make the base class possible

@eidheim
Copy link
Owner

eidheim commented Feb 14, 2018

Sorry for the late reply, but thank you for this.

I have not yet had time to study the changes, but hopefully I will get a look at it soon.

@eidheim
Copy link
Owner

eidheim commented Feb 16, 2018

I went a slightly different direction in https://github.com/eidheim/Simple-Web-Server/tree/base_class. Not sure what solution is best, but at least I know it is doable:)

@ttislerdg
Copy link
Author

Your method looks cleaner. I like that there is minimal duplicated code. That's something I hadn't given enough effort to avoiding.

So to make sure, I'd do something like this to create the server

SimpleWeb::ServerInterface* server = https ?
         new SimpleWeb::Server<SimpleWeb::HTTPS>(cert, key) :
         new SimpleWeb::Server<SimpleWeb::HTTP>();

and then access the methods like before

  server->default_resource["GET"] = [this](std::shared_ptr<SimpleWeb::ServerInterface::Response> response,
                                           std::shared_ptr<SimpleWeb::ServerInterface::Request> request) { }

@eidheim
Copy link
Owner

eidheim commented Feb 16, 2018

Yes, for instance:

#include "client_https.hpp"
#include "server_https.hpp"
#include <vector>

int main() {
  auto io_service = std::make_shared<SimpleWeb::asio::io_service>();
  std::vector<std::unique_ptr<SimpleWeb::ServerInterface>> servers;

  servers.emplace_back(new SimpleWeb::Server<SimpleWeb::HTTP>());
  servers.back()->io_service = io_service;
  servers.back()->config.port = 8080;
  servers.back()->default_resource["GET"] = [](std::shared_ptr<SimpleWeb::ServerInterface::Response> response,
                                               std::shared_ptr<SimpleWeb::ServerInterface::Request> /*request*/) {
    response->write("Hello World");
  };

  servers.emplace_back(new SimpleWeb::Server<SimpleWeb::HTTPS>("server.crt", "server.key"));
  servers.back()->io_service = io_service;
  servers.back()->config.port = 8081;
  servers.back()->default_resource["GET"] = servers[0]->default_resource["GET"];

  for(auto &server : servers)
    server->start();

  io_service->run();
}

Although I'll probably change the names ServerInterface and ServerBase. Have to have a look what is common to use in such cases.

@eidheim
Copy link
Owner

eidheim commented Feb 16, 2018

For the time being, I changed it so that one now can write:

#include "client_https.hpp"
#include "server_https.hpp"
#include <vector>

int main() {
  auto io_service = std::make_shared<SimpleWeb::asio::io_service>();
  std::vector<std::unique_ptr<SimpleWeb::ServerBase>> servers;

  servers.emplace_back(new SimpleWeb::Server<SimpleWeb::HTTP>());
  servers.back()->io_service = io_service;
  servers.back()->config.port = 8080;
  servers.back()->default_resource["GET"] = [](std::shared_ptr<SimpleWeb::Server<>::Response> response,
                                               std::shared_ptr<SimpleWeb::Server<>::Request> /*request*/) {
    response->write("Hello World");
  };

  servers.emplace_back(new SimpleWeb::Server<SimpleWeb::HTTPS>("server.crt", "server.key"));
  servers.back()->io_service = io_service;
  servers.back()->config.port = 8081;
  servers.back()->default_resource["GET"] = servers[0]->default_resource["GET"];

  for(auto &server : servers)
    server->start();

  io_service->run();
}

The class name ServerTemplate bothers me though.

@eidheim
Copy link
Owner

eidheim commented Feb 17, 2018

I managed to simplify https://github.com/eidheim/Simple-Web-Server/tree/base_class further by splitting the Response class instead of the Connection class.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants