mattpolzin / JSONAPI

Swift Codable JSON:API framework
MIT License
75 stars 19 forks source link

Allow for easier calls to included data #109

Open reshadf opened 1 year ago

reshadf commented 1 year ago

Hi Matt,

We are using this library intensively. However, one thing that is very cumbersome somehow, is working with relationships and included object.

We are looking for way to expose the relationships included data easier. Something like this

let objectDocument = Document(..)
print(objectDocument.name) // attribute
print(objectDocument.relationships.employee.id) // which already works
print(objectDocument.includedRelation(id: objectDocument.relationships.employee.id).name // which doesn't exist but would be nice to have

And maybe something like this for the entire array?

print(objectDocument.includedRelation(relation: objectDocument.relationships.employee) // would return [Employee]

I have seen the examples of all the caches etc but it results in a lot of boilerplate code and messy code imo. Is something like above possible or is this some limitations we have in Swift currently? I would also love to help with a PR myself if you could give some advice on how to approach this :)

Love to hear your thoughts!

mattpolzin commented 1 year ago

I have seen the examples of all the caches etc

I am not sure if you mean you’ve already looked into https://github.com/mattpolzin/JSONAPI-ResourceStorage but if not then that’s the direction I would point you. Although that isn’t as polished of a library as my main JSONAPI library, it does show two alternative approaches to storing and retrieving information found in JSONAPI documents.

The reason I don’t plan to integrate a solution into this main JSONAPI library is that I can see benefits to totally different approaches and I don’t want this library to make that decision for the end user. Personally, I like leaning into values and avoiding classes so I’ve mostly used the approach that the above-linked library calls JSONAPIResourceCache. However, what the above-linked library calls JSONAPIResourceStore allows for the type of code you mentioned above.

So it isn’t so much about what Swift allows as it is about how Swift makes it possible to lean heavily on value types if you want to, but value types don’t allow for the type of code you wanted to write above.

The nice thing about the extra library that implements the two strategies is that it really doesn’t contain many lines of code — feel free to use the library if it works as is, or use the ideas, or rip the code out of it and modify it to be exactly what your ideal solution would be.

Let me know if you have more questions or if I’ve not quite answered this one!

reshadf commented 4 months ago

Hi Matt,

Thanks for your response.

I tried the library and it works fine for most parts. However, I tried a larger include (Include13) which is not supported. So I added extension for 12 and 13. But now it complains about the following but I am not sure which library here is at fault or what I missed..

Referencing instance method 'resourceCache()' on 'EncodableJSONAPIDocument' requires the types 'Poly13<...>` and Poly0 be equivalent. Not sure where to go from here. Could you perhaps put me in the right direction?

Thank you so much.

mattpolzin commented 4 months ago

I suspect the problem is in your resource cache code or possibly a bug in that library.

Can you share your ResourceCache type and Materializable conformances? Or at least the Materializable conformances for the primary resource and the 13th include type in the case where your above error is thrown?

reshadf commented 4 months ago

Well, from the docs I assumed I only needed to cache all the includes and not the primaryResource.. I now added it.

struct EntryCache: Equatable, ResourceCache {
    var entries: ResourceHash<EntryResource> = [:] // primary resource
    var types: ResourceHash<TypeResource> = [:] // include

    mutating func merge(_ other: EntryCache) {
        entries.merge(other.entries, uniquingKeysWith: { $1 })
        types.merge(other.types, uniquingKeysWith: { $1 })
    }
}

extension TypeDescription: Materializable {
    static var cachePath: WritableKeyPath<EntryCache, ResourceHash<TypeResource>> { \.types }
}

extension EntryDescription: Materializable {
    static var cachePath: WritableKeyPath<EntryCache, ResourceHash<EntryResource>> { \.entries }
}

And this is the Include13

extension Include13: CacheableResource where
    A: CacheableResource,
    B: CacheableResource,
    C: CacheableResource,
    D: CacheableResource,
    E: CacheableResource,
    F: CacheableResource,
    G: CacheableResource,
    H: CacheableResource,
    I: CacheableResource,
    J: CacheableResource,
    K: CacheableResource,
    L: CacheableResource,
    M: CacheableResource,
    A.Cache == B.Cache,
    B.Cache == C.Cache,
    C.Cache == D.Cache,
    D.Cache == E.Cache,
    E.Cache == F.Cache,
    F.Cache == G.Cache,
    G.Cache == H.Cache,
    H.Cache == I.Cache,
    I.Cache == J.Cache,
    J.Cache == K.Cache,
    K.Cache == L.Cache,
    L.Cache == M.Cache {

    public func cache(in cache: inout A.Cache) {
        switch self {
        case .a(let resource):
            resource.cache(in: &cache)
        case .b(let resource):
            resource.cache(in: &cache)
        case .c(let resource):
            resource.cache(in: &cache)
        case .d(let resource):
            resource.cache(in: &cache)
        case .e(let resource):
            resource.cache(in: &cache)
        case .f(let resource):
            resource.cache(in: &cache)
        case .g(let resource):
            resource.cache(in: &cache)
        case .h(let resource):
            resource.cache(in: &cache)
        case .i(let resource):
            resource.cache(in: &cache)
        case .j(let resource):
            resource.cache(in: &cache)
        case .k(let resource):
            resource.cache(in: &cache)
        case .l(let resource):
            resource.cache(in: &cache)
        case .m(let resource):
            resource.cache(in: &cache)
        }
    }
}

If I dive into the function resourceCache I see that it goes to

extension EncodableJSONAPIDocument where
    BodyData.PrimaryResourceBody: ManyResourceBodyProtocol,
    BodyData.PrimaryResourceBody.PrimaryResource: CacheableResource,
    BodyData.IncludeType == NoIncludes {
    public func resourceCache() -> BodyData.PrimaryResourceBody.PrimaryResource.Cache? {
        guard let bodyData = body.data else {
            return nil
        }

        var cache = BodyData.PrimaryResourceBody.PrimaryResource.Cache()

        for resource in bodyData.primary.values {
            resource.cache(in: &cache)
        }

        return cache
    }
}

While I think that it should go to the one with includes?

mattpolzin commented 4 months ago

I think that it should go to the one with includes?

Yeah, that's a good lead. I tried to throw together the simplest possible reproduction of your situation I could to see where Poly0 (also known as NoIncludes) and Poly13 (a.k.a Include13) were getting mixed up. I didn't hit your error yet, but I wonder if there's a meaningful difference in the Document type I declared and yours?

typealias Doc = JSONAPI.Document<ManyResourceBody<EntryResource>, NoMetadata, NoLinks, Include13<TypeResource,TypeResource,TypeResource,TypeResource,TypeResource,TypeResource,TypeResource,TypeResource,TypeResource,TypeResource,TypeResource,TypeResource,TypeResource>, NoAPIDescription, UnknownJSONAPIError>

It's nonsensical to make every type of include the same, but I just wanted to reproduce the compiler error you were seeing.

reshadf commented 4 months ago

this is how it looks for me

typealias EntryResourceResponse = JSONAPI.Document<ManyResourceBody<EntryResource>, SCMetadata, NoLinks, Include13<BalanceAccount, CostCenter, MonetaryAccount, CardResource, Dimension1, Dimension2, Dimension3, CostType, VatType, BookResource, ClientResource, CostUnit, TypeResource>, NoAPIDescription, BasicJSONAPIError<String>>
mattpolzin commented 4 months ago

Nothing stands out as obviously problematic so instead of just asking for even more of your source code, how about:

  1. What version of Swift are you running?
  2. Do you mind seeing if the following gist encounters compilation errors in your current project (it doesn't on my machine)? https://gist.github.com/mattpolzin/49373b24a58b545d4ade5b0f6225212e
reshadf commented 4 months ago

Hi Matt, I debugged for quite a bit until I noticed that I did not have everything implementing Materializable. 😅 after fixing that it seems to work better.

mattpolzin commented 4 months ago

Oh, gotcha. I wish the error were clearer but that does make sense.