alexliesenfeld / httpmock

HTTP mocking library for Rust
https://httpmock.rs
MIT License
472 stars 42 forks source link

Inconvenient MockRef<'a> API #26

Closed gdesmott closed 3 years ago

gdesmott commented 3 years ago

Hi,

I'm trying to write a non-trivial mock server. To do so I'd need to store both the MockServer object and a bunch of MockRef<'a> in the same struct (the one representing my mock server). Unfortunately it's pretty hard to do as MockRef is itself keeping a ref on Server, introducing the well-known self-referential problem.

It is possible to workaround it with tricks such as rental or owning_ref but it greatly increase the code complexity. An easier approach would be to remove the server borrow in MockRef by wrapping it around a Rc or Arc. Would you be opened to such change?

alexliesenfeld commented 3 years ago

Hi,

the idea behind this was to model the fact that a mock object can never outlive the mock server it was created on. I feel that removing this constraint would have some negative impact on developer experience.

However, I think a workaround could be to expose the id attribute from the MockRef struct and also add a constructor function new. They could be be used this way:

struct MyMockRef { id: usize }
struct MyWrapper { server: MockServer, mocks: RefCell<Vec<MyMockRef>>}

#[test]
fn wrapper_test() {
    let w = MyWrapper { server: MockServer::start(), mocks: RefCell::new(vec![]) };

    let mock = w.server.mock(|when, then| {
        when.path("/test");
        then.status(200);
    });

    // store the mock id for later
    w.mocks.borrow_mut().push(MyMockRef{id: mock.id}); // accessing mock.id would be made possible

    // recreate the MockRef from the previously stored mock id
    let mock = MockRef::new(mock.id, &w.server); // MockRef::new would be added

    mock.assert();
}

This way you would be able to recreate the MockRef structure yourself. Would that be sufficient for you?

gdesmott commented 3 years ago

Thanks for your quick reply Alex.

Yes, such change would solve my problem as well. From an API pov I'm wondering if having MockServer::fetch_mock(&self, id: usize) -> Result<MockRef, _> wouldn't be cleaner?

alexliesenfeld commented 3 years ago

Because MockServer::fetch_mock wouldn't actually do anything with the MockServer itself but only create a new MockRef instance, I think it should go into the MockRef structure.

I have created an initial implementation for this in #28. The code that is interesting for you is in file src/api/mock.rs (see here). Comments are very welcome!

To avoid confusion, the new functionality is only accessible via the newly created MockRefExt trait. You can access the new functionality once you import it into your code.

There is an alpha release that you can use to check if it works for you.

gdesmott commented 3 years ago

I just tested the alpha release and it works great for my use cases. Thanks a lot @alexliesenfeld !

alexliesenfeld commented 3 years ago

Very welcome :-)

alexliesenfeld commented 3 years ago

Has been released in v0.5.4