mirror of https://github.com/aria2/aria2
2009-10-04 Tatsuhiro Tsujikawa <t-tujikawa@users.sourceforge.net>
Fixed the bug that the option values changed by XML-RPC method(changeOption and changeGlobalOption) are overwritten to the previous value by the next these request which doesn't contain that option value. Supporse max-download-limit is initially 0. You changed this value to 100K by changeOption. Then you issue changeOption request to change max-upload-limit to 50K. This second request doesn't contain xml-download-limit, so it is back to initial value, 0. Another improvement is that exception is thrown when changeOption and changeGlobalOption request contains option name which doesn't allowed in each request. * src/DownloadEngine.h * src/XmlRpcMethod.cc * src/XmlRpcMethod.h * src/XmlRpcMethodImpl.cc * src/download_helper.cc * src/download_helper.h * test/XmlRpcMethodTest.ccpull/1/head
parent
ce3de835b4
commit
c0595d17ff
20
ChangeLog
20
ChangeLog
|
@ -1,3 +1,23 @@
|
|||
2009-10-04 Tatsuhiro Tsujikawa <t-tujikawa@users.sourceforge.net>
|
||||
|
||||
Fixed the bug that the option values changed by XML-RPC
|
||||
method(changeOption and changeGlobalOption) are overwritten to the
|
||||
previous value by the next these request which doesn't contain
|
||||
that option value. Supporse max-download-limit is initially 0. You
|
||||
changed this value to 100K by changeOption. Then you issue
|
||||
changeOption request to change max-upload-limit to 50K. This
|
||||
second request doesn't contain xml-download-limit, so it is back
|
||||
to initial value, 0. Another improvement is that exception is
|
||||
thrown when changeOption and changeGlobalOption request contains
|
||||
option name which doesn't allowed in each request.
|
||||
* src/DownloadEngine.h
|
||||
* src/XmlRpcMethod.cc
|
||||
* src/XmlRpcMethod.h
|
||||
* src/XmlRpcMethodImpl.cc
|
||||
* src/download_helper.cc
|
||||
* src/download_helper.h
|
||||
* test/XmlRpcMethodTest.cc
|
||||
|
||||
2009-10-04 Tatsuhiro Tsujikawa <t-tujikawa@users.sourceforge.net>
|
||||
|
||||
Added missing MetalinkParserStateImpl.{cc,h}
|
||||
|
|
|
@ -155,7 +155,7 @@ public:
|
|||
SharedHandle<RequestGroupMan> _requestGroupMan;
|
||||
SharedHandle<FileAllocationMan> _fileAllocationMan;
|
||||
SharedHandle<CheckIntegrityMan> _checkIntegrityMan;
|
||||
const Option* option;
|
||||
Option* option;
|
||||
|
||||
DownloadEngine(const SharedHandle<EventPoll>& eventPoll);
|
||||
|
||||
|
|
|
@ -46,6 +46,7 @@
|
|||
#include "XmlRpcRequest.h"
|
||||
#include "XmlRpcResponse.h"
|
||||
#include "prefs.h"
|
||||
#include "StringFormat.h"
|
||||
|
||||
namespace aria2 {
|
||||
|
||||
|
@ -74,23 +75,32 @@ XmlRpcResponse XmlRpcMethod::execute
|
|||
}
|
||||
}
|
||||
|
||||
|
||||
template<typename InputIterator>
|
||||
static void gatherOption
|
||||
(InputIterator first, InputIterator last,
|
||||
const SharedHandle<Option>& option, const BDE& optionsDict,
|
||||
const std::set<std::string>& changeableOptions,
|
||||
const SharedHandle<Option>& option,
|
||||
const SharedHandle<OptionParser>& optionParser)
|
||||
{
|
||||
for(; first != last; ++first) {
|
||||
if(optionsDict.containsKey(*first)) {
|
||||
const BDE& value = optionsDict[*first];
|
||||
const std::string& optionName = (*first).first;
|
||||
if(changeableOptions.find(optionName) == changeableOptions.end()) {
|
||||
throw DL_ABORT_EX
|
||||
(StringFormat
|
||||
("%s cannot be changed or unknown option.", optionName.c_str()).str());
|
||||
} else {
|
||||
SharedHandle<OptionHandler> optionHandler =
|
||||
optionParser->findByName(*first);
|
||||
optionParser->findByName(optionName);
|
||||
if(optionHandler.isNull()) {
|
||||
continue;
|
||||
throw DL_ABORT_EX
|
||||
(StringFormat
|
||||
("We don't know how to deal with %s option",
|
||||
optionName.c_str()).str());
|
||||
}
|
||||
// header and index-out option can take array as value
|
||||
if((*first == PREF_HEADER || *first == PREF_INDEX_OUT) && value.isList()){
|
||||
const BDE& value = (*first).second;
|
||||
if((optionName == PREF_HEADER || optionName == PREF_INDEX_OUT) &&
|
||||
value.isList()){
|
||||
for(BDE::List::const_iterator argiter = value.listBegin();
|
||||
argiter != value.listEnd(); ++argiter) {
|
||||
if((*argiter).isString()) {
|
||||
|
@ -107,17 +117,33 @@ static void gatherOption
|
|||
void XmlRpcMethod::gatherRequestOption
|
||||
(const SharedHandle<Option>& option, const BDE& optionsDict)
|
||||
{
|
||||
gatherOption(listRequestOptions().begin(), listRequestOptions().end(),
|
||||
option, optionsDict, _optionParser);
|
||||
gatherOption(optionsDict.dictBegin(), optionsDict.dictEnd(),
|
||||
listRequestOptions(),
|
||||
option, _optionParser);
|
||||
}
|
||||
|
||||
const std::vector<std::string>& listChangeableOptions()
|
||||
// Copy option in the range [optNameFirst, optNameLast) from src to
|
||||
// dest.
|
||||
template<typename InputIterator>
|
||||
static void applyOption(InputIterator optNameFirst,
|
||||
InputIterator optNameLast,
|
||||
Option* dest,
|
||||
Option* src)
|
||||
{
|
||||
for(; optNameFirst != optNameLast; ++optNameFirst) {
|
||||
if(src->defined(*optNameFirst)) {
|
||||
dest->put(*optNameFirst, src->get(*optNameFirst));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const std::set<std::string>& listChangeableOptions()
|
||||
{
|
||||
static const std::string OPTIONS[] = {
|
||||
PREF_MAX_UPLOAD_LIMIT,
|
||||
PREF_MAX_DOWNLOAD_LIMIT,
|
||||
};
|
||||
static std::vector<std::string> options
|
||||
static std::set<std::string> options
|
||||
(&OPTIONS[0], &OPTIONS[arrayLength(OPTIONS)]);
|
||||
return options;
|
||||
}
|
||||
|
@ -125,18 +151,25 @@ const std::vector<std::string>& listChangeableOptions()
|
|||
void XmlRpcMethod::gatherChangeableOption
|
||||
(const SharedHandle<Option>& option, const BDE& optionsDict)
|
||||
{
|
||||
gatherOption(listChangeableOptions().begin(), listChangeableOptions().end(),
|
||||
option, optionsDict, _optionParser);
|
||||
gatherOption(optionsDict.dictBegin(), optionsDict.dictEnd(),
|
||||
listChangeableOptions(),
|
||||
option, _optionParser);
|
||||
}
|
||||
|
||||
const std::vector<std::string>& listChangeableGlobalOptions()
|
||||
void XmlRpcMethod::applyChangeableOption(Option* dest, Option* src) const
|
||||
{
|
||||
applyOption(listChangeableOptions().begin(), listChangeableOptions().end(),
|
||||
dest, src);
|
||||
}
|
||||
|
||||
const std::set<std::string>& listChangeableGlobalOptions()
|
||||
{
|
||||
static const std::string OPTIONS[] = {
|
||||
PREF_MAX_OVERALL_UPLOAD_LIMIT,
|
||||
PREF_MAX_OVERALL_DOWNLOAD_LIMIT,
|
||||
PREF_MAX_CONCURRENT_DOWNLOADS,
|
||||
};
|
||||
static std::vector<std::string> options
|
||||
static std::set<std::string> options
|
||||
(&OPTIONS[0], &OPTIONS[arrayLength(OPTIONS)]);
|
||||
return options;
|
||||
}
|
||||
|
@ -144,9 +177,16 @@ const std::vector<std::string>& listChangeableGlobalOptions()
|
|||
void XmlRpcMethod::gatherChangeableGlobalOption
|
||||
(const SharedHandle<Option>& option, const BDE& optionsDict)
|
||||
{
|
||||
gatherOption(listChangeableGlobalOptions().begin(),
|
||||
listChangeableGlobalOptions().end(),
|
||||
option, optionsDict, _optionParser);
|
||||
gatherOption(optionsDict.dictBegin(), optionsDict.dictEnd(),
|
||||
listChangeableGlobalOptions(),
|
||||
option, _optionParser);
|
||||
}
|
||||
|
||||
void XmlRpcMethod::applyChangeableGlobalOption(Option* dest, Option* src) const
|
||||
{
|
||||
applyOption(listChangeableGlobalOptions().begin(),
|
||||
listChangeableGlobalOptions().end(),
|
||||
dest, src);
|
||||
}
|
||||
|
||||
} // namespace xmlrpc
|
||||
|
|
|
@ -78,8 +78,16 @@ protected:
|
|||
void gatherChangeableOption(const SharedHandle<Option>& option,
|
||||
const BDE& optionDict);
|
||||
|
||||
// Copy options which is changeable in XML-RPC changeOption command
|
||||
// to dest.
|
||||
void applyChangeableOption(Option* dest, Option* src) const;
|
||||
|
||||
void gatherChangeableGlobalOption(const SharedHandle<Option>& option,
|
||||
const BDE& optionDict);
|
||||
|
||||
// Copy options which is changeable in XML-RPC changeGlobalOption
|
||||
// command to dest.
|
||||
void applyChangeableGlobalOption(Option* dest, Option* src) const;
|
||||
public:
|
||||
XmlRpcMethod();
|
||||
|
||||
|
|
|
@ -592,15 +592,17 @@ BDE ChangeOptionXmlRpcMethod::process
|
|||
throw DL_ABORT_EX
|
||||
(StringFormat("Cannot change option for GID#%d", gid).str());
|
||||
}
|
||||
SharedHandle<Option> option(new Option(*group->getOption().get()));
|
||||
SharedHandle<Option> option(new Option());
|
||||
if(params.size() > 1 && params[1].isDict()) {
|
||||
gatherChangeableOption(option, params[1]);
|
||||
}
|
||||
if(option->defined(PREF_MAX_DOWNLOAD_LIMIT)) {
|
||||
group->setMaxDownloadSpeedLimit(option->getAsInt(PREF_MAX_DOWNLOAD_LIMIT));
|
||||
}
|
||||
if(option->defined(PREF_MAX_UPLOAD_LIMIT)) {
|
||||
group->setMaxUploadSpeedLimit(option->getAsInt(PREF_MAX_UPLOAD_LIMIT));
|
||||
applyChangeableOption(group->getOption().get(), option.get());
|
||||
if(option->defined(PREF_MAX_DOWNLOAD_LIMIT)) {
|
||||
group->setMaxDownloadSpeedLimit
|
||||
(option->getAsInt(PREF_MAX_DOWNLOAD_LIMIT));
|
||||
}
|
||||
if(option->defined(PREF_MAX_UPLOAD_LIMIT)) {
|
||||
group->setMaxUploadSpeedLimit(option->getAsInt(PREF_MAX_UPLOAD_LIMIT));
|
||||
}
|
||||
}
|
||||
return BDE_OK;
|
||||
}
|
||||
|
@ -613,8 +615,10 @@ BDE ChangeGlobalOptionXmlRpcMethod::process
|
|||
if(params.empty() || !params[0].isDict()) {
|
||||
return BDE_OK;
|
||||
}
|
||||
SharedHandle<Option> option(new Option(*e->option));
|
||||
SharedHandle<Option> option(new Option());
|
||||
gatherChangeableGlobalOption(option, params[0]);
|
||||
applyChangeableGlobalOption(e->option, option.get());
|
||||
|
||||
if(option->defined(PREF_MAX_OVERALL_DOWNLOAD_LIMIT)) {
|
||||
e->_requestGroupMan->setMaxOverallDownloadSpeedLimit
|
||||
(option->getAsInt(PREF_MAX_OVERALL_DOWNLOAD_LIMIT));
|
||||
|
|
|
@ -67,7 +67,7 @@
|
|||
|
||||
namespace aria2 {
|
||||
|
||||
const std::vector<std::string>& listRequestOptions()
|
||||
const std::set<std::string>& listRequestOptions()
|
||||
{
|
||||
static const std::string REQUEST_OPTIONS[] = {
|
||||
PREF_DIR,
|
||||
|
@ -138,7 +138,7 @@ const std::vector<std::string>& listRequestOptions()
|
|||
PREF_PARAMETERIZED_URI,
|
||||
PREF_REALTIME_CHUNK_CHECKSUM
|
||||
};
|
||||
static std::vector<std::string> requestOptions
|
||||
static std::set<std::string> requestOptions
|
||||
(&REQUEST_OPTIONS[0],
|
||||
&REQUEST_OPTIONS[arrayLength(REQUEST_OPTIONS)]);;
|
||||
|
||||
|
@ -385,7 +385,7 @@ static void createRequestGroupForUriList
|
|||
}
|
||||
|
||||
SharedHandle<Option> requestOption(new Option(*option.get()));
|
||||
for(std::vector<std::string>::const_iterator i =
|
||||
for(std::set<std::string>::const_iterator i =
|
||||
listRequestOptions().begin(); i != listRequestOptions().end(); ++i) {
|
||||
if(tempOption->defined(*i)) {
|
||||
requestOption->put(*i, tempOption->get(*i));
|
||||
|
|
|
@ -39,7 +39,7 @@
|
|||
|
||||
#include <string>
|
||||
#include <deque>
|
||||
#include <vector>
|
||||
#include <set>
|
||||
|
||||
#include "SharedHandle.h"
|
||||
|
||||
|
@ -48,7 +48,7 @@ namespace aria2 {
|
|||
class RequestGroup;
|
||||
class Option;
|
||||
|
||||
const std::vector<std::string>& listRequestOptions();
|
||||
const std::set<std::string>& listRequestOptions();
|
||||
|
||||
#ifdef ENABLE_BITTORRENT
|
||||
// Create RequestGroup object using torrent file specified by
|
||||
|
|
|
@ -46,9 +46,11 @@ class XmlRpcMethodTest:public CppUnit::TestFixture {
|
|||
#endif // ENABLE_METALINK
|
||||
CPPUNIT_TEST(testChangeOption);
|
||||
CPPUNIT_TEST(testChangeOption_withBadOption);
|
||||
CPPUNIT_TEST(testChangeOption_withNotAllowedOption);
|
||||
CPPUNIT_TEST(testChangeOption_withoutGid);
|
||||
CPPUNIT_TEST(testChangeGlobalOption);
|
||||
CPPUNIT_TEST(testChangeGlobalOption_withBadOption);
|
||||
CPPUNIT_TEST(testChangeGlobalOption_withNotAllowedOption);
|
||||
CPPUNIT_TEST(testTellStatus_withoutGid);
|
||||
CPPUNIT_TEST(testTellWaiting);
|
||||
CPPUNIT_TEST(testTellWaiting_fail);
|
||||
|
@ -94,9 +96,11 @@ public:
|
|||
#endif // ENABLE_METALINK
|
||||
void testChangeOption();
|
||||
void testChangeOption_withBadOption();
|
||||
void testChangeOption_withNotAllowedOption();
|
||||
void testChangeOption_withoutGid();
|
||||
void testChangeGlobalOption();
|
||||
void testChangeGlobalOption_withBadOption();
|
||||
void testChangeGlobalOption_withNotAllowedOption();
|
||||
void testTellStatus_withoutGid();
|
||||
void testTellWaiting();
|
||||
void testTellWaiting_fail();
|
||||
|
@ -369,8 +373,12 @@ void XmlRpcMethodTest::testChangeOption()
|
|||
CPPUNIT_ASSERT_EQUAL(0, res._code);
|
||||
CPPUNIT_ASSERT_EQUAL((unsigned int)100*1024,
|
||||
group->getMaxDownloadSpeedLimit());
|
||||
CPPUNIT_ASSERT_EQUAL(std::string("102400"),
|
||||
group->getOption()->get(PREF_MAX_DOWNLOAD_LIMIT));
|
||||
#ifdef ENABLE_BITTORRENT
|
||||
CPPUNIT_ASSERT_EQUAL((unsigned int)50*1024, group->getMaxUploadSpeedLimit());
|
||||
CPPUNIT_ASSERT_EQUAL(std::string("51200"),
|
||||
group->getOption()->get(PREF_MAX_UPLOAD_LIMIT));
|
||||
#endif // ENABLE_BITTORRENT
|
||||
}
|
||||
|
||||
|
@ -389,6 +397,21 @@ void XmlRpcMethodTest::testChangeOption_withBadOption()
|
|||
CPPUNIT_ASSERT_EQUAL(1, res._code);
|
||||
}
|
||||
|
||||
void XmlRpcMethodTest::testChangeOption_withNotAllowedOption()
|
||||
{
|
||||
SharedHandle<RequestGroup> group(new RequestGroup(_option));
|
||||
_e->_requestGroupMan->addReservedGroup(group);
|
||||
|
||||
ChangeOptionXmlRpcMethod m;
|
||||
XmlRpcRequest req("aria2.changeOption", BDE::list());
|
||||
req._params << BDE("1");
|
||||
BDE opt = BDE::dict();
|
||||
opt[PREF_MAX_OVERALL_DOWNLOAD_LIMIT] = BDE("100K");
|
||||
req._params << opt;
|
||||
XmlRpcResponse res = m.execute(req, _e.get());
|
||||
CPPUNIT_ASSERT_EQUAL(1, res._code);
|
||||
}
|
||||
|
||||
void XmlRpcMethodTest::testChangeOption_withoutGid()
|
||||
{
|
||||
ChangeOptionXmlRpcMethod m;
|
||||
|
@ -412,9 +435,13 @@ void XmlRpcMethodTest::testChangeGlobalOption()
|
|||
CPPUNIT_ASSERT_EQUAL(0, res._code);
|
||||
CPPUNIT_ASSERT_EQUAL((unsigned int)100*1024,
|
||||
_e->_requestGroupMan->getMaxOverallDownloadSpeedLimit());
|
||||
CPPUNIT_ASSERT_EQUAL(std::string("102400"),
|
||||
_e->option->get(PREF_MAX_OVERALL_DOWNLOAD_LIMIT));
|
||||
#ifdef ENABLE_BITTORRENT
|
||||
CPPUNIT_ASSERT_EQUAL((unsigned int)50*1024,
|
||||
_e->_requestGroupMan->getMaxOverallUploadSpeedLimit());
|
||||
CPPUNIT_ASSERT_EQUAL(std::string("51200"),
|
||||
_e->option->get(PREF_MAX_OVERALL_UPLOAD_LIMIT));
|
||||
#endif // ENABLE_BITTORRENT
|
||||
}
|
||||
|
||||
|
@ -429,6 +456,17 @@ void XmlRpcMethodTest::testChangeGlobalOption_withBadOption()
|
|||
CPPUNIT_ASSERT_EQUAL(1, res._code);
|
||||
}
|
||||
|
||||
void XmlRpcMethodTest::testChangeGlobalOption_withNotAllowedOption()
|
||||
{
|
||||
ChangeGlobalOptionXmlRpcMethod m;
|
||||
XmlRpcRequest req("aria2.changeGlobalOption", BDE::list());
|
||||
BDE opt = BDE::dict();
|
||||
opt[PREF_MAX_DOWNLOAD_LIMIT] = BDE("100K");
|
||||
req._params << opt;
|
||||
XmlRpcResponse res = m.execute(req, _e.get());
|
||||
CPPUNIT_ASSERT_EQUAL(1, res._code);
|
||||
}
|
||||
|
||||
void XmlRpcMethodTest::testNoSuchMethod()
|
||||
{
|
||||
NoSuchMethodXmlRpcMethod m;
|
||||
|
|
Loading…
Reference in New Issue