commit e073e5e5f72c146897773aaa12d342e2dc1aae23
parent 9d80fe0a9347a40327b6cbbf1f2cce5b1f7e9198
Author: cfillion <cfillion@users.noreply.github.com>
Date: Tue, 8 Mar 2016 02:08:55 -0500
drop .ReaPackRemote files, always import from the index URL
Diffstat:
17 files changed, 164 insertions(+), 242 deletions(-)
diff --git a/src/import.cpp b/src/import.cpp
@@ -19,13 +19,12 @@
#include "download.hpp"
#include "encoding.hpp"
+#include "errors.hpp"
+#include "index.hpp"
#include "reapack.hpp"
#include "remote.hpp"
#include "resource.hpp"
-#include <cerrno>
-#include <sstream>
-
#include <reaper_plugin_functions.h>
#ifndef _WIN32 // for SWELL
@@ -35,7 +34,6 @@
using namespace std;
const char *Import::TITLE = "ReaPack: Import a repository";
-const char *SUBTITLE = "Select a .ReaPackRemote file";
Import::Import(ReaPack *reapack)
: Dialog(IDD_IMPORT_DIALOG), m_reapack(reapack), m_download(nullptr)
@@ -45,10 +43,8 @@ Import::Import(ReaPack *reapack)
void Import::onInit()
{
SetWindowTextA(handle(), TITLE);
- SetWindowTextA(getControl(IDC_GROUPBOX), SUBTITLE);
m_url = getControl(IDC_URL);
- m_file = getControl(IDC_FILE);
m_progress = getControl(IDC_PROGRESS);
m_ok = getControl(IDOK);
@@ -62,11 +58,8 @@ void Import::onInit()
void Import::onCommand(const int id)
{
switch(id) {
- case IDC_BROWSE:
- browseFile();
- break;
case IDOK:
- import();
+ fetch();
break;
case IDCANCEL:
if(m_download)
@@ -85,77 +78,34 @@ void Import::onTimer(int)
#endif
}
-void Import::browseFile()
+void Import::fetch()
{
- string path(4096, 0);
- if(!GetUserFileNameForRead(&path[0], SUBTITLE, "ReaPackRemote")) {
- SetFocus(handle());
- return;
- }
-
- SetWindowText(m_file, make_autostring(path).c_str());
- import(true);
-}
+ if(m_download)
+ return;
-void Import::import(const bool fileOnly)
-{
- auto_string input(4096, 0);
+ auto_string url(4096, 0);
+ GetWindowText(m_url, &url[0], (int)url.size());
- if(!fileOnly) {
- GetWindowText(m_url, &input[0], (int)input.size());
+ const size_t end = url.find(AUTO_STR('\0'));
- if(input[0] != 0) {
- download(from_autostring(input));
- return;
- }
- }
-
- GetWindowText(m_file, &input[0], (int)input.size());
-
- if(input[0] == 0) {
+ if(end == 0) { // url is empty
close();
return;
}
-
- Remote::ReadCode code;
- if(!import(Remote::fromFile(from_autostring(input), &code), code))
- SetFocus(m_file);
-}
-
-bool Import::import(const Remote &remote, const Remote::ReadCode code)
-{
- switch(code) {
- case Remote::ReadFailure:
- ShowMessageBox(strerror(errno), TITLE, 0);
- return false;
- case Remote::InvalidName:
- ShowMessageBox("Invalid .ReaPackRemote file! (invalid name)", TITLE, 0);
- return false;
- case Remote::InvalidUrl:
- ShowMessageBox("Invalid .ReaPackRemote file! (invalid url)", TITLE, 0);
- return false;
- default:
- break;
- };
-
- m_reapack->import(remote);
- close();
-
- return true;
-}
-
-void Import::download(const string &url)
-{
- if(m_download)
- return;
+ else {
+ // remove extra nulls from the string
+ url.resize(end);
+ }
setWaiting(true);
- Download *dl = new Download({}, url);
+ Download *dl = m_download = new Download({}, from_autostring(url));
+
dl->onFinish([=] {
const Download::State state = dl->state();
if(state == Download::Aborted) {
- // we are deleted here, so there is nothing much we can do without crashing
+ // at this point `this` is deleted, so there is nothing else
+ // we can do without crashing
return;
}
@@ -167,10 +117,7 @@ void Import::download(const string &url)
return;
}
- istringstream stream(dl->contents());
-
- Remote::ReadCode code;
- if(!import(Remote::fromFile(stream, &code), code))
+ if(!import())
SetFocus(m_url);
});
@@ -183,8 +130,25 @@ void Import::download(const string &url)
});
dl->start();
+}
+
+bool Import::import()
+{
+ assert(m_download);
+
+ try {
+ IndexPtr index = Index::load({}, m_download->contents().c_str());
+ m_reapack->import({index->name(), m_download->url()});
- m_download = dl;
+ close();
+
+ return true;
+ }
+ catch(const reapack_error &e) {
+ const string msg = "The received file is invalid: " + string(e.what());
+ ShowMessageBox(msg.c_str(), TITLE, MB_OK);
+ return false;
+ }
}
void Import::setWaiting(const bool wait)
diff --git a/src/import.hpp b/src/import.hpp
@@ -40,10 +40,8 @@ protected:
void onTimer(int) override;
private:
- void browseFile();
- void import(bool fileOnly = false);
- bool import(const Remote &remote, Remote::ReadCode code);
- void download(const std::string &url);
+ void fetch();
+ bool import();
void setWaiting(bool);
ReaPack *m_reapack;
@@ -51,7 +49,6 @@ private:
short m_fakePos;
HWND m_url;
- HWND m_file;
HWND m_progress;
HWND m_ok;
};
diff --git a/src/index.cpp b/src/index.cpp
@@ -43,19 +43,23 @@ auto Index::linkTypeFor(const char *rel) -> LinkType
return WebsiteLink;
}
-IndexPtr Index::load(const string &name)
+IndexPtr Index::load(const string &name, const char *data)
{
TiXmlDocument doc;
- FILE *file = FS::open(pathFor(name));
+ if(data)
+ doc.Parse(data);
+ else {
+ FILE *file = FS::open(pathFor(name));
- if(!file)
- throw reapack_error(FS::lastError().c_str());
+ if(!file)
+ throw reapack_error(FS::lastError().c_str());
- const bool success = doc.LoadFile(file);
- fclose(file);
+ doc.LoadFile(file);
+ fclose(file);
+ }
- if(!success)
+ if(doc.ErrorId())
throw reapack_error(doc.ErrorDesc());
TiXmlHandle docHandle(&doc);
@@ -68,14 +72,24 @@ IndexPtr Index::load(const string &name)
root->Attribute("version", &version);
if(!version)
- throw reapack_error("invalid version");
+ throw reapack_error("index version not found");
+
+ Index *ri = new Index(name);
+
+ // ensure the memory is released if an exception is
+ // thrown during the loading process
+ unique_ptr<Index> ptr(ri);
switch(version) {
case 1:
- return loadV1(root, name);
+ loadV1(root, ri);
+ break;
default:
- throw reapack_error("unsupported version");
+ throw reapack_error("index version is unsupported");
}
+
+ ptr.release();
+ return IndexPtr(ri);
}
Download *Index::fetch(const Remote &remote, const bool stale)
@@ -95,8 +109,6 @@ Download *Index::fetch(const Remote &remote, const bool stale)
Index::Index(const string &name)
: m_name(name)
{
- if(m_name.empty())
- throw reapack_error("empty index name");
}
Index::~Index()
@@ -105,6 +117,15 @@ Index::~Index()
delete cat;
}
+void Index::setName(const string &newName)
+{
+ if(!m_name.empty())
+ throw reapack_error("index name is already set");
+
+ // validation is taken care of later by Remote's constructor
+ m_name = newName;
+}
+
void Index::addCategory(const Category *cat)
{
if(cat->index() != this)
diff --git a/src/index.hpp b/src/index.hpp
@@ -46,12 +46,13 @@ public:
static Path pathFor(const std::string &name);
static LinkType linkTypeFor(const char *rel);
- static IndexPtr load(const std::string &name);
+ static IndexPtr load(const std::string &name, const char *data = 0);
static Download *fetch(const Remote &, bool stale = false);
Index(const std::string &name);
~Index();
+ void setName(const std::string &);
const std::string &name() const { return m_name; }
void setAboutText(const std::string &rtf) { m_about = rtf; }
@@ -66,7 +67,7 @@ public:
const PackageList &packages() const { return m_packages; }
private:
- static IndexPtr loadV1(TiXmlElement *, const std::string &);
+ static void loadV1(TiXmlElement *, Index *);
std::string m_name;
std::string m_about;
diff --git a/src/index_v1.cpp b/src/index_v1.cpp
@@ -28,14 +28,8 @@ static void LoadCategoryV1(TiXmlElement *, Index *ri);
static void LoadPackageV1(TiXmlElement *, Category *cat);
static void LoadVersionV1(TiXmlElement *, Package *pkg);
-IndexPtr Index::loadV1(TiXmlElement *root, const string &name)
+void Index::loadV1(TiXmlElement *root, Index *ri)
{
- Index *ri = new Index(name);
-
- // ensure the memory is released if an exception is
- // thrown during the loading process
- unique_ptr<Index> ptr(ri);
-
TiXmlElement *node = root->FirstChildElement("category");
while(node) {
@@ -48,9 +42,6 @@ IndexPtr Index::loadV1(TiXmlElement *root, const string &name)
if(node)
LoadMetadataV1(node, ri);
-
- ptr.release();
- return IndexPtr(ri);
}
void LoadMetadataV1(TiXmlElement *meta, Index *ri)
@@ -58,12 +49,17 @@ void LoadMetadataV1(TiXmlElement *meta, Index *ri)
TiXmlElement *node = meta->FirstChildElement("description");
if(node) {
- const char *rtf = node->GetText();
-
- if(rtf)
+ if(const char *rtf = node->GetText())
ri->setAboutText(rtf);
}
+ node = meta->FirstChildElement("name");
+
+ if(node && ri->name().empty()) {
+ if(const char *name = node->GetText())
+ ri->setName(name);
+ }
+
node = meta->FirstChildElement("link");
while(node) {
@@ -163,9 +159,7 @@ void LoadVersionV1(TiXmlElement *verNode, Package *pkg)
node = verNode->FirstChildElement("changelog");
if(node) {
- const char *changelog = node->GetText();
-
- if(changelog)
+ if(const char *changelog = node->GetText())
ver->setChangelog(changelog);
}
diff --git a/src/remote.cpp b/src/remote.cpp
@@ -20,7 +20,6 @@
#include "encoding.hpp"
#include "errors.hpp"
-#include <fstream>
#include <regex>
#include <sstream>
@@ -54,47 +53,6 @@ static bool ValidateUrl(const string &url)
return !match.empty();
}
-Remote Remote::fromFile(const string &path, ReadCode *code)
-{
- ifstream file(make_autostring(path));
-
- if(!file) {
- if(code)
- *code = ReadFailure;
-
- return {};
- }
-
- return fromFile(file, code);
-}
-
-Remote Remote::fromFile(istream &stream, ReadCode *code)
-{
- string name;
- getline(stream, name);
-
- string url;
- getline(stream, url);
-
- if(!ValidateName(name)) {
- if(code)
- *code = InvalidName;
-
- return {};
- }
- else if(!ValidateUrl(url)) {
- if(code)
- *code = InvalidUrl;
-
- return {};
- }
-
- if(code)
- *code = Success;
-
- return {name, url};
-}
-
Remote Remote::fromString(const string &data, ReadCode *code)
{
istringstream stream(data);
diff --git a/src/remote.hpp b/src/remote.hpp
@@ -18,7 +18,6 @@
#ifndef REAPACK_REMOTE_HPP
#define REAPACK_REMOTE_HPP
-#include <istream>
#include <map>
#include <string>
#include <vector>
@@ -32,8 +31,6 @@ public:
InvalidUrl,
};
- static Remote fromFile(const std::string &path, ReadCode *code = nullptr);
- static Remote fromFile(std::istream &, ReadCode *code = nullptr);
static Remote fromString(const std::string &data, ReadCode *code = nullptr);
Remote();
diff --git a/src/resource.hpp b/src/resource.hpp
@@ -54,7 +54,5 @@
#define IDC_PACKAGES 220
#define IDC_GROUPBOX 221
#define IDC_URL 222
-#define IDC_FILE 223
-#define IDC_BROWSE 224
#endif
diff --git a/src/resource.rc b/src/resource.rc
@@ -62,18 +62,16 @@ BEGIN
DEFPUSHBUTTON "&Close", IDOK, 409, 250, 45, 14
END
-IDD_IMPORT_DIALOG DIALOGEX 0, 0, 290, 90
+IDD_IMPORT_DIALOG DIALOGEX 0, 0, 290, 70
STYLE DIALOG_STYLE
FONT DIALOG_FONT
BEGIN
- GROUPBOX "", IDC_GROUPBOX, 2, 4, 287, 60
- RTEXT "From internet:", IDC_LABEL, 5, 19, 50, 10
+ GROUPBOX "Repository settings", IDC_GROUPBOX, 2, 4, 287, 41
+ RTEXT "Index URL:", IDC_LABEL, 5, 19, 50, 10
EDITTEXT IDC_URL, 60, 16, 220, 14, ES_AUTOHSCROLL
- CTEXT "- or -", IDC_LABEL2, 0, 34, 290, 10
- RTEXT "From a file:", IDC_LABEL3, 5, 48, 50, 10
- EDITTEXT IDC_FILE, 60, 44, 177, 14, ES_AUTOHSCROLL
- PUSHBUTTON "&Browse...", IDC_BROWSE, 240, 44, 40, 14
- CONTROL "", IDC_PROGRESS, PROGRESS_CLASS, PBS_MARQUEE, 10, 74, 150, 5
- DEFPUSHBUTTON "&OK", IDOK, 200, 70, 40, 14
- PUSHBUTTON "&Cancel", IDCANCEL, 244, 70, 40, 14
+ LTEXT "Type or paste the URL to a repository index in the box above.",
+ IDC_LABEL2, 60, 33, 220, 10
+ CONTROL "", IDC_PROGRESS, PROGRESS_CLASS, PBS_MARQUEE, 10, 54, 150, 5
+ DEFPUSHBUTTON "&OK", IDOK, 200, 50, 40, 14
+ PUSHBUTTON "&Cancel", IDCANCEL, 244, 50, 40, 14
END
diff --git a/test/index.cpp b/test/index.cpp
@@ -11,7 +11,7 @@ using namespace std;
static const char *M = "[index]";
-TEST_CASE("file not found", M) {
+TEST_CASE("index file not found", M) {
UseRootPath root(RIPATH);
try {
@@ -23,7 +23,23 @@ TEST_CASE("file not found", M) {
}
}
-TEST_CASE("broken", M) {
+TEST_CASE("load index from raw data", M) {
+ SECTION("valid") {
+ Index::load("", "<index version=\"1\"/>\n");
+ }
+
+ SECTION("broken") {
+ try {
+ Index::load("", "<index>\n");
+ FAIL();
+ }
+ catch(const reapack_error &e) {
+ REQUIRE(string(e.what()) == "Error reading end tag.");
+ }
+ }
+}
+
+TEST_CASE("broken index", M) {
UseRootPath root(RIPATH);
try {
@@ -55,7 +71,7 @@ TEST_CASE("invalid version", M) {
FAIL();
}
catch(const reapack_error &e) {
- REQUIRE(string(e.what()) == "invalid version");
+ REQUIRE(string(e.what()) == "index version not found");
}
}
@@ -67,7 +83,7 @@ TEST_CASE("future version", M) {
FAIL();
}
catch(const reapack_error &e) {
- REQUIRE(string(e.what()) == "unsupported version");
+ REQUIRE(string(e.what()) == "index version is unsupported");
}
}
@@ -78,16 +94,6 @@ TEST_CASE("unicode index path", M) {
REQUIRE(ri->name() == "Новая папка");
}
-TEST_CASE("empty index name", M) {
- try {
- Index idx({});
- FAIL();
- }
- catch(const reapack_error &e) {
- REQUIRE(string(e.what()) == "empty index name");
- }
-}
-
TEST_CASE("add category", M) {
Index ri("a");
Category *cat = new Category("a", &ri);
@@ -231,6 +237,26 @@ TEST_CASE("repository links", M) {
}
}
+TEST_CASE("set index name", M) {
+ SECTION("set") {
+ Index ri({});
+ ri.setName("Hello/World!");
+ REQUIRE(ri.name() == "Hello/World!");
+ }
+
+ SECTION("override") {
+ Index ri("hello");
+ try {
+ ri.setName("world");
+ FAIL();
+ }
+ catch(const reapack_error &e) {
+ REQUIRE(string(e.what()) == "index name is already set");
+ }
+ REQUIRE(ri.name() == "hello");
+ }
+}
+
TEST_CASE("link type from string", M) {
REQUIRE(Index::linkTypeFor("website") == Index::WebsiteLink);
REQUIRE(Index::linkTypeFor("donation") == Index::DonationLink);
diff --git a/test/index_v1.cpp b/test/index_v1.cpp
@@ -181,6 +181,10 @@ TEST_CASE("read index metadata", M) {
UseRootPath root(RIPATH);
IndexPtr ri = Index::load("metadata");
+
+ SECTION("name (ignored)") {
+ REQUIRE(ri->name() == "metadata");
+ }
SECTION("description") {
REQUIRE(ri->aboutText() == "Chunky\nBacon");
@@ -206,3 +210,29 @@ TEST_CASE("read index metadata", M) {
REQUIRE(links[0]->url == "http://paypal.com");
}
}
+
+TEST_CASE("read index name (from raw data only)") {
+ SECTION("valid") {
+ IndexPtr ri = Index::load({}, R"XML(
+ <index version="1">
+ <metadata>
+ <name>Hello World</name>
+ </metadata>
+ </index>
+ )XML");
+
+ REQUIRE(ri->name() == "Hello World");
+ }
+
+ SECTION("broken") {
+ IndexPtr ri = Index::load({}, R"XML(
+ <index version="1">
+ <metadata>
+ <name/>
+ </metadata>
+ </index>
+ )XML");
+
+ REQUIRE(ri->name() == "");
+ }
+}
diff --git a/test/indexes/v1/ReaPack/metadata.xml b/test/indexes/v1/ReaPack/metadata.xml
@@ -1,5 +1,6 @@
<index version="1">
<metadata>
+ <name>This Is Ignored Unless When Importing From URL</name>
<link rel="website" href="http://cfillion.tk" />
<link rel="website" href="https://github.com/cfillion"></link>
<link rel="website">http://twitter.com/cfi30</link>
diff --git a/test/remote.cpp b/test/remote.cpp
@@ -183,62 +183,6 @@ TEST_CASE("get remote by name", M) {
REQUIRE_FALSE(list.get("hello").isNull());
}
-TEST_CASE("remote from file", M) {
- Remote remote;
-
- SECTION("not found") {
- Remote::ReadCode code;
- const Remote remote = Remote::fromFile(RPATH "404.ReaPackRemote", &code);
- REQUIRE(code == Remote::ReadFailure);
- REQUIRE_FALSE(remote.isValid());
- }
-
- SECTION("valid") {
- Remote::ReadCode code;
- const Remote remote = Remote::fromFile(RPATH "test.ReaPackRemote", &code);
- REQUIRE(code == Remote::Success);
- REQUIRE(remote.isValid());
- REQUIRE(remote.name() == "name");
- REQUIRE(remote.url() == "url");
- }
-
- SECTION("invalid name") {
- Remote::ReadCode code;
- const Remote remote = Remote::fromFile(
- RPATH "invalid_name.ReaPackRemote", &code);
-
- REQUIRE(code == Remote::InvalidName);
- REQUIRE_FALSE(remote.isValid());
- }
-
- SECTION("invalid url") {
- Remote::ReadCode code;
- const Remote remote = Remote::fromFile(
- RPATH "invalid_url.ReaPackRemote", &code);
-
- REQUIRE(code == Remote::InvalidUrl);
- REQUIRE_FALSE(remote.isValid());
- }
-
- SECTION("unicode name") {
- Remote::ReadCode code;
- Remote::fromFile(RPATH "Новая папка.ReaPackRemote", &code);
-
- REQUIRE(code == Remote::Success);
- }
-
- SECTION("string stream") {
- istringstream stream("name\nurl");
-
- Remote::ReadCode code;
- const Remote remote = Remote::fromFile(stream, &code);
- REQUIRE(code == Remote::Success);
- REQUIRE(remote.isValid());
- REQUIRE(remote.name() == "name");
- REQUIRE(remote.url() == "url");
- }
-};
-
TEST_CASE("unserialize remote", M) {
SECTION("invalid name") {
Remote::ReadCode code;
diff --git a/test/remote/invalid_name.ReaPackRemote b/test/remote/invalid_name.ReaPackRemote
@@ -1,2 +0,0 @@
-whatever &|%{}
-url
diff --git a/test/remote/invalid_url.ReaPackRemote b/test/remote/invalid_url.ReaPackRemote
@@ -1 +0,0 @@
-hello world
diff --git a/test/remote/test.ReaPackRemote b/test/remote/test.ReaPackRemote
@@ -1,2 +0,0 @@
-name
-url
diff --git a/test/remote/Новая папка.ReaPackRemote b/test/remote/Новая папка.ReaPackRemote
@@ -1,2 +0,0 @@
-name
-url
