Separate FoundationNetworking from Core Library

[kaishin]

Hi @Sherlouk. I previously opened issue #91 to pitch adding support for NIO and server-side Swift projects. It was subsequently closed since I couldn't dedicate time to address the questions raised nor to open source the NIO package I mentioned.

Since then, I managed to open source the NIO client implementation as meilisearch-nio and I have been using it for over 2 years now.

In the original pitch I brought attention to the amount of duplication between the two projects, especially the models and request payloads. I still think there is value in either merging or making the NIO client depend on this repository, but that may require some potentially breaking changes.

Ideally, this repo can host the base models, the URLSession client for Darwin, and the NIO client as three separate packages. That would provide a lot of flexibility while keeping complexity at a minimum.

I would love to hear your thoughts.

[Sherlouk]

Hey Reda.

Pedro asked a really valuable question in the initial pitch:

Is it possible to use this custom URLSession from the Config initializer

In fact, this is the exact approach I personally took in my meilisearch-vapor project. Just like your proposal, it utilises NIO and AHC via Vapor. It could be made more generic to support any NIO implementation (though it wasn't my use case and so didn't build it). It has full Linux support and I've been using in production for a couple months now.

Why does this approach not work for what you're trying to achieve?

[kaishin]

Hi Sherlouk,

Thank you for taking the time to reply.

Pedro asked a really valuable question in the initial pitch:

I initially didn't manage to look into Pedro's suggestion but looking at your vapor bridge I see the idea now. This approach does indeed solve the need to have separate clients for client-side apps and servers, especially if it's abstracted away like it is the case in your Vapor wrapper.

If you think this should be the recommended way to interface with MeiliSearch from server-side Swift, a mention of the Vapor repo in the README would be quite valuable instead of the current demo link.

Out of curiosity, is this kind of request bridging common in the server-side Swift ecosystem ? I don't recall coming across it before.

---

For the record, I will try to keep my NIO implementation as up-to-date as possible (though mostly on a need basis) as I still prefer using NIO types directly instead of bridging with Foundation, but I understand if the goals of this library is to cover all bases at the cost of introducing some indirection when used in the server.

Please feel free to close this issue!

[kaishin]

Also, if it's not too much to ask, I would love if you can link to my implementation in the README in case it's a better fit for some. I'll do the same on my side :)

[Sherlouk]

Out of curiosity, is this kind of request bridging common in the server-side Swift ecosystem ? I don't recall coming across it before.

Even the internal code within Vapor has to bridge between different types for the same thing - but I do agree that relying on Foundation types is not ideal especially when it requires conditional support for FoundationNetworking. The service pattern seen in my project is very common even within Vapor's first-party projects.

I think the idea of passing in a request handler is a sensible thing to continue supporting, maintaining two separate implementations of the project is (as you're noticing) no easy feat especially when usage is already so light and we all want to benefit from the core contributions here (whether that be async/await, or being up-to-date with the latest Meilisearch features, and many more improvements coming up).

I think there could be two actions off the back of this:

1. Separate the network handling from Foundation types. This will allow extensions (such as for NIO) to not need to rely on FoundationNetworking and to ease the bridging. At this point I think it makes sense for there to be a meilisearch-swift-nio implementation which acts as a lightweight networking extension with appropriate server-side Swift compatibility (adding little maintenance overhead and continuing to receive the benefits of this project).
2. With agreement from the Meilisearch team (@curquiza) we can add such extensions (not replacements) to the README.

If you're happy, we can rename this issue to cover off these tasks.

[curquiza]

Hello everyone 👋

With agreement from the Meilisearch team (@curquiza) we can add such extensions (not replacements) to the README.

Yes, I agree with this, as long as the linked package are not outdated and maintained. It can be a good fit for the users! I also recommend to add your package here, if not already done: https://github.com/meilisearch/awesome-meilisearch

If they become out-dated for too long, we will remove the mention of course.

[kaishin]

The service pattern seen in my project is very common even within Vapor's first-party projects.

Yes, that's a well established pattern indeed. What's not common, and I asked the Vapor core team about it to double check, is the bridging approach. I suspect it will keep working, and perhaps even better as foundation networking bugs on Linux are squashed. But for the time being it remains a novelty.

Separate the network handling from Foundation types.

I am all for doing this one. We still need to figure out if we want to do it in the same repo with two packages or separate repos. I don't personally have a preference; whatever is easier and comes with less overhead. We could name the types library meilisearch-swift-core, the foundation networking implementation in this repository meilisearch-swift-foundation, and the NIO one meilisearch-swift-nio. It's not clear to me whether you meant the 3rd one can also be part of this repository, but for the record, I don't see why not if we use macros to only pull NIO as a dependency on Linux.

I am happy tackling most of the work needed here, as I need some features in 1.6 and would like to update the implementation to support those.

@Sherlouk what do you think of the above?

[Sherlouk]

even better as foundation networking bugs on Linux are squashed

The point above though is we would be removing Foundation Networking from the project, completing eliminating this concern.

I don't see why not if we use macros to only pull NIO as a dependency on Linux

I'd be happy with this compromise. My initial concern was simply around Xcode resolving dependencies unnecessarily, and especially at the cost which NIO brings.

We would then have three targets: MeiliSearch (for backwards compatibility and would contain URLSession with FoundationNetworking), MeiliSearchCore (with the bulk of the project, but with no FoundationNetworking), and MeiliSearchNIO (only on Linux, but with the necessary service for NIO projects). We'd update the Vapor example to use this.

I do think that MeiliSearch (the URLSession implementation) should continue to use conditional imports for FoundationNetworking for users who are happy with that, and again for backwards compatibility. You won't receive this in the NIO package.

[kaishin]

The above points sound good. We can go over the minutiae in the PR itself. Please feel free to assign me to this.

[kaishin]

Going over the changes necessary to bring in my NIO implementation, it's not going to be as straightforward as I hoped to, especially if backward compatibility is a priority. For one I modeled certain things like error codes in a stricter way. Also some names etc. that need to updated. As such I think it'd make more sense to do this gradually if I can upstream some of these pain points first (each in their own PR), then once things are close enough adding the NIO module is going to be much easier.

@Sherlouk How open are you to breaking changes? I see that the version is <1.0 so I assume there are no guarantees, but I can understand if you want to keep them at a minimum. In #437 I used one alias to avoid any breaking changes, so we can keep using that approach, but it will eventually be cumbersome if we want to raise the cadence of updates to support newer features.

[Sherlouk]

How open are you to breaking changes?

Handling them on a case-by-case basis at the moment, especially weighted based on the assumed usage of the API (a breaking change to the search API should be avoided as much as possible, but a change to error handling might be more acceptable). It's worth noting that main has a good number of breaking changes in already so now's probably the best time to make them so we can become stable more quickly.