feat(server): Implements query API

Also includes a few bug and clippy fixes I found while testing

docs/protocol-spec.md

@@ -60,7 +60,7 @@ This section describes two modes for querying. An implementation of Bindle MUST
 
 If a service supports both strict and standard modes, then strict mode SHOULD only be applied when the `strict` parameter is set to `true`. In all other cases, the standard mode SHOULD be applied.
 
-Whether strict or standard, a query MUST NOT match a yanked bindle.
+Whether strict or standard, a query MUST NOT match a yanked bindle unless the `yanked` parameter is set to `true`.
 
 Whether strict or standard, a query MAY choose to treat an empty query string as a universal match, matching all non-yanked bindles.
 
@@ -132,25 +132,69 @@ An example Rust implementation of the above is the [`semver` crate](https://crat
 When a query is executed without error, the following structure MUST be used for responses. In this specification, the format is TOML. However, if the `ACCEPT` header indicates otherwise, implementations MAY select different encoding formats.
 
 ```toml
-query = "foo/bar/baz"
+query = "mybindle"
 strict = true
 offset = 0
-limit = 20
-timestamp = 1234567890
-total = 2
+limit = 50
+total = 1
 more = false
+yanked = false
 
-[[bindle]]
-name = "foo/bar/baz"
-version = "v0.1.0"
-authors = ["Matt Butcher <[email protected]>"]
-description = "My first bindle"
+[[invoices]]
+bindleVersion = "1.0.0"
 
-[[bindle]]
-name = "hello/foo/bar/baz/goodbye"
-version = "v8.1.0"
+[invoices.bindle]
+name = "mybindle"
+description = "My first bindle"
+version = "0.1.0"
 authors = ["Matt Butcher <[email protected]>"]
-description = "Another bindle example"
+
+[invoices.annotations]
+myname = "myvalue"
+
+[[invoices.parcels]]
+[invoices.parcels.label]
+sha256 = "e1706ab0a39ac88094b6d54a3f5cdba41fe5a901"
+mediaType = "text/html"
+name = "myparcel.html"
+
+[[invoices.parcels]]
+[invoices.parcels.label]
+sha256 = "098fa798779ac88094b6d54a3f5cdba41fe5a901"
+mediaType = "text/css"
+name = "style.css"
+
+[[invoices.parcels]]
+[invoices.parcels.label]
+sha256 = "5b992e90b71d5fadab3cd3777230ef370df75f5b"
+mediaType = "application/x-javascript"
+name = "foo.js"
+size = 248098
+
+[[invoices]]
+bindleVersion = "1.0.0"
+
+[invoices.bindle]
+name = "example.com/mybindle"
+version = "0.1.0"
+
+[[invoices.parcels]]
+[invoices.parcels.label]
+sha256 = "5b992e90b71d5fadab3cd3777230ef370df75f5b"
+mediaType = "application/x-javascript"
+name = "first"
+
+[[invoices.parcels]]
+[invoices.parcels.label]
+omitted...
+
+[[invoices.parcels]]
+[invoices.parcels.label]
+omitted...
+
+[[invoices.parcels]]
+[invoices.parcels.label]
+omitted...
 ```
 
 The top-level search fields are:
@@ -160,10 +204,11 @@ The top-level search fields are:
 - `offset`: (REQUIRED) The offset (as an unsigned 64-bit integer) for the first record in the returned results
 - `limit`: (REQUIRED) The maximum number of results that this query would return on this page
 - `timestamp`: (REQUIRED) The UNIX timestamp (as a 64-bit integer) at which the query was processed
+- `yanked`: (REQUIRED) A boolean flag indicating whether the list of invoices includes potentially yanked invoices 
 - `total`: (OPTIONAL) The total number of matches found. If this is set to 0, it means no matches were found. If it is unset, it MAY be interpreted that the match count was not tallied.
 - `more`: (OPTIONAL) A boolean flag indicating whether more matches are available on the server at the time indicated by `timestamp`.
 
-The attached list of bindles MUST contain the `[bindle]` fields of the `invoice` object. Results MAY also contain `[annotations]` data (in a separate annotations section). Results MAY contain `[[parcel]]` definitions.
+The attached list of invoices MUST contain the `[bindle]` fields of the `invoice` object. Results MAY also contain `[annotations]` data (in a separate annotations section). Results MAY contain `[[parcel]]` definitions.
 
 See the [Invoice Specification](invoice-spec.md) for a description of the `[bindle]` fields.
 

src/search.rs

@@ -1,7 +1,9 @@
-use semver::{Version, VersionReq};
 use std::collections::BTreeMap;
 use std::ops::RangeInclusive;
 
+use semver::{Version, VersionReq};
+use serde::Serialize;
+
 /// The search options for performing this query and returning results
 pub struct SearchOptions {
     /// The offset from the last search results
@@ -26,7 +28,10 @@ impl Default for SearchOptions {
 }
 
 /// Describes the matches that are returned
+#[derive(Debug, Serialize)]
 pub struct Matches {
+    /// The query used to find this match set
+    pub query: String,
     /// Whether the search engine used strict mode
     pub strict: bool,
     /// The offset of the first result in the matches
@@ -39,18 +44,21 @@ pub struct Matches {
     pub total: u64,
     /// Whether there are more results than the ones returned here
     pub more: bool,
+    /// Whether this list includes potentially yanked invoices
+    pub yanked: bool,
     /// The list of invoices returned as this part of the query
     ///
     /// The length of this Vec will be less than or equal to the limit.
+    // This needs to go at the bottom otherwise the table serialization in TOML gets weird. See
+    // https://github.com/alexcrichton/toml-rs/issues/258
     pub invoices: Vec<crate::Invoice>,
-    /// Whether this list includes potentially yanked invoices
-    pub yanked: bool,
 }
 
 impl Matches {
-    fn new(opts: &SearchOptions) -> Self {
+    fn new(opts: &SearchOptions, query: String) -> Self {
         Matches {
             // Assume options are definitive.
+            query,
             strict: opts.strict,
             offset: opts.offset,
             limit: opts.limit,
@@ -66,6 +74,8 @@ impl Matches {
 
 /// This trait describes the minimal set of features a Bindle provider must implement
 /// to provide query support.
+// TODO: Perhaps we should make this async and put the burden of locking on the Search
+// implementations rather than on the users of them
 pub trait Search {
     /// A high-level function that can take raw search strings (queries and filters) and options.
     ///
@@ -125,12 +135,12 @@ impl Search for StrictEngine {
             .filter(|(key, value)| {
                 // Term and version have to be exact matches.
                 // TODO: Version should have matching turned on.
-                *key == &term && version_compare(value.bindle.version.as_str(), &filter)
+                *key == &term && version_compare(&value.bindle.version, &filter)
             })
             .map(|(_, v)| (*v).clone())
             .collect();
 
-        let mut matches = Matches::new(&options);
+        let mut matches = Matches::new(&options, term);
         matches.strict = true;
         matches.yanked = false;
         matches.total = found.len() as u64;
@@ -186,12 +196,15 @@ impl Search for StrictEngine {
 /// In all other cases, if the version satisfies the requirement, this returns true.
 /// And if it fails to satisfy the requirement, this returns false.
 pub fn version_compare(version: &str, requirement: &str) -> bool {
+    println!(
+        "Got version compare. Version: {}, Requirement: {}",
+        version, requirement
+    );
     if requirement.is_empty() {
         return true;
     }
 
     if let Ok(req) = VersionReq::parse(requirement) {
-        println!("Parsed {}", req);
         return match Version::parse(version) {
             Ok(ver) => req.matches(&ver),
             Err(e) => {

src/server/filters.rs

@@ -9,12 +9,42 @@ use warp::reject::{custom, Reject, Rejection};
 use warp::Filter;
 
 use super::TOML_MIME_TYPE;
+use crate::search::SearchOptions;
 
 #[derive(Debug, Deserialize)]
 pub struct InvoiceQuery {
     pub yanked: Option<bool>,
 }
 
+#[derive(Debug, Deserialize)]
+#[serde(deny_unknown_fields)]
+pub struct QueryOptions {
+    #[serde(alias = "q")]
+    pub query: Option<String>,
+    #[serde(alias = "v")]
+    pub version: Option<String>,
+    #[serde(alias = "o")]
+    pub offset: Option<u64>,
+    #[serde(alias = "l")]
+    pub limit: Option<u8>,
+    pub strict: Option<bool>,
+    pub yanked: Option<bool>,
+}
+
+// This isn't a `From` implementation because it isn't symmetrical. Converting from a
+// `SearchOptions` to a `QueryOptions` would always end up with some fields set to none.
+impl Into<SearchOptions> for QueryOptions {
+    fn into(self) -> SearchOptions {
+        let defaults = SearchOptions::default();
+        SearchOptions {
+            limit: self.limit.unwrap_or(defaults.limit),
+            offset: self.offset.unwrap_or(defaults.offset),
+            strict: self.strict.unwrap_or(defaults.strict),
+            yanked: self.yanked.unwrap_or(defaults.yanked),
+        }
+    }
+}
+
 // Lovingly borrowed from https://docs.rs/warp/0.2.5/src/warp/filters/body.rs.html
 pub fn toml<T: DeserializeOwned + Send>() -> impl Filter<Extract = (T,), Error = Rejection> + Copy {
     // We can't use the http type constant here because clippy is warning about it having internal

src/server/handlers.rs

@@ -14,15 +14,33 @@ pub mod v1 {
 
     use std::io::Read;
 
+    use crate::server::filters::QueryOptions;
     use bytes::buf::BufExt;
     use tokio::stream::StreamExt;
     use tokio_util::codec::{BytesCodec, FramedRead};
 
     //////////// Invoice Functions ////////////
     pub async fn query_invoices<S: Search>(
+        options: QueryOptions,
         index: Arc<RwLock<S>>,
     ) -> Result<impl warp::Reply, Infallible> {
-        Ok(reply::toml(&"yay".to_string()))
+        let term = options.query.clone().unwrap_or_default();
+        let version = options.version.clone().unwrap_or_default();
+        let locked_index = index.read().await;
+        let matches = match locked_index.query(term, version, options.into()) {
+            Ok(m) => m,
+            Err(e) => {
+                return Ok(reply::reply_from_error(
+                    e,
+                    warp::http::StatusCode::BAD_REQUEST,
+                ))
+            }
+        };
+
+        Ok(warp::reply::with_status(
+            reply::toml(&matches),
+            warp::http::StatusCode::OK,
+        ))
     }
 
     pub async fn create_invoice<S: Storage>(

src/server/reply.rs

@@ -23,9 +23,9 @@ where
     T: Serialize,
 {
     Toml {
-        // TODO: When we add logging, log the error here so we know when there is a serialize
-        // failure
-        inner: toml::to_vec(val).map_err(|_| ()),
+        inner: toml::to_vec(val).map_err(|e| {
+            eprintln!("Error while serializing TOML: {:?}", e);
+        }),
     }
 }
 

src/server/routes.rs

@@ -22,7 +22,8 @@ pub mod v1 {
         {
             warp::path("_q")
                 .and(warp::get())
-                .map(move || index.clone())
+                .and(warp::query::<filters::QueryOptions>())
+                .and(warp::any().map(move || index.clone()))
                 .and_then(query_invoices)
         }
 

src/storage/file/mod.rs

@@ -121,6 +121,9 @@ impl<T: crate::search::Search + Send + Sync> Storage for FileStorage<T> {
 
         // Open the destination or error out if it already exists.
         let dest = self.invoice_toml_path(invoice_id);
+        if dest.exists() {
+            return Err(StorageError::Exists);
+        }
         let mut out = OpenOptions::new()
             .create_new(true)
             .write(true)