Skip to content
This repository
Browse code

Merge pull request #1802 from Montellese/urloptions_fix

allow key-only URL options and add CUrlOptions::RemoveOption()
  • Loading branch information...
commit a44e59f3b76f5d1f84d27f87c4896a699c8e914e 2 parents 7ec59c2 + a5d4013
jmarshallnz authored November 18, 2012
7  project/VS2010Express/XBMC.vcxproj
@@ -1213,6 +1213,13 @@
1213 1213
     </ClInclude>
1214 1214
     <ClInclude Include="..\..\xbmc\interfaces\json-rpc\AddonsOperations.h" />
1215 1215
     <ClCompile Include="..\..\xbmc\ThumbLoader.cpp" />
  1216
+    <ClCompile Include="..\..\xbmc\utils\test\TestUrlOptions.cpp">
  1217
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug (DirectX)|Win32'">true</ExcludedFromBuild>
  1218
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Debug (OpenGL)|Win32'">true</ExcludedFromBuild>
  1219
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release (DirectX)|Win32'">true</ExcludedFromBuild>
  1220
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Release (OpenGL)|Win32'">true</ExcludedFromBuild>
  1221
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='Template|Win32'">true</ExcludedFromBuild>
  1222
+    </ClCompile>
1216 1223
     <ClCompile Include="..\..\xbmc\video\VideoThumbLoader.cpp" />
1217 1224
     <ClCompile Include="..\..\xbmc\music\MusicThumbLoader.cpp" />
1218 1225
     <ClCompile Include="..\..\xbmc\ThumbnailCache.cpp" />
3  project/VS2010Express/XBMC.vcxproj.filters
@@ -2948,6 +2948,9 @@
2948 2948
     <ClCompile Include="..\..\xbmc\cores\AudioEngine\Utils\AELimiter.cpp">
2949 2949
       <Filter>cores\AudioEngine\Utils</Filter>
2950 2950
     </ClCompile>
  2951
+    <ClCompile Include="..\..\xbmc\utils\test\TestUrlOptions.cpp">
  2952
+      <Filter>utils\test</Filter>
  2953
+    </ClCompile>
2951 2954
   </ItemGroup>
2952 2955
   <ItemGroup>
2953 2956
     <ClInclude Include="..\..\xbmc\win32\pch.h">
6  xbmc/DbUrl.cpp
@@ -130,6 +130,12 @@ void CDbUrl::AddOptions(const std::string &options)
130 130
   updateOptions();  
131 131
 }
132 132
 
  133
+void CDbUrl::RemoveOption(const std::string &key)
  134
+{
  135
+  CUrlOptions::RemoveOption(key);
  136
+  updateOptions(); 
  137
+}
  138
+
133 139
 bool CDbUrl::validateOption(const std::string &key, const CVariant &value)
134 140
 {
135 141
   if (key.empty())
1  xbmc/DbUrl.h
@@ -46,6 +46,7 @@ class CDbUrl : public CUrlOptions
46 46
   virtual void AddOption(const std::string &key, double value);
47 47
   virtual void AddOption(const std::string &key, bool value);
48 48
   virtual void AddOptions(const std::string &options);
  49
+  virtual void RemoveOption(const std::string &key);
49 50
 
50 51
 protected:
51 52
   virtual bool parse() = 0;
2  xbmc/URL.cpp
@@ -801,6 +801,6 @@ void CURL::SetOption(const CStdString &key, const CStdString &value)
801 801
 
802 802
 void CURL::RemoveOption(const CStdString &key)
803 803
 {
804  
-  m_options.AddOption(key, "");
  804
+  m_options.RemoveOption(key);
805 805
   SetOptions(m_options.GetOptionsString(true));
806 806
 }
2  xbmc/dialogs/GUIDialogMediaFilter.cpp
@@ -617,7 +617,7 @@ bool CGUIDialogMediaFilter::SetPath(const std::string &path)
617 617
 
618 618
   // remove "filter" option
619 619
   if (m_dbUrl->HasOption("filter"))
620  
-    m_dbUrl->AddOption("filter", "");
  620
+    m_dbUrl->RemoveOption("filter");
621 621
 
622 622
   if (video)
623 623
     m_mediaType = ((CVideoDbUrl*)m_dbUrl)->GetItemType();
30  xbmc/filesystem/SmartPlaylistDirectory.cpp
@@ -111,7 +111,11 @@ namespace XFILE
111 111
           if (!playlist.SaveAsJson(xsp, !filter))
112 112
             return false;
113 113
         }
