2
\$\begingroup\$

I'm trying to learn Rust, and have taken a stab at it by create a REST API, that stores the data in a local JSON file. Before I get deeper into things I'd like some feedback on if I'm doing things in a "Rusty" sort of way, or if there's a better approach to what I'm doing.

https://github.com/TomHart/books-api

main.rs

mod books;

use actix_web::{get, put, web, App, HttpResponse, HttpServer, Responder, http::header};
use std::fs;
use json;
use crate::books::structs::{Book, Books};

#[get("/books")]
async fn books_route() -> impl Responder {
    let books: Books = books::get();
    let string: String = serde_json::to_string(&books).expect("Err");

    HttpResponse::Ok()
        .insert_header(header::ContentType::json())
        .body(string)
}

#[get("/books/{book_id}")]
async fn index(path: web::Path<String>) -> impl Responder {
    let book_id = path.into_inner();

    let book_result = books::get_by_id(book_id);

    let book: Book = match book_result {
        Ok(book) => book,
        Err(_err) => return HttpResponse::NotFound().finish()
    };

    let string: String = serde_json::to_string(&book).expect("Err");

    HttpResponse::Ok()
        .insert_header(header::ContentType::json())
        .body(string)
}

#[put("/books")]
async fn add_book(form: web::Json<Book>) -> impl Responder {
    let books: Books = books::add_book(form.into_inner());
    let string: String = serde_json::to_string_pretty(&books).expect("Err");
    HttpResponse::Ok()
        .insert_header(header::ContentType::json())
        .body(string)
}


fn init_json() -> bool {
    let raw_json = fs::read_to_string("data.json").unwrap_or_else(|_error| {
        return create_json();
    });

    json::parse(&raw_json).unwrap_or_else(|_error| {
        return json::parse(&create_json()).unwrap();
    });

    return true;
}

fn create_json() -> String {
    fs::OpenOptions::new().write(true)
        .create_new(true)
        .open("data.json").expect("Couldn't create data.json");

    fs::write("data.json", "{\"books\": []}").expect("Unable to write to data.json");

    return fs::read_to_string("data.json").unwrap();
}

#[actix_web::main]
async fn main() -> std::io::Result<()> {
    if !init_json() {
        println!("Can't create data.json");
    }

    println!("Starting");
    HttpServer::new(|| {
        App::new()
            .service(books_route)
            .service(add_book)
            .service(index)
    })
        .bind(("127.0.0.1", 8080))?
        .run()
        .await
}

books.rs

pub mod structs;

use std::fs;
use uuid::Uuid;
use crate::books::structs::{Book, Books, BooksList};

pub fn get() -> Books {
    let raw_json = fs::read_to_string("data.json").expect("Failed reading data.json").to_owned();
    let books: BooksList = serde_json::from_str(&raw_json).unwrap();

    books.books
}

pub fn add_book(mut book: Book) -> Books {
    println!("Name: {}", book.name);

    if book.id.is_none() {
        book.id = Some(Uuid::new_v4().to_string());
    }

    let raw_json = fs::read_to_string("data.json").expect("Failed reading data.json").to_owned();
    let mut books: BooksList = serde_json::from_str(&raw_json).unwrap();

    let index_of_first_even_number = books.books.0.iter().position(|x| x.id == book.id);

    if index_of_first_even_number.is_none() {
        books.books.0.push(book);
    } else {
        books.books.0[index_of_first_even_number.unwrap()] = book;
    }

    let string: String = serde_json::to_string_pretty(&books).expect("Err");
    fs::write("data.json", string).expect("Unable to write to data.json");

    books.books
}

pub fn get_by_id(book_id: String) -> Result<Book, String>
{
    let raw_json = fs::read_to_string("data.json").expect("Failed reading data_.json").to_owned();
    let books: BooksList = serde_json::from_str(&raw_json).unwrap();

    let tmp: Vec<Book> = books.books.0;

    for book in tmp {
        let id = book.id.as_ref();
        if !id.is_none() && id.unwrap() == &book_id {
            return Ok(book);
        }
    }

    Err("Not found".to_string())
}

books/structs.rs

use chrono::{DateTime, Utc, serde::ts_seconds_option};
use serde::{Deserialize, Serialize};

#[derive(Serialize, Deserialize)]
pub struct BooksList {
    pub books: Books,
}

#[derive(Serialize, Deserialize)]
pub struct Books(pub Vec<Book>);

#[derive(Serialize, Deserialize)]
pub struct Book {
    pub name: String,
    pub id: Option<String>,
    #[serde(with = "ts_seconds_option")]
    pub started: Option<DateTime<Utc>>,
    #[serde(with = "ts_seconds_option")]
    pub finished: Option<DateTime<Utc>>,
}

impl std::fmt::Display for Book {
    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
        let started = match self.started {
            Some(date) => format!("started at {}", date.format("%Y-%m-%d %H:%M:%S").to_string()),
            None => "not started yet".to_string()
        };

        write!(f, "{}, {}", self.name, started)
    }
}

impl std::fmt::Display for Books {
    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
        self.0.iter().fold(Ok(()), |result, album| {
            result.and_then(|_| writeln!(f, "{}", album))
        })
    }
}
```
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

I'd suggest that you try holding the book data in memory, loading it when the program starts, and writing copies as it changes, but using the in memory copy for lookups. The issue is that you've avoided one of the tricky bits that can arise in web applications by not having any state shared between your different invocations. If you want to get a handle on Rust, you should probably do that.

You've got lots of trivial duplication, for example:

let raw_json = fs::read_to_string("data.json").expect("Failed reading data.json").to_owned();
let mut books: BooksList = serde_json::from_str(&raw_json).unwrap();

Instead of duplicating this, make a function and call it.

This bit is confusing, because it has nothing to do with even numbers.

let index_of_first_even_number = books.books.0.iter().position(|x| x.id == book.id);

Also, you should probably use a dictionary to store the books if you want to lookup and store by id.

let raw_json = fs::read_to_string("data.json").unwrap_or_else(|_error| {
    return create_json();
});

I would suggest checking the error, to make sure it really indicates not found and not some other error.

The Display trait is a generic display function, and it doesn't make sense to implement a specific case in there. So its weird that you've define a std::fmt::Display that just prints some of the information.

You don't need to create a file before you write to it:

fn create_json() -> String {
    fs::OpenOptions::new().write(true)
        .create_new(true)
        .open("data.json").expect("Couldn't create data.json");

    fs::write("data.json", "{\"books\": []}").expect("Unable to write to data.json");

    return fs::read_to_string("data.json").unwrap();
}

fs::write will create the file if it does not exist.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for your comments, I'll have a go at refactoring to take those things into account! \$\endgroup\$
    – TMH
    Commented Apr 1, 2023 at 21:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.