fix: Support different query parameter names in search

[rluetzner]

This fixes an issue that easily reproducible with KOReader, but might affect other OPDS clients too.

Until now, we expected the client to define a searchTerms query parameter. However, according to the OPDS spec at

https://specs.opds.io/opds-1.2#3-search

the query parameter should actually be called q instead. Even more confusing is, that KOReader uses query.

For backwards compatibility reasons we keep searchTerms and add the other two options as well. All parameters are optional, so we need to check which one we got.

There is one small caveat to this implementation. When, for example, I run a search without search terms against Project Gutenberg's OPDS endpoint, I get back the entire catalog.

However, because we cannot set the default value of the optional parameters to null, we cannot determine if any or none of the options were used.

I ultimately decided to return a 400 Bad Request instead, in case the searchTerms ends up empty. This will ensure that we will detect issues with other OPDS clients in the future, in case there are implementations out there that use yet another parameter name for the search terms.

If we instead processed the request as empty by default, a client that uses a different query parameter name would just have the entire catalog returned. Users would be confused why their filters did not work and the underlying issue might be hard to track down.

This fixes #40.

[rluetzner]

I tested this by building a new DLL and replacing the plugin on my productive Jellyfin instance. The fix works as expected and I was able to search for books by their title. :slightly_smiling_face:

According to the OPDS specs, there are additional query parameters to, e.g. search for authors instead (or in addition to a title). This would be easy to add and might be something for the future. Sadly, at least KOReader does not support searching for an author directly, so I don't have an OPDS client at hand to test such a change anyway.

[crobibero]

Huh I was pretty sure that I originally tested with KOReader 🤷

Here is a patch that sets the default parameter to null

Index: Jellyfin.Plugin.Opds/OpdsApi.cs
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/Jellyfin.Plugin.Opds/OpdsApi.cs b/Jellyfin.Plugin.Opds/OpdsApi.cs
--- a/Jellyfin.Plugin.Opds/OpdsApi.cs   (revision 3928bbc093bbda8d0912d4adb600ea1e56e6b1eb)
+++ b/Jellyfin.Plugin.Opds/OpdsApi.cs   (date 1715693482237)
@@ -215,14 +215,25 @@
     /// <summary>
     /// Gets the search result.
     /// </summary>
-    /// <param name="searchTerms">The search terms.</param>
+    /// <param name="searchTerms">The search terms as historically used by this plugin.</param>
+    /// <param name="q">The search terms as defined by the OPDS spec.</param>
+    /// <param name="query">The search terms as some OPDS clients provide them.</param>
     /// <returns>The search feed xml.</returns>
     [HttpGet("Search")]
     [ProducesResponseType(StatusCodes.Status200OK)]
-    public async Task<ActionResult> SearchBookFromQuery([FromQuery] string searchTerms)
+    public async Task<ActionResult> SearchBookFromQuery(
+        [FromQuery] string? searchTerms = null,
+        [FromQuery] string? q = null,
+        [FromQuery] string? query = null)
     {
         try
         {
+            searchTerms ??= q ?? query;
+            if (string.IsNullOrEmpty(searchTerms))
+            {
+                return BadRequest();
+            }
+
             var userId = await AuthorizeAsync().ConfigureAwait(false);
             var feeds = _opdsFeedProvider.SearchBooks(Request.PathBase, userId, searchTerms);
             return BuildOutput(feeds);

[rluetzner]

Nice. I'm not in reach of a PC right now, but I'll work in the patch later. That's way better. 🙂

[rluetzner]

Done. I also replaced

           if (string.IsNullOrEmpty(searchTerms))
           {
               return BadRequest();
           }

with

           if (searchTerms == null)
           {
               return BadRequest();
           }

instead, which is more correct. If one of the query parameters was given as an empty string, we can just return the entire catalog, which lines up with how e.g. Project Gutenberg behaves when running a search without a filter.

Thanks for teaching me searchTerms ??= q by the way. I hadn't seen that before. :slightly_smiling_face: