Return Status from import methods

mikee47 · mikee47 · commit 62fe6f9ba30d · 2024-08-27T20:04:28.000+01:00
diff --git a/src/Database.cpp b/src/Database.cpp
@@ -196,20 +196,20 @@ bool Database::exportToFile(const Format& format, const String& filename)
 	}
 
 	if(stream.getLastError() == FS_OK) {
-		debug_d("[CFGDB] Database saved '%s' OK", filename.c_str());
+		debug_d("[CFGDB] Database saved to '%s'", filename.c_str());
 		return true;
 	}
 
-	debug_e("[CFGDB] Database save '%s' failed: %s", filename.c_str(), stream.getLastErrorString().c_str());
+	debug_e("[CFGDB] Database save to '%s' failed: %s", filename.c_str(), stream.getLastErrorString().c_str());
 	return false;
 }
 
-bool Database::importFromFile(const Format& format, const String& filename)
+Status Database::importFromFile(const Format& format, const String& filename)
 {
 	FileStream stream;
 	if(!stream.open(filename, File::ReadOnly)) {
-		debug_w("open('%s') failed: %s", filename.c_str(), stream.getLastErrorString().c_str());
-		return false;
+		debug_w("[CFGDB] open '%s' failed: %s", filename.c_str(), stream.getLastErrorString().c_str());
+		return {Result::fileError, stream.getLastError()};
 	}
 
 	return format.importFromStream(*this, stream);
diff --git a/src/Json/Format.cpp b/src/Json/Format.cpp
@@ -61,28 +61,22 @@ std::unique_ptr<ImportStream> Format::createImportStream(std::shared_ptr<Store>
 	return std::make_unique<WriteStream>(store, object);
 }
 
-bool Format::importFromStream(Object& object, Stream& source) const
+Status Format::importFromStream(Object& object, Stream& source) const
 {
 	auto status = WriteStream::parse(object, source);
-
-	if(status == JSON::Status::EndOfDocument) {
-		return true;
+	if(!status) {
+		debug_w("JSON load '%s': %s", object.getName().c_str(), toString(status).c_str());
 	}
-
-	debug_w("JSON load '%s': %s", object.getName().c_str(), toString(status).c_str());
-	return false;
+	return status;
 }
 
-bool Format::importFromStream(Database& database, Stream& source) const
+Status Format::importFromStream(Database& database, Stream& source) const
 {
 	auto status = WriteStream::parse(database, source);
-
-	if(status == JSON::Status::EndOfDocument) {
-		return true;
+	if(!status) {
+		debug_w("JSON load '%s': %s", database.getName().c_str(), toString(status).c_str());
 	}
-
-	debug_w("JSON load '%s': %s", database.getName().c_str(), toString(status).c_str());
-	return false;
+	return status;
 }
 
 } // namespace ConfigDB::Json
diff --git a/src/Json/WriteStream.cpp b/src/Json/WriteStream.cpp
@@ -30,14 +30,18 @@ WriteStream::~WriteStream()
 	}
 }
 
-JSON::Status WriteStream::parse(Database& database, Stream& source)
+Status WriteStream::parse(Database& database, Stream& source)
 {
-	return WriteStream(database).parser.parse(source);
+	WriteStream writer(database);
+	writer.parser.parse(source);
+	return writer.status;
 }
 
-JSON::Status WriteStream::parse(Object& object, Stream& source)
+Status WriteStream::parse(Object& object, Stream& source)
 {
-	return WriteStream(object).parser.parse(source);
+	WriteStream writer(object);
+	writer.parser.parse(source);
+	return writer.status;
 }
 
 bool WriteStream::startElement(const JSON::Element& element)
@@ -46,18 +50,21 @@ bool WriteStream::startElement(const JSON::Element& element)
 		return true;
 	}
 
