DownloadQuery filters and sorts DownloadItems.

content/browser/download/download_query.h

Index: content/browser/download/download_query.h
diff --git a/content/browser/download/download_query.h b/content/browser/download/download_query.h
new file mode 100644
index 0000000000000000000000000000000000000000..d0843c7ff931cd242d94306f8ceeed931067d294
--- /dev/null
+++ b/content/browser/download/download_query.h
@@ -0,0 +1,149 @@
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_QUERY_H_
+#define CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_QUERY_H_
+#pragma once
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "base/callback.h"
+#include "content/browser/download/download_item.h"
+
+namespace download_util {

[Randy Smith (Not in Mondays), 2011/11/23 21:07:34]

Given that we've got a pretty descriptive top level class name that everything's
under, I'm not sure there's any need to put this in download_util.

[benjhayden, 2011/11/30 16:21:47]

Done.

[cbentzel, 2011/12/05 21:56:26]

Have there been any thoughts about moving content/browser/downloads into the
downloads namespace?

[Randy Smith (Not in Mondays), 2011/12/12 18:17:54]

We've thought about it, but it's never seemed needed.  Any thoughts for what
determines when something should have its own namespace? 

+
+// Filter and sort a vector of DownloadItem*s.
+//
+// The following example copies from |all_items| to |results| those
+// DownloadItem*s whose start time is not 0 or whose id is odd, sorts primarily
+// by bytes received ascending and secondarily by url descending, and limits the
+// results to 20 items. Any number of filter fields or sort rules is
+// permissible. After all explicit sort rules are applied, two implicit sort
+// rules are applied: id ascending then db_handle ascending.

[Randy Smith (Not in Mondays), 2011/11/23 21:07:34]

nit: I find this comment sentence a little confusing, as if you do (presumed
stable :-}) sorts in order, it's the later ones that end up being most primary,
whereas I presume you mean here that id ascending and db_handle ascending are
least primary.  Is there a way to rephrase the comment to eliminate the
confusion?

[benjhayden, 2011/11/30 16:21:47]

Done.