114  
-        videoUrl.AddOption(option, xsp);
  114
+
  115
+        if (!xsp.empty())
  116
+          videoUrl.AddOption(option, xsp);
  117
+        else
  118
+          videoUrl.RemoveOption(option);
115 119
         
116 120
         CDatabase::Filter dbfilter;
117 121
         success = db.GetSortedVideos(mediaType, videoUrl.ToString(), sorting, items, dbfilter);
@@ -140,7 +144,11 @@ namespace XFILE
140 144
           if (!playlist.SaveAsJson(xsp, !filter))
141 145
             return false;
142 146
         }
143  
-        musicUrl.AddOption(option, xsp);
  147
+
  148
+        if (!xsp.empty())
  149
+          musicUrl.AddOption(option, xsp);
  150
+        else
  151
+          musicUrl.RemoveOption(option);
144 152
 
145 153
         CDatabase::Filter dbfilter;
146 154
         success = db.GetAlbumsByWhere(musicUrl.ToString(), dbfilter, items, sorting);
@@ -165,7 +173,11 @@ namespace XFILE
165 173
           if (!playlist.SaveAsJson(xsp, !filter))
166 174
             return false;
167 175
         }
168  
-        musicUrl.AddOption(option, xsp);
  176
+
  177
+        if (!xsp.empty())
  178
+          musicUrl.AddOption(option, xsp);
  179
+        else
  180
+          musicUrl.RemoveOption(option);
169 181
 
170 182
         CDatabase::Filter dbfilter;
171 183
         success = db.GetArtistsNav(musicUrl.ToString(), items, !g_guiSettings.GetBool("musiclibrary.showcompilationartists"), -1, -1, -1, dbfilter, sorting);
@@ -195,7 +207,11 @@ namespace XFILE
195 207
           if (!songPlaylist.SaveAsJson(xsp, !filter))
196 208
             return false;
197 209
         }
198  
-        musicUrl.AddOption(option, xsp);
  210
+
  211
+        if (!xsp.empty())
  212
+          musicUrl.AddOption(option, xsp);
  213
+        else
  214
+          musicUrl.RemoveOption(option);
199 215
 
200 216
         CDatabase::Filter dbfilter;
201 217
         success = db.GetSongsByWhere(musicUrl.ToString(), dbfilter, items, sorting);
@@ -224,7 +240,11 @@ namespace XFILE
224 240
           if (!mvidPlaylist.SaveAsJson(xsp, !filter))
225 241
             return false;
226 242
         }
227  
-        videoUrl.AddOption(option, xsp);
  243
+
  244
+        if (!xsp.empty())
  245
+          videoUrl.AddOption(option, xsp);
  246
+        else
  247
+          videoUrl.RemoveOption(option);
228 248
         
229 249
         CFileItemList items2;
230 250
         success2 = db.GetSortedVideos(MediaTypeMusicVideo, videoUrl.ToString(), sorting, items2);
2  xbmc/music/MusicDatabase.cpp
@@ -5445,7 +5445,7 @@ bool CMusicDatabase::GetFilter(CDbUrl &musicUrl, Filter &filter, SortDescription
5445 5445
     }
5446 5446
     // remove the filter if it doesn't match the item type
5447 5447
     else
5448  
-      musicUrl.AddOption("filter", "");
  5448
+      musicUrl.RemoveOption("filter");
5449 5449
   }
5450 5450
 
5451 5451
   return true;
