andrebocchini / swiftynab

iOS/macOS/WatchOS/tvOS Swift framework for the YNAB API
MIT License
39 stars 9 forks source link

How is the lastKnowledgeOfServer handled? #14

Open TheFern2 opened 6 days ago

TheFern2 commented 6 days ago

I see a bunch of functions just like in the api that can use lastKnowledgeOfServer for delta requests, but I don't see any service functions that actually save or return lastKnowledgeOfServer. I am a little confused how should I get this value from this library so I can provide to the functions that can use it. Was this a design flaw on the library or I am missing something?

Also I would be happy to take over this repo or at least be added as a collaborator, it looks like perhaps you have moved on from this project which I totally get. I have been using my own local fork, there are many structs that do not have init, and you can't make proper mocks when working on things offline with mock data.

TheFern2 commented 6 days ago

For example:

public func getBudget(
    budgetId: String,
    lastKnowledgeOfServer: Int? = nil
) async throws -> BudgetDetail

Source

Should probably return a server_knowledge as well like the api:

{
  "data": {
    "server_knowledge": 100,
    "budget": {
      "id": "16da87a0-66c7-442f-8216-a3daf9cb82a8",
      ...
    }
  }
}

Source

Probably the easiest thing to do thinking aloud, would be to have a bool property in BudgetService, and when creating a YNAB instance take that bool and track server knowledge internally, so a library user doesn't have to track that server knowledge at all, but if they do for some reason then those api calls that do return it should probably still return it to the user just in case.

TheFern2 commented 6 days ago

Ok, it looks like functionality is there for some response models, but nothing is being done with it.

Currently you have:

struct AccountsResponse: Codable {
    let accounts: [Account]
    let serverKnowledge: Int
}

but the service doesn't return it:

public func getAccounts(
        budgetId: String,
        lastKnowledgeOfServer: Int? = nil
    ) async throws -> [Account] {
        let request = AccountsRequest(
            budgetId: budgetId,
            lastKnowledgeOfServer: lastKnowledgeOfServer
        )
        let response: AccountsResponse = try await client.request(request)
        return response.accounts

but it should be like this, so the end user has the server knowledge:

public func getAccounts(
        budgetId: String,
        lastKnowledgeOfServer: Int? = nil
    ) async throws -> ([Account], Int) {
        let request = AccountsRequest(
            budgetId: budgetId,
            lastKnowledgeOfServer: lastKnowledgeOfServer
        )
        let response: AccountsResponse = try await client.request(request)
        return (response.accounts, response.serverKnowledge)

Am I reading this correctly?

TheFern2 commented 6 days ago

Working on this on this branch, I added a new Struct ServerKnowledge for clarity even though is just an int. https://github.com/TheFern2/swiftynab/blob/delta-responses/SwiftYNAB/SwiftYNAB/services/BudgetService.swift#L40

public func getBudget(
        budgetId: String,
        lastKnowledgeOfServer: Int? = nil
    ) async throws -> (BudgetDetail, ServerKnowledge) {
        let request = BudgetDetailRequest(
            budgetId: budgetId,
            lastKnowledgeOfServer: lastKnowledgeOfServer
        )
        let response: BudgetDetailResponse = try await client.request(request)
        return (response.budget, ServerKnowledge(value: response.serverKnowledge))
    }

And the usage would be like this:

let (budget, serverKnowledge) = try await ynab.budgets.getBudget(
budgetId: budgetId, lastKnowledgeOfServer: serverKnowledge)

print("Budget Name: \(budget.name)")
print("Server Knowledge: \(serverKnowledge.value)")