-	auto notInSchema = [&element]() -> bool {
-		debug_e("[JSON] '%s' not in schema", element.key);
+	auto notInSchema = [&]() -> bool {
+		debug_w("[CFGDB] '%s' not in schema", element.key);
+		status.result = Result::formatError;
 		return false;
 	};
 
-	auto arrayExpected = [&element]() -> bool {
-		debug_w("[JSON] '%s' not an array", element.key);
+	auto arrayExpected = [&]() -> bool {
+		debug_w("[CFGDB] '%s' not an array", element.key);
+		status.result = Result::formatError;
 		return false;
 	};
 
-	auto badSelector = [&element]() -> bool {
-		debug_w("[JSON] '%s' bad selector", element.key);
+	auto badSelector = [&]() -> bool {
+		debug_w("[CFGDB] '%s' bad selector", element.key);
+		status.result = Result::formatError;
 		return false;
 	};
 
@@ -66,7 +73,11 @@ bool WriteStream::startElement(const JSON::Element& element)
 			return notInSchema();
 		}
 		const char* value = (element.type == JSON::Element::Type::Null) ? nullptr : element.value;
-		return prop.setJsonValue(value, element.valueLength);
+		if(!prop.setJsonValue(value, element.valueLength)) {
+			status.result = Result::formatError;
+			return false;
+		}
+		return true;
 	};
 
 	if(db && element.level == 1) {
@@ -77,7 +88,7 @@ bool WriteStream::startElement(const JSON::Element& element)
 			store.reset();
 			store = db->openStore(root, true);
 			if(!store || !*store) {
-				// Fatal: store is locked
+				status.result = Result::updateConflict;
 				return false;
 			}
 			info[0] = *store;
@@ -97,7 +108,7 @@ bool WriteStream::startElement(const JSON::Element& element)
 		auto& type = *db->typeinfo.stores[i];
 		store = db->openStore(type, true);
 		if(!store || !*store) {
-			// Fatal: store is locked
+			status.result = Result::updateConflict;
 			return false;
 		}
 		info[1] = Object(*store);
@@ -247,10 +258,9 @@ size_t WriteStream::write(const uint8_t* data, size_t size)
 
 	auto jsonStatus = parser.parse(reinterpret_cast<const char*>(data), size);
 	switch(jsonStatus) {
+	case JSON::Status::Ok:
 	case JSON::Status::EndOfDocument:
-		break;
 	case JSON::Status::Cancelled:
-		status.result = Result::updateConflict;
 		break;
 	default:
 		status.result = Result::formatError;
diff --git a/src/Json/WriteStream.h b/src/Json/WriteStream.h
@@ -48,9 +48,9 @@ class WriteStream : public ImportStream, private JSON::Listener
 
 	~WriteStream();
 
-	static JSON::Status parse(Database& db, Stream& source);
+	static Status parse(Database& db, Stream& source);
 
-	static JSON::Status parse(Object& object, Stream& source);
+	static Status parse(Object& object, Stream& source);
 
 	bool isValid() const override
 	{
diff --git a/src/Object.cpp b/src/Object.cpp
@@ -271,23 +271,23 @@ bool Object::exportToFile(const Format& format, const String& filename) const
 	}
 
 	if(stream.getLastError() == FS_OK) {
-		debug_d("[JSON] Store saved '%s' OK", filename.c_str());
+		debug_d("[CFGDB] Object saved to '%s'", filename.c_str());
 		return true;
 	}
 
-	debug_e("[JSON] Store save '%s' failed: %s", filename.c_str(), stream.getLastErrorString().c_str());
+	debug_e("[CFGDB] Object save to '%s' failed: %s", filename.c_str(), stream.getLastErrorString().c_str());
 	return false;
 }
 
-bool Object::importFromFile(const Format& format, const String& filename)
+Status Object::importFromFile(const Format& format, const String& filename)
 {
 	FileStream stream;
 	if(!stream.open(filename, File::ReadOnly)) {
 		if(stream.getLastError() == IFS::Error::NotFound) {
-			return true;
+			return {};
 		}
-		debug_w("open('%s') failed", filename.c_str());
-		return false;
+		debug_w("[CFGDB] open '%s' failed", filename.c_str());
+		return {Result::fileError, stream.getLastError()};
 	}
 
 	return format.importFromStream(*this, stream);
diff --git a/src/include/ConfigDB/Database.h b/src/include/ConfigDB/Database.h
@@ -92,12 +92,12 @@ class Database
 
 	bool exportToFile(const Format& format, const String& filename);
 
-	bool importFromStream(const Format& format, Stream& source)
+	Status importFromStream(const Format& format, Stream& source)
 	{
 		return format.importFromStream(*this, source);
 	}
 
-	bool importFromFile(const Format& format, const String& filename);
+	Status importFromFile(const Format& format, const String& filename);
 
 	std::unique_ptr<ImportStream> createImportStream(const Format& format)
 	{
diff --git a/src/include/ConfigDB/Format.h b/src/include/ConfigDB/Format.h
@@ -60,6 +60,11 @@ struct Status {
 			return nullptr;
 		}
 	}
+
+	size_t printTo(Print& p) const
+	{
+		return p.print(toString());
+	}
 };
 
 class ImportStream : public ReadWriteStream
@@ -126,14 +131,14 @@ class Format
 	/**
 	 * @brief De-serialise content from stream into object (RAM)
 	 */
-	virtual bool importFromStream(Object& object, Stream& source) const = 0;
+	virtual Status importFromStream(Object& object, Stream& source) const = 0;
 
 	/**
 	 * @brief De-serialise content from stream into database
 	 * Each store is overwritten as it is loadded.
 	 * If a store entry is not represented in the data then it is left untouched.
 	 */
-	virtual bool importFromStream(Database& database, Stream& source) const = 0;
+	virtual Status importFromStream(Database& database, Stream& source) const = 0;
 
 	/**
 	 * @brief Get the standard file extension for the reader implementation
diff --git a/src/include/ConfigDB/Json/Format.h b/src/include/ConfigDB/Json/Format.h
@@ -34,8 +34,8 @@ class Format : public ConfigDB::Format
 	size_t exportToStream(Database& database, Print& output) const override;
 	std::unique_ptr<ImportStream> createImportStream(Database& db) const override;
 	std::unique_ptr<ImportStream> createImportStream(std::shared_ptr<Store> store, Object& object) const override;
-	bool importFromStream(Object& object, Stream& source) const override;
-	bool importFromStream(Database& database, Stream& source) const override;
+	Status importFromStream(Object& object, Stream& source) const override;
+	Status importFromStream(Database& database, Stream& source) const override;
 
 	String getFileExtension() const override
 	{
diff --git a/src/include/ConfigDB/Network/HttpImportResource.h b/src/include/ConfigDB/Network/HttpImportResource.h
@@ -79,6 +79,8 @@ class HttpImportResource : public HttpResource
 			case Result::ok:
 				break;
 			case Result::formatError:
+				response.code = HTTP_STATUS_BAD_REQUEST;
+				break;
 			case Result::updateConflict:
 				response.code = HTTP_STATUS_CONFLICT;
 				break;
diff --git a/src/include/ConfigDB/Object.h b/src/include/ConfigDB/Object.h
@@ -160,12 +160,12 @@ class Object
 
 	bool exportToFile(const Format& format, const String& filename) const;
 
-	bool importFromStream(const Format& format, Stream& source)
+	Status importFromStream(const Format& format, Stream& source)
 	{
 		return format.importFromStream(*this, source);
 	}
 
-	bool importFromFile(const Format& format, const String& filename);
+	Status importFromFile(const Format& format, const String& filename);
 
 	const ObjectInfo& typeinfo() const
 	{
diff --git a/src/include/ConfigDB/Store.h b/src/include/ConfigDB/Store.h
@@ -164,15 +164,15 @@ class Store : public Object
 		return exportToFile(format, filename);
 	}
 
-	bool importFromFile(const Format& format, const String& filename)
+	Status importFromFile(const Format& format, const String& filename)
 	{
 		incUpdate();
-		bool result = Object::importFromFile(format, filename);
+		auto result = Object::importFromFile(format, filename);
 		decUpdate();
 		return result;
 	}
 
-	bool importFromFile(const Format& format)
+	Status importFromFile(const Format& format)
 	{
 		String filename = getFilePath() + format.getFileExtension();
 		return importFromFile(format, filename);
diff --git a/test/modules/Streaming.cpp b/test/modules/Streaming.cpp
@@ -127,7 +127,8 @@ class StreamingTest : public TestGroup
 	{
 		MemoryDataStream mem;
 		mem << data;
-		return object.importFromStream(ConfigDB::Json::format, mem);
+		auto status = object.importFromStream(ConfigDB::Json::format, mem);
+		return bool(status);
 	}
 
 	template <typename T> String exportObject(T& object)

Original file line number	Diff line number	Diff line change
`@@ -196,20 +196,20 @@ bool Database::exportToFile(const Format& format, const String& filename)`
`196`	`196`	`}`
`197`	`197`
`198`	`198`	`if(stream.getLastError() == FS_OK) {`
`199`		`- debug_d("[CFGDB] Database saved '%s' OK", filename.c_str());`
	`199`	`+ debug_d("[CFGDB] Database saved to '%s'", filename.c_str());`
`200`	`200`	`return true;`
`201`	`201`	`}`
`202`	`202`
`203`		`- debug_e("[CFGDB] Database save '%s' failed: %s", filename.c_str(), stream.getLastErrorString().c_str());`
	`203`	`+ debug_e("[CFGDB] Database save to '%s' failed: %s", filename.c_str(), stream.getLastErrorString().c_str());`
`204`	`204`	`return false;`
`205`	`205`	`}`
`206`	`206`
`207`		`-bool Database::importFromFile(const Format& format, const String& filename)`
	`207`	`+Status Database::importFromFile(const Format& format, const String& filename)`
`208`	`208`	`{`
`209`	`209`	`FileStream stream;`
`210`	`210`	`if(!stream.open(filename, File::ReadOnly)) {`
`211`		`- debug_w("open('%s') failed: %s", filename.c_str(), stream.getLastErrorString().c_str());`
`212`		`- return false;`
	`211`	`+ debug_w("[CFGDB] open '%s' failed: %s", filename.c_str(), stream.getLastErrorString().c_str());`
	`212`	`+ return {Result::fileError, stream.getLastError()};`
`213`	`213`	`}`
`214`	`214`
`215`	`215`	`return format.importFromStream(*this, stream);`
Original file line number	Diff line number	Diff line change
`@@ -61,28 +61,22 @@ std::unique_ptr<ImportStream> Format::createImportStream(std::shared_ptr<Store>`
`61`	`61`	`return std::make_unique<WriteStream>(store, object);`
`62`	`62`	`}`
`63`	`63`
`64`		`-bool Format::importFromStream(Object& object, Stream& source) const`
	`64`	`+Status Format::importFromStream(Object& object, Stream& source) const`
`65`	`65`	`{`
`66`	`66`	`auto status = WriteStream::parse(object, source);`
`67`		`-`
`68`		`- if(status == JSON::Status::EndOfDocument) {`
`69`		`- return true;`
	`67`	`+ if(!status) {`
	`68`	`+ debug_w("JSON load '%s': %s", object.getName().c_str(), toString(status).c_str());`
`70`	`69`	`}`
`71`		`-`
`72`		`- debug_w("JSON load '%s': %s", object.getName().c_str(), toString(status).c_str());`
`73`		`- return false;`
	`70`	`+ return status;`
`74`	`71`	`}`
`75`	`72`
`76`		`-bool Format::importFromStream(Database& database, Stream& source) const`
	`73`	`+Status Format::importFromStream(Database& database, Stream& source) const`
`77`	`74`	`{`
`78`	`75`	`auto status = WriteStream::parse(database, source);`
`79`		`-`
`80`		`- if(status == JSON::Status::EndOfDocument) {`
`81`		`- return true;`
	`76`	`+ if(!status) {`
	`77`	`+ debug_w("JSON load '%s': %s", database.getName().c_str(), toString(status).c_str());`
`82`	`78`	`}`
`83`		`-`
`84`		`- debug_w("JSON load '%s': %s", database.getName().c_str(), toString(status).c_str());`
`85`		`- return false;`
	`79`	`+ return status;`
`86`	`80`	`}`
`87`	`81`
`88`	`82`	`} // namespace ConfigDB::Json`
Original file line number	Diff line number	Diff line change
`@@ -48,9 +48,9 @@ class WriteStream : public ImportStream, private JSON::Listener`
`48`	`48`
`49`	`49`	`~WriteStream();`
`50`	`50`
`51`		`- static JSON::Status parse(Database& db, Stream& source);`
	`51`	`+ static Status parse(Database& db, Stream& source);`
`52`	`52`
`53`		`- static JSON::Status parse(Object& object, Stream& source);`
	`53`	`+ static Status parse(Object& object, Stream& source);`
`54`	`54`
`55`	`55`	`bool isValid() const override`
`56`	`56`	`{`
Original file line number	Diff line number	Diff line change
`@@ -271,23 +271,23 @@ bool Object::exportToFile(const Format& format, const String& filename) const`
`271`	`271`	`}`
`272`	`272`
`273`	`273`	`if(stream.getLastError() == FS_OK) {`
`274`		`- debug_d("[JSON] Store saved '%s' OK", filename.c_str());`
	`274`	`+ debug_d("[CFGDB] Object saved to '%s'", filename.c_str());`
`275`	`275`	`return true;`
`276`	`276`	`}`
`277`	`277`
`278`		`- debug_e("[JSON] Store save '%s' failed: %s", filename.c_str(), stream.getLastErrorString().c_str());`
	`278`	`+ debug_e("[CFGDB] Object save to '%s' failed: %s", filename.c_str(), stream.getLastErrorString().c_str());`
`279`	`279`	`return false;`
`280`	`280`	`}`
`281`	`281`
`282`		`-bool Object::importFromFile(const Format& format, const String& filename)`
	`282`	`+Status Object::importFromFile(const Format& format, const String& filename)`
`283`	`283`	`{`
`284`	`284`	`FileStream stream;`
`285`	`285`	`if(!stream.open(filename, File::ReadOnly)) {`
`286`	`286`	`if(stream.getLastError() == IFS::Error::NotFound) {`
`287`		`- return true;`
	`287`	`+ return {};`
`288`	`288`	`}`
`289`		`- debug_w("open('%s') failed", filename.c_str());`
`290`		`- return false;`
	`289`	`+ debug_w("[CFGDB] open '%s' failed", filename.c_str());`
	`290`	`+ return {Result::fileError, stream.getLastError()};`
`291`	`291`	`}`
`292`	`292`
`293`	`293`	`return format.importFromStream(*this, stream);`
Original file line number	Diff line number	Diff line change
`@@ -92,12 +92,12 @@ class Database`
`92`	`92`
`93`	`93`	`bool exportToFile(const Format& format, const String& filename);`
`94`	`94`
`95`		`- bool importFromStream(const Format& format, Stream& source)`
	`95`	`+ Status importFromStream(const Format& format, Stream& source)`
`96`	`96`	`{`
`97`	`97`	`return format.importFromStream(*this, source);`
`98`	`98`	`}`
`99`	`99`
`100`		`- bool importFromFile(const Format& format, const String& filename);`
	`100`	`+ Status importFromFile(const Format& format, const String& filename);`
`101`	`101`
`102`	`102`	`std::unique_ptr<ImportStream> createImportStream(const Format& format)`
`103`	`103`	`{`
Original file line number	Diff line number	Diff line change
`@@ -160,12 +160,12 @@ class Object`
`160`	`160`
`161`	`161`	`bool exportToFile(const Format& format, const String& filename) const;`
`162`	`162`
`163`		`- bool importFromStream(const Format& format, Stream& source)`
	`163`	`+ Status importFromStream(const Format& format, Stream& source)`
`164`	`164`	`{`
`165`	`165`	`return format.importFromStream(*this, source);`
`166`	`166`	`}`
`167`	`167`
`168`		`- bool importFromFile(const Format& format, const String& filename);`
	`168`	`+ Status importFromFile(const Format& format, const String& filename);`
`169`	`169`
`170`	`170`	`const ObjectInfo& typeinfo() const`
`171`	`171`	`{`