31  xbmc/utils/UrlOptions.cpp
@@ -46,7 +46,9 @@ std::string CUrlOptions::GetOptionsString(bool withLeadingSeperator /* = false *
46 46
     if (opt != m_options.begin())
47 47
       options += "&";
48 48
 
49  
-    options += CURL::Encode(opt->first) + "=" + CURL::Encode(opt->second.asString());
  49
+    options += CURL::Encode(opt->first);
  50
+    if (!opt->second.empty())
  51
+      options += "=" + CURL::Encode(opt->second.asString());
50 52
   }
51 53
 
52 54
   if (withLeadingSeperator && !options.empty())
@@ -68,11 +70,7 @@ void CUrlOptions::AddOption(const std::string &key, const std::string &value)
68 70
   if (key.empty())
69 71
     return;
70 72
 
71  
-  UrlOptions::iterator option = m_options.find(key);
72  
-  if (!value.empty())
73  
-    m_options[key] = value;
74  
-  else if (option != m_options.end())
75  
-    m_options.erase(option);
  73
+  m_options[key] = value;
76 74
 }
77 75
 
78 76
 void CUrlOptions::AddOption(const std::string &key, int value)
@@ -130,13 +128,12 @@ void CUrlOptions::AddOptions(const std::string &options)
130 128
     if (option->empty())
131 129
       continue;
132 130
 
133  
-    // every option must have the format key=value
134  
-    size_t pos = option->find('=');
135  
-    if (pos == string::npos || pos == 0 || pos >= option->size() - 1)
136  
-      continue;
  131
+    string key, value;
137 132
 
138  
-    string key = CURL::Decode(option->substr(0, pos));
139  
-    string value = CURL::Decode(option->substr(pos + 1));
  133
+    size_t pos = option->find('=');
  134
+    key = CURL::Decode(option->substr(0, pos));
  135
+    if (pos != string::npos)
  136
+      value = CURL::Decode(option->substr(pos + 1));
140 137
 
141 138
     // the key cannot be empty
142 139
     if (!key.empty())
@@ -149,6 +146,16 @@ void CUrlOptions::AddOptions(const CUrlOptions &options)
149 146
   m_options.insert(options.m_options.begin(), options.m_options.end());
150 147
 }
151 148
 
  149
+void CUrlOptions::RemoveOption(const std::string &key)
  150
+{
  151
+  if (key.empty())
  152
+    return;
  153
+
  154
+  UrlOptions::iterator option = m_options.find(key);
  155
+  if (option != m_options.end())
  156
+    m_options.erase(option);
  157
+}
  158
+
152 159
 bool CUrlOptions::HasOption(const std::string &key) const
153 160
 {
154 161
   if (key.empty())
1  xbmc/utils/UrlOptions.h
@@ -45,6 +45,7 @@ class CUrlOptions
45 45
   virtual void AddOption(const std::string &key, bool value);
46 46
   virtual void AddOptions(const std::string &options);
47 47
   virtual void AddOptions(const CUrlOptions &options);
  48
+  virtual void RemoveOption(const std::string &key);
48 49
 
49 50
   virtual bool HasOption(const std::string &key) const;
50 51
   virtual bool GetOption(const std::string &key, CVariant &value) const;
4  xbmc/utils/Variant.cpp
@@ -725,8 +725,10 @@ bool CVariant::empty() const
725 725
     return m_data.string->empty();
726 726
   else if (m_type == VariantTypeWideString)
727 727
     return m_data.wstring->empty();
728  
-  else
  728
+  else if (m_type == VariantTypeNull)
729 729
     return true;
  730
+
  731
+  return false;
730 732
 }
731 733
 
732 734
 void CVariant::clear()
1  xbmc/utils/test/Makefile
@@ -49,6 +49,7 @@ SRCS=	\
49 49
 	TestTimeSmoother.cpp \
50 50
 	TestTimeUtils.cpp \
51 51
 	TestURIUtils.cpp \
  52
+	TestUrlOptions.cpp \
52 53
 	TestVariant.cpp \
53 54
 	TestXBMCTinyXML.cpp \
54 55
 	TestXMLUtils.cpp
204  xbmc/utils/test/TestUrlOptions.cpp
... ...
@@ -0,0 +1,204 @@
  1
+/*
  2
+ *      Copyright (C) 2005-2012 Team XBMC
  3
+ *      http://www.xbmc.org
  4
+ *
  5
+ *  This Program is free software; you can redistribute it and/or modify
  6
+ *  it under the terms of the GNU General Public License as published by
  7
+ *  the Free Software Foundation; either version 2, or (at your option)
  8
+ *  any later version.
  9
+ *
  10
+ *  This Program is distributed in the hope that it will be useful,
  11
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
  12
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
  13
+ *  GNU General Public License for more details.
  14
+ *
  15
+ *  You should have received a copy of the GNU General Public License
  16
+ *  along with XBMC; see the file COPYING.  If not, see
  17
+ *  <http://www.gnu.org/licenses/>.
  18
+ *
  19
+ */
  20
+
  21
+#include "utils/UrlOptions.h"
  22
+
  23
+#include "gtest/gtest.h"
  24
+
  25
+TEST(TestUrlOptions, Clear)
  26
+{
  27
+  const char *key = "foo";
  28
+
  29
+  CUrlOptions urlOptions;
  30
+  urlOptions.AddOption(key, "bar");
  31
+  EXPECT_TRUE(urlOptions.HasOption(key));
  32
+
  33
+  urlOptions.Clear();
  34
+  EXPECT_FALSE(urlOptions.HasOption(key));
  35
+}
  36
+
  37
+TEST(TestUrlOptions, AddOption)
  38
+{
  39
+  const char *keyChar = "char";
  40
+  const char *keyString = "string";
  41
+  const char *keyEmpty = "empty";
  42
+  const char *keyInt = "int";
  43
+  const char *keyFloat = "float";
  44
+  const char *keyDouble = "double";
  45
+  const char *keyBool = "bool";
  46
+
  47
+  const char *valueChar = "valueChar";
  48
+  const std::string valueString = "valueString";
  49
+  const char *valueEmpty = "";
  50
+  int valueInt = 1;
  51
+  float valueFloat = 1.0f;
  52
+  double valueDouble = 1.0;
  53
+  bool valueBool = true;
  54
+
  55
+  CVariant variantValue;
  56
+
  57
+  CUrlOptions urlOptions;
  58
+  urlOptions.AddOption(keyChar, valueChar);
  59
+  {
  60
+    CVariant variantValue;
  61
+    EXPECT_TRUE(urlOptions.GetOption(keyChar, variantValue));
  62
+    EXPECT_TRUE(variantValue.isString());
  63
+    EXPECT_STREQ(valueChar, variantValue.asString().c_str());
  64
+  }
  65
+
  66
+  urlOptions.AddOption(keyString, valueString);
  67
+  {
  68
+    CVariant variantValue;
  69
+    EXPECT_TRUE(urlOptions.GetOption(keyString, variantValue));
  70
+    EXPECT_TRUE(variantValue.isString());
  71
+    EXPECT_STREQ(valueString.c_str(), variantValue.asString().c_str());
  72
+  }
  73
+
  74
+  urlOptions.AddOption(keyEmpty, valueEmpty);
  75
+  {
  76
+    CVariant variantValue;
  77
+    EXPECT_TRUE(urlOptions.GetOption(keyEmpty, variantValue));
  78
+    EXPECT_TRUE(variantValue.isString());
  79
+    EXPECT_STREQ(valueEmpty, variantValue.asString().c_str());
  80
+  }
  81
+
  82
+  urlOptions.AddOption(keyInt, valueInt);
  83
+  {
  84
+    CVariant variantValue;
  85
+    EXPECT_TRUE(urlOptions.GetOption(keyInt, variantValue));
  86
+    EXPECT_TRUE(variantValue.isInteger());
  87
+    EXPECT_EQ(valueInt, (int)variantValue.asInteger());
  88
+  }
  89
+
  90
+  urlOptions.AddOption(keyFloat, valueFloat);
  91
+  {
  92
+    CVariant variantValue;
  93
+    EXPECT_TRUE(urlOptions.GetOption(keyFloat, variantValue));
  94
+    EXPECT_TRUE(variantValue.isDouble());
  95
+    EXPECT_EQ(valueFloat, variantValue.asFloat());
  96
+  }
  97
+
  98
+  urlOptions.AddOption(keyDouble, valueDouble);
  99
+  {
  100
+    CVariant variantValue;
  101
+    EXPECT_TRUE(urlOptions.GetOption(keyDouble, variantValue));
  102
+    EXPECT_TRUE(variantValue.isDouble());
  103
+    EXPECT_EQ(valueDouble, variantValue.asDouble());
  104
+  }
  105
+
  106
+  urlOptions.AddOption(keyBool, valueBool);
  107
+  {
  108
+    CVariant variantValue;
  109
+    EXPECT_TRUE(urlOptions.GetOption(keyBool, variantValue));
  110
+    EXPECT_TRUE(variantValue.isBoolean());
  111
+    EXPECT_EQ(valueBool, variantValue.asBoolean());
  112
+  }
  113
+}
  114
+
  115
+TEST(TestUrlOptions, AddOptions)
  116
+{
  117
+  std::string ref = "foo=bar&key=value";
  118
+
  119
+  CUrlOptions urlOptions(ref);
  120
+  {
  121
+    CVariant value;
  122
+    EXPECT_TRUE(urlOptions.GetOption("foo", value));
  123
+    EXPECT_TRUE(value.isString());
  124
+    EXPECT_STREQ("bar", value.asString().c_str());
  125
+  }
  126
+  {
  127
+    CVariant value;
  128
+    EXPECT_TRUE(urlOptions.GetOption("key", value));
  129
+    EXPECT_TRUE(value.isString());
  130
+    EXPECT_STREQ("value", value.asString().c_str());
  131
+  }
  132
+
  133
+  ref = "foo=bar&key";
  134
+  urlOptions.Clear();
  135
+  urlOptions.AddOptions(ref);
  136
+  {
  137
+    CVariant value;
  138
+    EXPECT_TRUE(urlOptions.GetOption("foo", value));
  139
+    EXPECT_TRUE(value.isString());
  140
+    EXPECT_STREQ("bar", value.asString().c_str());
  141
+  }
  142
+  {
  143
+    CVariant value;
  144
+    EXPECT_TRUE(urlOptions.GetOption("key", value));
  145
+    EXPECT_TRUE(value.isString());
  146
+    EXPECT_TRUE(value.empty());
  147
+  }
  148
+}
  149
+
  150
+TEST(TestUrlOptions, RemoveOption)
  151
+{
  152
+  const char *key = "foo";
  153
+
  154
+  CUrlOptions urlOptions;
  155
+  urlOptions.AddOption(key, "bar");
  156
+  EXPECT_TRUE(urlOptions.HasOption(key));
  157
+
  158
+  urlOptions.RemoveOption(key);
  159
+  EXPECT_FALSE(urlOptions.HasOption(key));
  160
+}
  161
+
  162
+TEST(TestUrlOptions, HasOption)
  163
+{
  164
+  const char *key = "foo";
  165
+
  166
+  CUrlOptions urlOptions;
  167
+  urlOptions.AddOption(key, "bar");
  168
+  EXPECT_TRUE(urlOptions.HasOption(key));
  169
+  EXPECT_FALSE(urlOptions.HasOption("bar"));
  170
+}
  171
+
  172
+TEST(TestUrlOptions, GetOptions)
  173
+{
  174
+  const char *key1 = "foo";
  175
+  const char *key2 = "key";
  176
+  const char *value1 = "bar";
  177
+  const char *value2 = "value";
  178
+
  179
+  CUrlOptions urlOptions;
  180
+  urlOptions.AddOption(key1, value1);
  181
+  urlOptions.AddOption(key2, value2);
  182
+  const CUrlOptions::UrlOptions &options = urlOptions.GetOptions();
  183
+  EXPECT_FALSE(options.empty());
  184
+  EXPECT_EQ(2, options.size());
  185
+
  186
+  CUrlOptions::UrlOptions::const_iterator it1 = options.find(key1);
  187
+  EXPECT_TRUE(it1 != options.end());
  188
+  CUrlOptions::UrlOptions::const_iterator it2 = options.find(key2);
  189
+  EXPECT_TRUE(it2 != options.end());
  190
+  EXPECT_FALSE(options.find("wrong") != options.end());
  191
+  EXPECT_TRUE(it1->second.isString());
  192
+  EXPECT_TRUE(it2->second.isString());
  193
+  EXPECT_STREQ(value1, it1->second.asString().c_str());
  194
+  EXPECT_STREQ(value2, it2->second.asString().c_str());
  195
+}
  196
+
  197
+TEST(TestUrlOptions, GetOptionsString)
  198
+{
  199
+  const char *ref = "foo=bar&key";
  200
+
  201
+  CUrlOptions urlOptions(ref);
  202
+  std::string value = urlOptions.GetOptionsString();
  203
+  EXPECT_STREQ(ref, value.c_str());
  204
+}
2  xbmc/video/VideoDatabase.cpp
@@ -9342,7 +9342,7 @@ bool CVideoDatabase::GetFilter(CDbUrl &videoUrl, Filter &filter, SortDescription
9342 9342
     }
9343 9343
     // remove the filter if it doesn't match the item type
9344 9344
     else
9345  
-      videoUrl.AddOption("filter", "");
  9345
+      videoUrl.RemoveOption("filter");
9346 9346
   }
9347 9347
 
9348 9348
   return true;

0 notes on commit a44e59f

Please sign in to comment.
Something went wrong with that request. Please try again.