software-mansion / starknet.swift

Starknet SDK for Swift language
MIT License
24 stars 15 forks source link

`StarknetRequest` introduced in `v0.11.0` should be a protocol not a struct to enable mocking #212

Closed mluisbrown closed 1 month ago

mluisbrown commented 3 months ago

What happened

With the changes in v0.11.0, StarknetProviderProtocol now returns StarknetRequest<U>, on which in turn you have to call the async method send() on StarknetRequest to get the result.

However, since StarknetRequest has no public initialiser it's impossible for any type outside of this library to conform to StarknetProviderProtocol since in order to conform to it you need to return instances of StarknetRequest, which cannot be constructed since it has no public initialisers.

However, even adding a public init would not be terribly helpful, although IMO it should be made available as a matter of correctness.

If we want to create a mock implementation of StarknetProviderProtocol for testing, we currently cannot do so, due to the above limitation. What would solve this is if we had a StarknetRequestProtocol as generic a protocol with a single method:

public protocol StarknetRequestProtocol<U> {
    associatedtype U: Decodable
    func send() async throws -> U
}

///  this would be mostly like the current `StarknetRequest` type
/// I just implemented it with a `value` so I could check this would compile
public struct StarknetRequest<T: Decodable>: StarknetRequestProtocol {
    let value: T
    init(value: T) {
        self.value = value
    }

    public func send() async throws -> T {
        return value
    }
}

/// return types of StarknetProviderProtocol would need to be changed to `any StarknetRequestProtocol<ReturnType>`
public protocol StarknetProviderProtocol {
    func getSpecVersion() -> any StarknetRequestProtocol<String>
}

/// same for the implementation
public class StarknetProvider: StarknetProviderProtocol {
    func getSpecVersion() -> any FooRequestProtocol<String> {
        return StarknetRequest<String>(value: "")
    }
}

This approach would allow creating a mock implementation of StarknetProviderProtocol:

public struct MockRequest<T: Decodable>: StarknetRequestProtocol {
    public func send() async throws -> T {
        throw StarknetProviderError.emptyBatchRequestError // or whatever
    }
}

class MockStarknetProvider: StarknetProviderProtocol {
    func getSpecVersion() -> any StarknetRequestProtocol<String> {
        return MockRequest()
    }
}

Until this is resolved we cannot migrate to v0.11.1 as we won't be able to adapt our tests to the new StarknetProviderProtocol methods.

Stack trace

N/A

Steps to reproduce

N/A

SDK Version

0.11.1

Language version

5.10

Is there an existing issue for this?

franciszekjob commented 3 months ago

Hey @mluisbrownhave, have you tried implementing your solution with a similar protocol for batch requests (StarknetBatchRequestProtocol) and using MockProvider with MockBatchRequest?

It seems that using a parameterized protocol in the batchRequests method causes a segfault on build without any useful logs that allow to debug this.

mluisbrown commented 3 months ago

Hi @franciszekjob, I didn't actually implement it, although the code I wrote above did compile.

franciszekjob commented 1 month ago

Closing this as it's no longer relevant due to https://github.com/software-mansion/starknet.swift/issues/215 (which has been resolved).