-
Notifications
You must be signed in to change notification settings - Fork 14
Added global capture and capture enum #45
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
Conversation
bjones1
left a comment
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.
Would you also add tests to exercise these functions? I definitely want to see something to check that os-specific stuff implied in the UUID get/save routines works.
It would be nice if we can use SQLite for local testing, instead of requiring a Postgres server?
server/Cargo.toml
Outdated
| # the CodeChat Editor. If not, see | ||
| # [http://www.gnu.org/licenses/](http://www.gnu.org/licenses/). | ||
| # | ||
| # # `Cargo.toml` -- Rust build/package management config for the server |
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.
Revert this change.
server/Cargo.toml
Outdated
| name = "codechat-editor-server" | ||
| readme = "../README.md" | ||
| repository = "https://github.com/bjones1/CodeChat_Editor" | ||
| version = "0.1.5" |
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.
Revert this change.
server/Cargo.toml
Outdated
| # ## Dependencies | ||
| [dependencies] | ||
| actix = "0.13.1" | ||
| async-once-cell = "0.5.0" |
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.
Would you install cargo sort then run it? I think most of these changes are due to sorted vs. non-sorted dependencies.
server/src/capture.rs
Outdated
| use tokio_postgres::{Client, NoTls}; | ||
| use uuid::Uuid; | ||
|
|
||
| pub static GLOBAL_EVENT_CAPTURE: OnceCell<Arc<EventCapture>> = OnceCell::new(); |
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.
Remove pub -- I assume this is only used by the function below.
server/src/capture.rs
Outdated
| let new_uuid = Uuid::new_v4().to_string(); | ||
|
|
||
| // Ensure the parent directory exists | ||
| if let Some(parent) = path.parent() { |
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.
Wrong logic -- only runs if path.parent() is Some, not if it exists. The parent should always exist. I'm guessing you want
if !path.parent().expect("Config directory has no parent").exists()`.
server/src/webserver.rs
Outdated
| // ## Webserver core | ||
| #[actix_web::main] | ||
| pub async fn main(port: u16) -> std::io::Result<()> { | ||
| // Initialize EventCapture |
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.
Put this in run_server, since the old init code is there.
server/src/webserver.rs
Outdated
| pub async fn run_server(port: u16) -> std::io::Result<()> { | ||
| // Connect to the Capture Database | ||
| //let _event_capture = EventCapture::new("config.json").await?; | ||
| let _event_capture = EventCapture::new("config.json").await?; |
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.
Delete this, I think -- I assume the code above will replace this?
server/src/capture.rs
Outdated
| The `Event` struct represents an event to be stored in the database. | ||
| Fields: - `user_id`: The ID of the user associated with the event. - | ||
| Fields: - |
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.
Please check formatting in CodeChat -- it's messed up here and elsewhere.
| } */ | ||
|
|
||
| pub struct EventCapture { | ||
| db_client: Arc<Mutex<Client>>, |
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.
Perhaps make this a Arc<Mutex<Option<Client>>>? If the config file is found and the db connections, the logging happens. Otherwise, no logging happens but the code still runs. (Currently, it errors if without a connection/config file/etc.)
server/src/capture.rs
Outdated
|
|
||
| pub static GLOBAL_EVENT_CAPTURE: OnceCell<Arc<EventCapture>> = OnceCell::new(); | ||
|
|
||
| pub async fn get_event_capture() -> Arc<EventCapture> { |
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.
Nice find! I like this!
Add global capture access and also added a global enum EventType to constrain events. Added UUID creation for each user so capture events can be done by context. UUID is created and persisted to the filesystem and uniquely identifies each user accessing capture without using usernames or other personal identifiers.