fixup! Add iterator support to url_search_params and ada_c

ada-url · Oct 10, 2023 · ed94fe5 · ed94fe5
1 parent 1a56d71
commit ed94fe5
Show file tree

Hide file tree

Showing 8 changed files with 61 additions and 28 deletions.
diff --git a/README.md b/README.md
@@ -211,7 +211,7 @@ search_params.append("g=h");
 search_params.get("g");  // will return "h"
 
 auto keys = search_params.get_keys();
-while (!keys.is_done()) {
+while (keys.has_next()) {
   auto key = keys.next();  // "a", "c", "e", "g"
 }
 ```

diff --git a/fuzz/parse.cc b/fuzz/parse.cc
@@ -122,17 +122,17 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
   }
 
   auto keys = search_params.get_keys();
-  while (!keys.is_done()) {
+  while (keys.has_next()) {
     keys.next();
   }
 
   auto values = search_params.get_values();
-  while (!values.is_done()) {
+  while (values.has_next()) {
     values.next();
   }
 
   auto entries = search_params.get_entries();
-  while (!entries.is_done()) {
+  while (entries.has_next()) {
     entries.next();
   }
 

diff --git a/include/ada/url_search_params-inl.h b/include/ada/url_search_params-inl.h
@@ -184,26 +184,26 @@ inline url_search_params_entries_iter url_search_params::get_entries() {
 }
 
 template <typename T, url_search_params_iter_type Type>
-inline bool url_search_params_iter<T, Type>::is_done() {
-  return pos >= params.params.size();
+inline bool url_search_params_iter<T, Type>::has_next() {
+  return pos < params.params.size();
 }
 
 template <>
 inline std::optional<std::string_view> url_search_params_keys_iter::next() {
-  if (is_done()) return std::nullopt;
+  if (!has_next()) return std::nullopt;
   return params.params[pos++].first;
 }
 
 template <>
 inline std::optional<std::string_view> url_search_params_values_iter::next() {
-  if (is_done()) return std::nullopt;
+  if (!has_next()) return std::nullopt;
   return params.params[pos++].second;
 }
 
 template <>
 inline std::optional<key_value_view_pair>
 url_search_params_entries_iter::next() {
-  if (is_done()) return std::nullopt;
+  if (!has_next()) return std::nullopt;
   return params.params[pos++];
 }
 

diff --git a/include/ada/url_search_params.h b/include/ada/url_search_params.h
@@ -95,7 +95,7 @@ struct url_search_params {
   inline std::string to_string();
 
   /**
-   * Returns a simple iterator over all of the keys in this url_search_params.
+   * Returns a simple JS-style iterator over all of the keys in this url_search_params.
    * The keys in the iterator are not unique. The valid lifespan of the iterator
    * is tied to the url_search_params. The iterator must be freed when you're
    * done with it.
@@ -104,22 +104,32 @@ struct url_search_params {
   inline url_search_params_keys_iter get_keys();
 
   /**
-   * Returns a simple iterator over all of the values in this url_search_params.
+   * Returns a simple JS-style iterator over all of the values in this url_search_params.
    * The valid lifespan of the iterator is tied to the url_search_params. The
    * iterator must be freed when you're done with it.
    * @see https://url.spec.whatwg.org/#interface-urlsearchparams
    */
   inline url_search_params_values_iter get_values();
 
   /**
-   * Returns a simple iterator over all of the entries in this
+   * Returns a simple JS-style iterator over all of the entries in this
    * url_search_params. The entries are pairs of keys and corresponding values.
    * The valid lifespan of the iterator is tied to the url_search_params. The
    * iterator must be freed when you're done with it.
    * @see https://url.spec.whatwg.org/#interface-urlsearchparams
    */
   inline url_search_params_entries_iter get_entries();
 
+  /**
+   * C++ style conventional iterator support. const only because we
+   * do not really want the params to be modified via the iterator.
+   */
+  inline const auto begin() const { return params.begin(); }
+  inline const auto end() const { return params.end(); }
+  inline const auto front() const { return params.front(); }
+  inline const auto back() const { return params.back(); }
+  inline const auto operator[](size_t index) const { return params[index]; }
+
  private:
   typedef std::pair<std::string, std::string> key_value_pair;
   std::vector<key_value_pair> params{};
@@ -133,6 +143,12 @@ struct url_search_params {
   friend struct url_search_params_iter;
 };  // url_search_params
 
+/**
+ * Implements a non-conventional iterator pattern that is closer in style to
+ * JavaScript's definition of an iterator.
+ *
+ * @see https://webidl.spec.whatwg.org/#idl-iterable
+ */
 template <typename T, url_search_params_iter_type Type>
 struct url_search_params_iter {
   url_search_params_iter(const url_search_params_iter &u) = default;
@@ -146,7 +162,8 @@ struct url_search_params_iter {
    * Return the next item in the iterator or std::nullopt if done.
    */
   inline std::optional<T> next();
-  inline bool is_done();
+
+  inline bool has_next();
 
  private:
   inline url_search_params_iter(url_search_params &params_) : params(params_) {}

diff --git a/include/ada_c.h b/include/ada_c.h
@@ -165,21 +165,21 @@ ada_string ada_strings_get(ada_strings result, size_t index);
 void ada_free_search_params_keys_iter(ada_url_search_params_keys_iter result);
 ada_string ada_search_params_keys_iter_next(
     ada_url_search_params_keys_iter result);
-bool ada_search_params_keys_iter_is_done(
+bool ada_search_params_keys_iter_has_next(
     ada_url_search_params_keys_iter result);
 
 void ada_free_search_params_values_iter(
     ada_url_search_params_values_iter result);
 ada_string ada_search_params_values_iter_next(
     ada_url_search_params_values_iter result);
-bool ada_search_params_values_iter_is_done(
+bool ada_search_params_values_iter_has_next(
     ada_url_search_params_values_iter result);
 
 void ada_free_search_params_entries_iter(
     ada_url_search_params_entries_iter result);
 ada_string_pair ada_search_params_entries_iter_next(
     ada_url_search_params_entries_iter result);
-bool ada_search_params_entries_iter_is_done(
+bool ada_search_params_entries_iter_has_next(
     ada_url_search_params_entries_iter result);
 
 #endif  // ADA_C_H
diff --git a/src/ada_c.cpp b/src/ada_c.cpp
@@ -626,12 +626,12 @@ ada_string ada_search_params_keys_iter_next(
   return ada_string_create(next->data(), next->length());
 }
 
-bool ada_search_params_keys_iter_is_done(
+bool ada_search_params_keys_iter_has_next(
     ada_url_search_params_keys_iter result) {
   ada::result<ada::url_search_params_keys_iter>* r =
       (ada::result<ada::url_search_params_keys_iter>*)result;
   if (!r) return true;
-  return (*r)->is_done();
+  return (*r)->has_next();
 }
 
 void ada_free_search_params_values_iter(
@@ -651,12 +651,12 @@ ada_string ada_search_params_values_iter_next(
   return ada_string_create(next->data(), next->length());
 }
 
-bool ada_search_params_values_iter_is_done(
+bool ada_search_params_values_iter_has_next(
     ada_url_search_params_values_iter result) {
   ada::result<ada::url_search_params_values_iter>* r =
       (ada::result<ada::url_search_params_values_iter>*)result;
   if (!r) return true;
-  return (*r)->is_done();
+  return (*r)->has_next();
 }
 
 void ada_free_search_params_entries_iter(
@@ -679,12 +679,12 @@ ada_string_pair ada_search_params_entries_iter_next(
       ada_string_create(next->second.data(), next->second.length())};
 }
 
-bool ada_search_params_entries_iter_is_done(
+bool ada_search_params_entries_iter_has_next(
     ada_url_search_params_entries_iter result) {
   ada::result<ada::url_search_params_entries_iter>* r =
       (ada::result<ada::url_search_params_entries_iter>*)result;
   if (!r) return true;
-  return (*r)->is_done();
+  return (*r)->has_next();
 }
 
 }  // extern "C"
diff --git a/tests/ada_c.cpp b/tests/ada_c.cpp
@@ -295,24 +295,24 @@ TEST(ada_c, ada_url_search_params) {
   ada_url_search_params_entries_iter entries =
       ada_search_params_get_entries(out);
 
-  ASSERT_FALSE(ada_search_params_keys_iter_is_done(keys));
-  ASSERT_FALSE(ada_search_params_values_iter_is_done(values));
-  ASSERT_FALSE(ada_search_params_entries_iter_is_done(entries));
+  ASSERT_TRUE(ada_search_params_keys_iter_has_next(keys));
+  ASSERT_TRUE(ada_search_params_values_iter_has_next(values));
+  ASSERT_TRUE(ada_search_params_entries_iter_has_next(entries));
 
   ASSERT_EQ(convert_string(ada_search_params_keys_iter_next(keys)), "a");
   ASSERT_EQ(convert_string(ada_search_params_keys_iter_next(keys)), "c");
   ASSERT_EQ(convert_string(ada_search_params_keys_iter_next(keys)), "c");
   ASSERT_EQ(convert_string(ada_search_params_keys_iter_next(keys)), "f");
   ASSERT_EQ(convert_string(ada_search_params_keys_iter_next(keys)), "key1");
-  ASSERT_TRUE(ada_search_params_keys_iter_is_done(keys));
+  ASSERT_FALSE(ada_search_params_keys_iter_has_next(keys));
 
   ASSERT_EQ(convert_string(ada_search_params_values_iter_next(values)), "b");
   ASSERT_EQ(convert_string(ada_search_params_values_iter_next(values)), "d");
   ASSERT_EQ(convert_string(ada_search_params_values_iter_next(values)), "e");
   ASSERT_EQ(convert_string(ada_search_params_values_iter_next(values)), "g");
   ASSERT_EQ(convert_string(ada_search_params_values_iter_next(values)),
             "value2");
-  ASSERT_TRUE(ada_search_params_values_iter_is_done(values));
+  ASSERT_FALSE(ada_search_params_values_iter_has_next(values));
 
   ada_string_pair pair = ada_search_params_entries_iter_next(entries);
   ASSERT_EQ(convert_string(pair.value), "b");
@@ -322,7 +322,7 @@ TEST(ada_c, ada_url_search_params) {
   ASSERT_EQ(convert_string(pair.value), "d");
   ASSERT_EQ(convert_string(pair.key), "c");
 
-  while (!ada_search_params_entries_iter_is_done(entries)) {
+  while (ada_search_params_entries_iter_has_next(entries)) {
     ada_search_params_entries_iter_next(entries);
   }
 

diff --git a/tests/url_search_params.cpp b/tests/url_search_params.cpp
@@ -169,6 +169,7 @@ TEST(url_search_params, has) {
 }
 
 TEST(url_search_params, iterators) {
+  // JS style iterators
   auto search_params =
       ada::url_search_params("key1=value1&key1=value2&key2=value3");
   auto keys = search_params.get_keys();
@@ -201,5 +202,20 @@ TEST(url_search_params, iterators) {
 
   ASSERT_FALSE(entries.next().has_value());
 
+  // C++ conventional iterator
+  std::vector<std::pair<std::string,std::string>> expected = {
+    {"foo", "bar"},
+    {"key2", "value3"},
+    {"key1", "value2"},
+    {"key1", "value1"},
+  };
+  for (auto& entry : search_params) {
+    auto check = expected.back();
+    expected.pop_back();
+    ASSERT_EQ(check.first, entry.first);
+    ASSERT_EQ(check.second, entry.second);
+  }
+  ASSERT_EQ(expected.size(), 0);
+
   SUCCEED();
 }