+//
+// DownloadQuery query;
+// CHECK(query.Filter(FILTER_FIELD_START_TIME, 0));
+// bool FilterOutOddDownloads(const DownloadItem& item) {
+//   return 0 == (item.GetId() % 2);
+// }
+// CHECK(query.Filter(FILTER_FIELD_FILTER, base::Bind(&FilterOutOddDownloads)));
+// query.Sort(SORT_FIELD_BYTES_RECEIVED, ASCENDING);
+// query.Sort(SORT_FIELD_URL, DESCENDING);
+// query.Limit(20);
+// DownloadVector all_items, results;
+// query.Search(all_items.begin(), all_items.end(), &results);
+class CONTENT_EXPORT DownloadQuery {
+ public:
+  typedef std::vector<DownloadItem*> DownloadVector;
+
+  enum FilterFieldName {
+    FILTER_FIELD_BYTES_RECEIVED,
+    FILTER_FIELD_DANGER,
+    FILTER_FIELD_DANGER_ACCEPTED,
+    FILTER_FIELD_ENDED_AFTER,
+    FILTER_FIELD_ENDED_BEFORE,
+    FILTER_FIELD_END_TIME,
+    FILTER_FIELD_ERROR,
+    FILTER_FIELD_FILENAME,
+    FILTER_FIELD_FILENAME_REGEX,
+    FILTER_FIELD_FILE_SIZE,
+    FILTER_FIELD_FILE_SIZE_GREATER,
+    FILTER_FIELD_FILE_SIZE_LESS,
+    FILTER_FIELD_FILTER,
+    FILTER_FIELD_MIME,
+    FILTER_FIELD_PAUSED,
+    FILTER_FIELD_QUERY,
+    FILTER_FIELD_STARTED_AFTER,
+    FILTER_FIELD_STARTED_BEFORE,
+    FILTER_FIELD_START_TIME,
+    FILTER_FIELD_STATE,
+    FILTER_FIELD_TOTAL_BYTES,
+    FILTER_FIELD_TOTAL_BYTES_GREATER,
+    FILTER_FIELD_TOTAL_BYTES_LESS,
+    FILTER_FIELD_URL,
+    FILTER_FIELD_URL_REGEX,
+  };
+
+  enum SortFieldName {
+    SORT_FIELD_BYTES_RECEIVED,
+    SORT_FIELD_DANGER,
+    SORT_FIELD_DANGER_ACCEPTED,
+    SORT_FIELD_END_TIME,
+    SORT_FIELD_ERROR,
+    SORT_FIELD_FILENAME,
+    SORT_FIELD_FILE_SIZE,
+    SORT_FIELD_MIME,
+    SORT_FIELD_PAUSED,
+    SORT_FIELD_START_TIME,
+    SORT_FIELD_STATE,
+    SORT_FIELD_TOTAL_BYTES,
+    SORT_FIELD_URL,
+  };
+
+  enum SortFieldDirection {
+    ASCENDING,
+    DESCENDING,
+  };
+
+  DownloadQuery();
+  ~DownloadQuery();
+
+  // Returns false if |name| is unrecognized or if |value| is the wrong type or
+  // malformed. Otherwise, makes a new filter named |name|, sets its value to
+  // |value|, and returns true. Search() will filter out all DownloadItem*s that
+  // do not match all filters.  See the implementation for the type of each
+  // |name|.  (DRY.)

[Randy Smith (Not in Mondays), 2011/11/23 21:07:34]

nit: What doe the "DRY" add to the comment?  (I'm a bit biased as I had
forgotten what it mean and had to look it up, but having done that the only
reason I can see is as a justification for why you don't include the type of
each |name|, and I don't think it's worthwhile putting such justifications in
comments.  Is there another reason to have it?)

[benjhayden, 2011/11/30 16:21:47]

Done.

+  template <typename ValueType>
+  bool Filter(FilterFieldName name, const ValueType& value);
+
+  // Makes a new sort field named |name| and sets its direction to |direction|.
+  // After filtering DownloadItem*s, Search() will sort the results primarily by
+  // the first sort field, secondarily by the second sort field, and so on. For

[Randy Smith (Not in Mondays), 2011/11/23 21:07:34]

nit: I'd say "field in first call to sort"; "first sort field" requires the
reader to interpolate an ordering, and while the ordering of Sort() calls is the
obvious one to interpolate, it's better not to have to guess.

[benjhayden, 2011/11/30 16:21:47]

Done.

+  // example, if the InputIterator passed to Search() yields four DownloadItems
+  // {id:0, error:0, start_time:0}, {id:1, error:0, start_time:1}, {id:2,
+  // error:1, start_time:0}, {id:3, error:1, start_time:1}, and Sort is called
+  // twice, once with (SORT_FIELD_ERROR, ASCENDING) then with
+  // (SORT_FIELD_START_TIME, DESCENDING), then Search() will return items
+  // ordered 1,0,3,2.
+  void Sort(SortFieldName name, SortFieldDirection direction);
+
+  // Limit the size of search results to |limit|.
+  void Limit(size_t limit) { limit_ = limit; }
+
+  // Filters DownloadItem*s from |iter| to |last| into |results|, sorts
+  // |results|, and limits the size of |results|. |results| must be non-NULL.
+  template <typename InputIterator>
+  void Search(InputIterator iter, const InputIterator last,
+              DownloadVector* results) const;
+
+  // Returns true if |item| matches all set filter fields; returns false if any
+  // filter fields have been set that the item does not match.
+  bool Matches(const DownloadItem& item) const;

[Randy Smith (Not in Mondays), 2011/11/23 21:07:34]

Why is this a public member function?

[benjhayden, 2011/11/30 16:21:47]

If you pass an id to search(), then DownloadSearchFunction will lookup the item
by id instead of using DownloadQuery::Search(). In order to see if that item
matches the other filter fields, DSF could either construct a vector and add the
one item to it, or it could call Matches(). So, basically, Matches() is a
shortcut for constructing a vector and adding one item to it and calling
Search(). It might also facilitate the test.

+
+ private:
+  class FilterFieldInterface;
+  struct OrderTerm {
+    OrderTerm(SortFieldName _name, SortFieldDirection _direction)
+      : name(_name),
+        direction(_direction) {
+    }
+    SortFieldName name;
+    SortFieldDirection direction;
+  };
+  typedef std::vector<FilterFieldInterface*> FilterFieldVector;
+  typedef std::vector<OrderTerm> OrderTermVector;
+
+  bool AddFilter(FilterFieldInterface* field);
+
+  FilterFieldVector filter_fields_;
+  OrderTermVector order_by_fields_;
+  size_t limit_;
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadQuery);
+};
+}  // namespace download_util
+
+#endif  // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_QUERY_H_