-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
Peer Review #13
base: review
Are you sure you want to change the base?
Peer Review #13
Changes from all commits
2a976f6
705b342
f26ed86
29c24a2
caccaed
ed2a14f
9c126ee
ab509da
df67474
ad67fdf
b0d3d3a
b8f3f98
4696641
71d0205
bd9d1a3
31ea4b8
ba9a677
ef7a259
c609200
55b931b
ee0c29f
f31815c
56a6469
fff2ddf
cba612c
23c8815
ddd105f
0d82308
43755ec
a181e77
2961008
280b9dd
6f62ded
1ebd1bd
5aab1cc
959fa13
18e180b
b994c5e
6fa29e7
b5b522b
ce2b7a2
b9e09ce
01b020c
98cd8af
cddd70e
94d74cb
97f8d63
c69e322
3df87b5
ddf6f3a
16a7aaf
8112c34
e7035e7
3566600
98e46b4
8f7c488
67baaef
ccd4e48
43b68ac
7a2d9c1
2e7d31c
9fb2526
9889cc1
d492a16
471aa62
f915d24
5478a6c
c1d1585
8b9e92f
af88185
f2d294a
47cfe01
193343f
ef3bfd5
feaa0b1
64c048d
dad6567
74798fa
40fc0c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
# Don't track these files | ||
*.pb.h | ||
*.pb.cc | ||
*.pb.o | ||
*.o | ||
backendStore | ||
backend_store | ||
service_layer | ||
client_layer | ||
key_value_store_service_impl_tests | ||
service_layer_service_impl_tests | ||
.DS_Store |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,56 @@ | ||
# 499-jkhoo | ||
# Jillian Khoo - [email protected] | ||
|
||
## Installation | ||
### Initial Setup: | ||
#### Download vagrant here: | ||
https://www.vagrantup.com/downloads.html | ||
#### Follow these instructions for Project Setup for your vagrant vm: | ||
https://www.vagrantup.com/intro/getting-started/project_setup.html | ||
#### Install the box: | ||
Use this box: https://app.vagrantup.com/generic/boxes/ubuntu1804 | ||
vagrant box add generic/ubuntu1804 | ||
Open the Vagrantfile and change its contents to this: | ||
`Vagrant.configure("2") do |config|` | ||
`config.vm.box = "hashicorp/precise64"` | ||
`config.vm.synced_folder ".", "/vagrant"` | ||
`config.vm.provision :shell, path: "bootstrap.sh" ` | ||
`end` | ||
#### Create the Shell Script bootstrap.sh | ||
Create the following shell script and save it as bootstrap.sh in the same directory as your Vagrantfile | ||
`apt-get update` | ||
`apt-get install -y apache2` | ||
`if ! [ -L /var/www ]; then` | ||
`rm -rf /var/www` | ||
`ln -fs /vagrant /var/www` | ||
`fi` | ||
#### Run `vagrant up` then `vagrant ssh` | ||
#### Install grpc and protobuf | ||
Use these instructions to install grpc and protobuf: https://github.com/grpc/grpc/blob/master/BUILDING.md | ||
Go to grpc/third_party/protobuf in the grpc repository and run `./autogen.sh`, `./configure`, `make && sudo make install` | ||
Go to the root directory of the grpc repository and run `sudo make install` | ||
#### Install gtest | ||
`sudo apt-get install libgtest-dev` | ||
`sudo apt-get install cmake | ||
cd /usr/src/gtest | ||
sudo cmake CMakeLists.txt | ||
sudo make | ||
sudo cp *.a /usr/lib` | ||
|
||
## Compiling and running the code: | ||
Go to 499-jkhoo/cpp and run `./compile_clean` to compile everything except tests | ||
Start the Key Value store with `./backend_store` | ||
Start the Service Layer with `./service_layer` | ||
Run the client layer with `./client_layer` and any appropriate flags | ||
#### Example runs for the client layer: | ||
`./client_layer --register <username>`registers the given user | ||
`./client_layer --user <username>` logs in the given user and the --user flag must be before any command other than register | ||
`./client_layer --user <username> --chirp <text>` creates a new chirp by the user with the given text | ||
`./client_layer --user <username> --chirp <text> --reply <parent_id>` creates a new chirp by the user with the given text as a reply to the chirp with the given parent_id | ||
`./client_layer --user <username> --follow <to_follow>` username starts following to_follow | ||
`./client_layer --user <username> --read <chirp_id>` reads the chirp thread starting at the given id | ||
`./client_layer --user <username> --monitor` streams new chirps from those currently followed | ||
|
||
## Compiling and running the tests: | ||
Go to 499-jkhoo/tests and run `./test_compile_clean` | ||
Run the key value store tests with `./key_value_store_service_impl_tests` | ||
Run the service layer tests with `./service_layer_service_impl_tests` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
#include "key_value_store_service_impl.h" | ||
|
||
// Listens on port 50000 and assembles the key value server | ||
void RunServer() { | ||
std::string server_address("0.0.0.0:50000"); | ||
KeyValueStoreServiceImpl service; | ||
|
||
grpc::ServerBuilder builder; | ||
builder.AddListeningPort(server_address, grpc::InsecureServerCredentials()); | ||
builder.RegisterService(&service); | ||
|
||
std::unique_ptr<grpc::Server> server(builder.BuildAndStart()); | ||
std::cout << "Server listening on " << server_address << std::endl; | ||
server->Wait(); | ||
} | ||
|
||
int main(int argc, char** argv) { | ||
RunServer(); | ||
return 0; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
#include "client.h" | ||
|
||
bool Client::registeruser(std::string username, std::unique_ptr<chirp::ServiceLayer::Stub>& stub_) { | ||
if (username.length() == 0) { | ||
std::cout << "Must enter non empty username" << std::endl; | ||
return false; | ||
} | ||
grpc::ClientContext context; | ||
chirp::RegisterRequest request; | ||
request.set_username(username); | ||
chirp::RegisterReply reply; | ||
grpc::Status status = stub_->registeruser(&context, request, &reply); | ||
if (status.error_code() != 0) { | ||
std::cout << "Something went wrong :(" << std::endl; | ||
return false; | ||
} | ||
std::cout << "User " << username << " registered!" << std::endl; | ||
return true; | ||
} | ||
|
||
void Client::sendchirp(std::string username, std::string text, std::string reply_id, std::unique_ptr<chirp::ServiceLayer::Stub>& stub_) { | ||
grpc::ClientContext context; | ||
chirp::ChirpRequest request; | ||
chirp::ChirpReply reply; | ||
request.set_username(username); | ||
request.set_text(text); | ||
request.set_parent_id(reply_id); | ||
|
||
grpc::Status status = stub_->chirp(&context, request, &reply); | ||
if (status.error_code() != 0) { | ||
std::cout << "Something went wrong :(" << std::endl; | ||
if (reply_id.length() > 0) { | ||
std::cout << "Make sure the parent id actually exists" << std::endl; | ||
} | ||
} | ||
else { | ||
std::cout << "Chirp with chirp id " << reply.chirp().id() << " created" << std::endl; | ||
} | ||
} | ||
|
||
void Client::follow(std::string username, std::string to_follow, std::unique_ptr<chirp::ServiceLayer::Stub>& stub_) { | ||
if (to_follow.length() == 0) { | ||
std::cout << "Must enter non empty user to follow" << std::endl; | ||
} | ||
else { | ||
grpc::ClientContext context; | ||
chirp::FollowRequest request; | ||
chirp::FollowReply reply; | ||
request.set_username(username); | ||
request.set_to_follow(to_follow); | ||
grpc::Status status = stub_->follow(&context, request, &reply); | ||
if (status.error_code() != 0) { | ||
std::cout << "Something went wrong :(" << std::endl; | ||
} | ||
else { | ||
std::cout << "User " << username << " now following " << to_follow << std::endl; | ||
} | ||
} | ||
} | ||
|
||
void Client::read(std::string id, std::unique_ptr<chirp::ServiceLayer::Stub>& stub_) { | ||
grpc::ClientContext context; | ||
chirp::ReadRequest request; | ||
chirp::ReadReply reply; | ||
request.set_chirp_id(id); | ||
grpc::Status status = stub_->read(&context, request, &reply); | ||
if (status.error_code() != 0) { | ||
std::cout << "Something went wrong :(" << std::endl; | ||
} | ||
else { | ||
int num_chirps = reply.chirps_size(); | ||
for(int i = 0; i < num_chirps - 1; i++) { | ||
chirp::Chirp this_chirp = reply.chirps(i); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you could declare this_chirp outside of the loop |
||
std::cout << this_chirp.username() << ": " << this_chirp.text() << std::endl; | ||
} | ||
} | ||
} | ||
|
||
void Client::monitor(std::string username, std::unique_ptr<chirp::ServiceLayer::Stub>& stub_) { | ||
grpc::ClientContext context; | ||
chirp::MonitorRequest request; | ||
request.set_username(username); | ||
chirp::MonitorReply reply; | ||
std::unique_ptr<grpc::ClientReader<chirp::MonitorReply> > stream_handle (stub_->monitor(&context, request)); | ||
while (true) { | ||
while (stream_handle->Read(&reply)) { | ||
chirp::Chirp this_chirp = reply.chirp(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also here, chirp could be declared outside of loop |
||
std::cout<<this_chirp.username() << ": " << this_chirp.text() << std::endl; | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
#ifndef CPP_CLIENT_H_ | ||
#define CPP_CLIENT_H_ | ||
#include "service.grpc.pb.h" | ||
#include "service.pb.h" | ||
#include <iostream> | ||
|
||
// class behind the client layer | ||
class Client { | ||
public: | ||
// registers user, returns true if successful | ||
bool registeruser(std::string username, std::unique_ptr<chirp::ServiceLayer::Stub>& stub_); | ||
// sends a chirp with the given information | ||
void sendchirp(std::string username, std::string text, std::string reply_id, std::unique_ptr<chirp::ServiceLayer::Stub>& stub_); | ||
// adds to_follow to username's list of users following | ||
void follow(std::string username, std::string to_follow, std::unique_ptr<chirp::ServiceLayer::Stub>& stub_); | ||
// reads the chirp string starting at id | ||
void read(std::string id, std::unique_ptr<chirp::ServiceLayer::Stub>& stub_); | ||
// streams chirps from users username is following | ||
void monitor(std::string username, std::unique_ptr<chirp::ServiceLayer::Stub>& stub_); | ||
private: | ||
}; | ||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
#include <grpcpp/grpcpp.h> | ||
#include "service.grpc.pb.h" | ||
#include "service.pb.h" | ||
#include <iostream> | ||
#include <gflags/gflags.h> | ||
#include "client.h" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. im p sure that there should be a blank line between iostream and the rest, and iostream should go first |
||
|
||
DEFINE_string(register, "", "Register a username"); | ||
DEFINE_string(user, "", "Login a user"); | ||
DEFINE_string(chirp, "", "Send a chirp"); | ||
DEFINE_string(reply, "", "Make a chirp a reply to another chirp"); | ||
DEFINE_string(follow, "", "Follow a user"); | ||
DEFINE_string(read, "", "Read a chirp"); | ||
DEFINE_bool(monitor, false, "Stream chirps from followed users"); | ||
|
||
// parses command line flags and triggers appropriate action | ||
int main(int argc, char** argv) { | ||
gflags::ParseCommandLineFlags(&argc, &argv, true); | ||
std::shared_ptr<grpc::Channel> service_connection_ = grpc::CreateChannel("localhost:50002", grpc::InsecureChannelCredentials()); | ||
std::unique_ptr<chirp::ServiceLayer::Stub> stub_ = chirp::ServiceLayer::NewStub(service_connection_); | ||
std::string my_username=""; | ||
Client client; | ||
if (FLAGS_register != "") { | ||
client.registeruser(FLAGS_register, stub_); | ||
} | ||
if (FLAGS_user != "") { | ||
my_username = FLAGS_user; | ||
} | ||
if (FLAGS_chirp != "" && my_username != "") { | ||
client.sendchirp(my_username, FLAGS_chirp, FLAGS_reply, stub_); | ||
} | ||
if (FLAGS_follow != "" && my_username != "") { | ||
client.follow(my_username, FLAGS_follow, stub_); | ||
} | ||
if (FLAGS_read != "") { | ||
client.read(FLAGS_read, stub_); | ||
} | ||
if (FLAGS_monitor && my_username != "") { | ||
client.monitor(my_username, stub_); | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove blank line maybe? |
||
return 0; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
#!/bin/bash | ||
# Script that will make clean until I have a makefile | ||
|
||
rm *.o | ||
rm -rf service_layer | ||
rm -rf backend_store | ||
protoc -I ../protos --cpp_out=. ../protos/backendStore.proto | ||
|
||
g++ -std=c++11 `pkg-config --cflags protobuf grpc` -g -c -o backendStore.pb.o backendStore.pb.cc | ||
|
||
protoc -I ../protos --grpc_out=. --plugin=protoc-gen-grpc=`which grpc_cpp_plugin` ../protos/backendStore.proto | ||
|
||
g++ -std=c++11 `pkg-config --cflags protobuf grpc` -g -c -o backendStore.grpc.pb.o backendStore.grpc.pb.cc | ||
|
||
g++ -std=c++11 `pkg-config --cflags protobuf grpc` -g -c -o key_value_store_service_impl.o key_value_store_service_impl.cc | ||
|
||
g++ -std=c++11 `pkg-config --cflags protobuf grpc` -g -c -o key_value_client_interface.o key_value_client_interface.cc | ||
|
||
g++ -std=c++11 `pkg-config --cflags protobuf grpc` -g -c -o key_value_client.o key_value_client.cc | ||
|
||
g++ -std=c++11 `pkg-config --cflags protobuf grpc` -g -c -o key_value_store.o key_value_store.cc | ||
|
||
g++ -std=c++11 `pkg-config --cflags protobuf grpc` -g -c -o backend_store.o backend_store.cc | ||
|
||
g++ backendStore.pb.o backendStore.grpc.pb.o backend_store.o key_value_store_service_impl.o key_value_store.o key_value_client_interface.o key_value_client.o -g -L/usr/local/lib `pkg-config --libs protobuf grpc++ grpc` -Wl,--no-as-needed -lgrpc++_reflection -Wl,--as-needed -ldl -o backend_store | ||
|
||
protoc -I ../protos --cpp_out=. ../protos/service.proto | ||
|
||
g++ -std=c++11 `pkg-config --cflags protobuf grpc` -c -o service.pb.o service.pb.cc | ||
|
||
protoc -I ../protos --grpc_out=. --plugin=protoc-gen-grpc=`which grpc_cpp_plugin` ../protos/service.proto | ||
|
||
g++ -std=c++11 `pkg-config --cflags protobuf grpc` -g -c -o key_value_client.o key_value_client.cc | ||
|
||
g++ -std=c++11 `pkg-config --cflags protobuf grpc` -g -c -o key_value_client_interface.o key_value_client_interface.cc | ||
|
||
g++ -std=c++11 `pkg-config --cflags protobuf grpc` -g -c -o service.grpc.pb.o service.grpc.pb.cc | ||
|
||
g++ -std=c++11 `pkg-config --cflags protobuf grpc` -g -c -o service.o service.cc | ||
|
||
g++ -std=c++11 `pkg-config --cflags protobuf grpc` -g -c -o service_layer_service_impl.o service_layer_service_impl.cc | ||
|
||
g++ -std=c++11 `pkg-config --cflags protobuf grpc` -g -c -o service_layer.o service_layer.cc | ||
|
||
g++ backendStore.pb.o backendStore.grpc.pb.o service.pb.o service.grpc.pb.o service_layer.o key_value_client.o service_layer_service_impl.o service.o key_value_client_interface.o -g -L/usr/local/lib `pkg-config --libs protobuf grpc++ grpc` -Wl,--no-as-needed -lgrpc++_reflection -Wl,--as-needed -ldl -o service_layer | ||
|
||
g++ -std=c++11 `pkg-config --cflags protobuf grpc` -g -c -o client.o client.cc | ||
|
||
g++ backendStore.pb.o backendStore.grpc.pb.o service.pb.o service.grpc.pb.o client_layer.cc key_value_client.o service_layer_service_impl.o service.o key_value_client_interface.o client.o -g -L/usr/local/lib `pkg-config --libs protobuf grpc++ grpc` -lgflags -Wl,--no-as-needed -lgrpc++_reflection -Wl,--as-needed -ldl -o client_layer |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
#include "key_value_client.h" | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove blank line? |
||
void KeyValueClient::put(const std::string& key, const std::string& value) { | ||
chirp::PutRequest request; | ||
request.set_key(key); | ||
request.set_value(value); | ||
chirp::PutReply reply; | ||
grpc::ClientContext context; | ||
grpc::Status status = stub_->put(&context, request, &reply); | ||
} | ||
|
||
std::deque<std::string> KeyValueClient::get(const std::string& key) { | ||
chirp::GetRequest request; | ||
request.set_key(key); | ||
chirp::GetReply reply; | ||
grpc::ClientContext context; | ||
|
||
std::unique_ptr<grpc::ClientReaderWriter<chirp::GetRequest, chirp::GetReply> > stream_handle (stub_->get(&context)); | ||
stream_handle->Write(request); | ||
|
||
std::deque<std::string> returnValues; | ||
while (stream_handle->Read(&reply)) { | ||
returnValues.push_back(reply.value()); | ||
} | ||
|
||
return returnValues; | ||
} | ||
|
||
void KeyValueClient::deletekey(const std::string& key) { | ||
chirp::DeleteRequest request; | ||
request.set_key(key); | ||
chirp::DeleteReply reply; | ||
grpc::ClientContext context; | ||
grpc::Status status = stub_->deletekey(&context, request, &reply); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
#ifndef CPP_KEY_VALUE_CLIENT_H_ | ||
#define CPP_KEY_VALUE_CLIENT_H_ | ||
#include <deque> | ||
#include <string> | ||
|
||
#include <grpcpp/grpcpp.h> | ||
#include "backendStore.grpc.pb.h" | ||
#include "key_value_client_interface.h" | ||
|
||
// data structure behind the KeyValueStoreServiceImpl | ||
class KeyValueClient : public KeyValueClientInterface { | ||
public: | ||
virtual ~KeyValueClient(){}; | ||
// puts key value pair in store_, appending value if there is already a value associated with key | ||
void put(const std::string& key, const std::string& value); | ||
// gets values associated with the key | ||
std::deque<std::string> get(const std::string& key); | ||
// deletes values associated with the key | ||
void deletekey(const std::string& key); | ||
|
||
private: | ||
// models connection to an endpoint | ||
std::shared_ptr<grpc::Channel> channel_ = grpc::CreateChannel("localhost:50000", grpc::InsecureChannelCredentials()); | ||
// stub to communicate with KeyValueStore | ||
std::unique_ptr<chirp::KeyValueStore::Stub> stub_ = chirp::KeyValueStore::NewStub(channel_); | ||
}; | ||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
#include "key_value_client_interface.h" | ||
KeyValueClientInterface::~KeyValueClientInterface(){}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
#ifndef CPP_KEY_VALUE_CLIENT_INTERFACE_H_ | ||
#define CPP_KEY_VALUE_CLIENT_INTERFACE_H_ | ||
#include <deque> | ||
#include <string> | ||
|
||
// Interface for KeyValueStore interactions from the Service Layer | ||
class KeyValueClientInterface { | ||
public: | ||
virtual ~KeyValueClientInterface() = 0; | ||
// puts key value pair in store_, appending value if there is already a value associated with key | ||
virtual void put(const std::string& key, const std::string& value) = 0; | ||
// gets values associated with the key, return deque is not guaranteed to be unmodified | ||
virtual std::deque<std::string> get(const std::string& key) = 0; | ||
// deletes values associated with the key | ||
virtual void deletekey(const std::string& key) = 0; | ||
}; | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the google style guide, they normally put elses on the same line as the closing bracket for the if
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should fix this throughout the codebase.