Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for easier calls to included data #109

Open
reshadf opened this issue Feb 16, 2023 · 9 comments
Open

Allow for easier calls to included data #109

reshadf opened this issue Feb 16, 2023 · 9 comments

Comments

@reshadf
Copy link

reshadf commented Feb 16, 2023

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
Copy link
Owner

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
Copy link
Author

reshadf commented Feb 29, 2024

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
Copy link
Owner

mattpolzin commented Feb 29, 2024

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
Copy link
Author

reshadf commented Feb 29, 2024

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
Copy link
Owner

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
Copy link
Author

reshadf commented Feb 29, 2024

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
Copy link
Owner

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
Copy link
Author

reshadf commented Mar 5, 2024

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants