Actix-web attempt to update actually deletes

mvolkmann · January 28, 2021, 11:04pm

Here is a simple actix-web REST services that should update a resource, but instead deletes it.

mvolkmann/rust-actix-web-demo/blob/master/src/main.rs#L58


    let id = Uuid::new_v4().to_string();
    let new_dog = json.into_inner();
    let dog = Dog {id: id.clone(), name: new_dog.name, breed: new_dog.breed};
    let mut lock = state.lock().unwrap();
    lock.dog_map.insert(id, dog.clone());
    Ok(HttpResponse::Ok().json(dog))
}
//#[put("/dog")]
async fn update_dog(json: web::Json<Dog>, state: web::Data<Mutex<AppState>>) -> Result<HttpResponse> {
    let dog = json.into_inner();
    let id = dog.id.clone();
    println!("updating dog with id {}", id);
    let mut lock = state.lock().unwrap();
    lock.dog_map.insert(id, dog.clone());
    Ok(HttpResponse::Ok().json(dog))
}
#[delete("/dog/{id}")]
async fn delete_dog(req: HttpRequest, state: web::Data<Mutex<AppState>>) -> Result<HttpResponse> {

Can you spot what I'm doing wrong?
The routes to get and create the resource work fine.
Do I have to explicitly put dog_map back into the state after modifying it?

mbrubeck · January 29, 2021, 12:20am

The problem is that this line:

        App::new()
            .data(Mutex::new(AppState { dog_map: dog_map.clone() }))

clones the HashMap each time a new thread is created, so you end up with n threads that each have their own separate sets of dogs. An update or creation on one thread will not be seen by the other threads.

Instead, you'll want to make sure that you have a single HashMap, and only clone pointers to it. You could do this by wrapping your state in Arc<T>, but this isn't necessary since Data<T> is already a wrapper for Arc<T>. Instead you can create a Data<Mutex<AppState>> outside the closure, clone it inside the closure, and pass it to app_data() instead of data():

    let data = web::Data::new(Mutex::new(AppState { dog_map }));

    HttpServer::new(move || {
        App::new()
            .app_data(data.clone()) // note: `app_data`, not `data`
            // ...
    }

For more details, see the Data and App::data docs.

mvolkmann · January 29, 2021, 1:55am

Thanks so much! Given that I haven't been able to find an example similar to this on the actix-web web site or though Google searches, I can't imagine how I would have figured this out on my own.

Maybe I'm mistaken, but it seems to me that what I'm trying to do (demonstrate basic CRUD operations on a single resource type) is one the most common things developers do with HTTP servers. I'm trying to learn about several of them and I've yet to find an example like this in the official docs of any of them. That's surprising to me. I'm looking at rocket, warp, tide, and actix-web.

cole-miller · January 29, 2021, 3:11am

Actix has a GitHub repo devoted to examples:

The examples/ directory in the crate root is often a good place to look too, e.g.

mbrubeck · January 29, 2021, 3:20am

My solution was actually copied from the example in the web::Data docs linked in my previous comment.

I think it's a bit unusual to write a web service doing CRUD operations on purely in-memory values with hand-rolled synchronization (like Mutex<HashMap<K, V>>). More often you would use some sort of database or file-backed storage, which handles these details for you.

The Actix examples include a CRUD app using postgres and one using sqlx.

mvolkmann · January 29, 2021, 12:32pm

I agree that implementing CRUD operations on a database is far more common that operating on in-memory values. But for learning purposes I find it very valuable to remove the complexity of working with a database and just focus on the mechanics of configuring REST APIs. When I'm learning frameworks like these I like to start by implementing this common pattern:

GET /name - gets all the resources with a given name
GET /name/{id} - gets a specific instance of the resource
POST /name - creates an instance of the resource using data in the request body
PUT /name/{id} - updates an instance of the resource using data in the request body
DELETE /name/{id} - deletes an instance of the resource

I've looked through all the examples I can find, including those liked by @cole-miller and haven't found a simple one for any of actix-web, rocket, tide, or warp. They quite possibly exist and I just haven't found them. But my point is that I think many new users of these frameworks would find it hugely valuable if such an example was present on their main websites rather than buried among 10's of examples is a GitHub directory.

I'll do my best to help by documenting these myself. I have a start (WIP) here: Mark Volkmann's blog See the "Receiving HTTP Requests" link.

cole-miller · January 29, 2021, 6:58pm

Fair enough -- sorry I was a bit flippant about that. I'm sure the actix-web/tide/etc. people are open to feedback about their documentation and how to better incorporate examples into it.

mvolkmann · January 29, 2021, 7:22pm

I didn't think you were being flippant. I really appreciate all the help this forum gives me!