Merge lp:~guillaume-chereau/stellarium/use-qt-json into lp:stellarium

[Guillaume Chereau]

Use the Qt 5 Json parser instead of the hand-made one we used so far.

This should be OK since the compilation requires qt 5 now.

[Fabien Chéreau (xalioth)]

did you try to build all plugins? Isn't anything using the iterator?

[Guillaume Chereau (guillaume-chereau)]

Yes, except the LogBook plugin that doesn't compile for other reasons.

[Alexander Wolf (alexwolf)]

It builds, but will not work without changes in plugins

[Guillaume Chereau (guillaume-chereau)]

>It builds, but will not work without changes in plugins
What plugin would be affected? I tried to keep it backward compatible
with the current API, except that I removed the iterator interface that
is not used anywhere as far as I can tell.

[Alexander Wolf (alexwolf)]

It's interesting. Current branch cannot parse pulsars.json catalog.

[Alexander Wolf (alexwolf)]

Current parser cannot understand digits with plus. Why?

[Alexander Wolf (alexwolf)]

I've updated pulsars.json

review: Approve

[Alexander Wolf (alexwolf)]

Maybe we should avoid using StelJsonParser and switch to use of QJsonDocument?

[Fabien Chéreau (xalioth)]

In a second step yes. I started to work on that once, but it's used in many
places.

On Fri, Nov 18, 2016 at 12:22 PM, Alexander Wolf <email address hidden>
wrote:

> Maybe we should avoid using StelJsonParser and switch to use of
> QJsonDocument?
> --
> https://code.launchpad.net/~guillaume-chereau/stellarium/
> use-qt-json/+merge/311241
> You are subscribed to branch lp:stellarium.
>

=== modified file 'src/core/StelJsonParser.cpp'
--- src/core/StelJsonParser.cpp	2016-06-17 15:00:04 +0000
+++ src/core/StelJsonParser.cpp	2016-11-18 09:14:03 +0000
@@ -18,475 +18,34 @@
 
 #include "StelJsonParser.hpp"
 #include <QDebug>
-#include <QBuffer>
-#include <QDateTime>
+#include <QJsonDocument>
 #include <stdexcept>
-#include <stdio.h>
-
-class StelJsonParserInstance
-{
-public:
-	StelJsonParserInstance(QIODevice* ain)
-		: input(ain)
-		, nextChar()
-		, hasNextChar(false)
-	{
-		cur = buffer;
-		last = cur;
-	}
-	inline void skipJson();
-	inline bool tryReadChar(char c);
-	inline bool skipAndConsumeChar(char r);
-	QByteArray readString();
-	QVariant readOther();
-	QVariant parse();
-
-private:
-	QIODevice* input;
-	char nextChar;
-	bool hasNextChar;
-
-#define BUFSIZESTATIC 65536
-	char buffer[BUFSIZESTATIC];
-	char* last;
-	char* cur;
-
-	inline bool getNextFromBuffer(char *c)
-	{
-		if (cur==last)
-		{
-			if (cur==buffer-1) // End of file
-				return false;
-
-			last = buffer + input->read(buffer, BUFSIZESTATIC);
-			if (last==buffer)
-			{
-				cur = buffer-1;
-				last = cur;
-				return false;
-			}
-			cur = buffer;
-		}
-		*c = *cur;
-		++cur;
-		return true;
-	}
-
-	inline bool getChar(char* c)
-	{
-		if (hasNextChar)
-		{
-			hasNextChar=false;
-			*c = nextChar;
-			return true;
-		}
-		return getNextFromBuffer(c);
-	}
-
-	inline void ungetChar(char c)
-	{
-		nextChar=c;
-		hasNextChar=true;
-	}
-
-	inline void skipLine()
-	{
-		if (hasNextChar && nextChar=='\n')
-		{
-			hasNextChar=false;
-			return;
-		}
-		hasNextChar=false;
-		char c;
-		while (getNextFromBuffer(&c) && c!='\n')
-		{}
-	}
-
-	inline bool atEnd()
-	{
-		return cur==buffer-1;
-	}
-};
-
-void StelJsonParserInstance::skipJson()
-{
-	// There is a weakness in this code -- it will cause any standalone '/' to be absorbed.
-	char c;
-	while (getChar(&c))
-	{
-		switch (c)
-		{
-			case ' ':
-			case '\t':
-			case '\n':
-			case '\r':
-				break;
-			case '/':
-				{
-					if (!getChar(&c))
-						return;
-
-					if (c=='/')
-						skipLine();
-					else
-					{
-						// We have a problem, we removed a '/'.. This should never happen with properly formatted JSON though
-						throw std::runtime_error(qPrintable(QString("Unexpected '/%1' in the JSON content").arg(c)));
-					}
-				}
-				break;
-			default:
-				ungetChar(c);
-				return;
-		}
-	}
-}
-
-
-bool StelJsonParserInstance::tryReadChar(char c)
-{
-	char r;
-	if (!getChar(&r))
-		return false;
-
-	if (r == c)
-		return true;
-
-	ungetChar(r);
-	return false;
-}
-
-bool StelJsonParserInstance::skipAndConsumeChar(char r)
-{
-	char c;
-	while (getChar(&c))
-	{
-		switch (c)
-		{
-			case ' ':
-			case '\t':
-			case '\n':
-			case '\r':
-				break;
-			case '/':
-				{
-					if (!getChar(&c))
-						throw std::runtime_error(qPrintable(QString("Unexpected '/%1' in the JSON content").arg(c)));
-
-					if (c=='/')
-						skipLine();
-					else
-					{
-						// We have a problem, we removed a '/'.. This should never happen with properly formatted JSON though
-						throw std::runtime_error(qPrintable(QString("Unexpected '/%1' in the JSON content").arg(c)));
-					}
-				}
-				break;
-			default:
-				if (r==c)
-					return true;
-				ungetChar(c);
-				return false;
-		}
-	}
-	return false;
-}
-
-// Read a string without the initial "
-QByteArray StelJsonParserInstance::readString()
-{
-	QByteArray name;
-	char c;
-	while (getChar(&c))
-	{
-		switch (c)
-		{
-			case '"':
-				return name;
-				break;
-			case '\\':
-			{
-				bool gotChar=getChar(&c);
-				if (!gotChar) {qWarning() << "cannot read further, error?"; continue;}
-				if (c=='b') c='\b';
-				if (c=='f') c='\f';
-				if (c=='n') c='\n';
-				if (c=='r') c='\r';
-				if (c=='t') c='\t';
-				if (c=='u') {qWarning() << "don't support \\uxxxx char"; continue;}
-			}
-				break;
-			default:
-				name+=c;
-				break;
-		}
-	}
-	if (atEnd())
-		throw std::runtime_error(qPrintable(QString("End of file before end of string: "+name)));
-	throw std::runtime_error(qPrintable(QString("Read error before end of string: "+name)));
-}
-
-QVariant StelJsonParserInstance::readOther()
-{
-	QByteArray str;
-	char c;
-	while (getChar(&c))
-	{
-		if (c==' ' || c==',' || c=='\n' || c=='\r' || c==']' || c=='\t' || c=='}')
-		{
-			ungetChar(c);
-			break;
-		}
-		str+=c;
-	}
-	bool ok;
-	const int i = str.toInt(&ok);
-	if (ok)
-		return i;
-	const double d = str.toDouble(&ok);
-	if (ok)
-		return d;
-	if (str=="true")
-		return QVariant(true);
-	if (str=="false")
-		return QVariant(false);
-	if (str=="null")
-		return QVariant();
-	QDateTime dt = QDateTime::fromString(str, Qt::ISODate);
-	if (dt.isValid())
-		return QVariant(dt);
-
-	throw std::runtime_error(qPrintable(QString("Invalid JSON value: \"")+str+"\""));
-}
-
-// Parse the given input stream
-QVariant StelJsonParserInstance::parse()
-{
-	skipJson();
-
-	char r;
-	if (!getChar(&r))
-		return QVariant();
-
-	switch (r)
-	{
-		case '{':
-		{
-			// We've got an object (a tuple)
-			QVariantMap map;
-			if (skipAndConsumeChar('}'))
-				return map;
-			for (;;)
-			{
-				if (!skipAndConsumeChar('\"'))
-				{
-					char cc=0;
-					if (getChar(&cc))
-						throw std::runtime_error(qPrintable(QString("Expected '\"' at beginning of string, found: '%1' (ASCII %2)").arg(cc).arg((int)(cc))));
-				}
-				const QByteArray& ar = readString();
-				const QString& key = QString::fromUtf8(ar.constData(), ar.size());
-				if (!skipAndConsumeChar(':'))
-					throw std::runtime_error(qPrintable(QString("Expected ':' after a member name: ")+key));
-
-				skipJson();
-				map.insert(key, parse());
-				if (!skipAndConsumeChar(','))
-					break;
-			}
-			if (!skipAndConsumeChar('}'))
-				throw std::runtime_error("Expected '}' to close an object");
-			return map;
-		}
-		case '[':
-		{
-			// We've got an array (a vector)
-			QVariantList list;
-			if (skipAndConsumeChar(']'))
-				return list;
-
-			for (;;)
-			{
-				list.append(parse());
-				if (!skipAndConsumeChar(','))
-					break;
-			}
-
-			if (!skipAndConsumeChar(']'))
-				throw std::runtime_error("Expected ']' to close an array");
-
-			return list;
-		}
-		case '\"':
-		{
-			// We've got a string
-			const QByteArray& ar = readString();
-			return QString::fromUtf8(ar.constData(), ar.size());
-		}
-		default:
-		{
-			ungetChar(r);
-			return readOther();
-		}
-	}
-}
-
-QHash<int, void (*)(const QVariant&, QIODevice*, int)> StelJsonParser::otherSerializer;
-
-// Serialize the passed QVariant as JSON into the output QIODevice
+
 void StelJsonParser::write(const QVariant& v, QIODevice* output, int indentLevel)
 {
-	switch (v.type())
-	{
-		case QVariant::Bool:
-			output->write(v.toBool()==true ? "true" : "false");
-			break;
-		case QVariant::Invalid:
-			output->write("null");
-			break;
-		case QVariant::ByteArray:
-		{
-			QByteArray s(v.toByteArray());
-			s.replace('\\', "\\\\");
-			s.replace('\"', "\\\"");
-			s.replace('\b', "\\b");
-			s.replace('\n', "\\n");
-			s.replace('\f', "\\f");
-			s.replace('\r', "\\r");
-			s.replace('\t', "\\t");
-			output->write("\"" + s + "\"");
-			break;
-		}
-		case QVariant::String:
-		{
-			QString s(v.toString());
-			s.replace('\\', "\\\\");
-			s.replace('\"', "\\\"");
-			s.replace('\b', "\\b");
-			s.replace('\n', "\\n");
-			s.replace('\f', "\\f");
-			s.replace('\r', "\\r");
-			s.replace('\t', "\\t");
-			output->write(QString("\"%1\"").arg(s).toUtf8());
-			break;
-		}
-		case QVariant::Int:
-		case QVariant::Double:
-			output->write(v.toString().toUtf8());
-			break;
-		case QVariant::DateTime:
-		{
-			output->write(v.toDateTime().toString(Qt::ISODate).toUtf8());
-			break;
-		}
-		case QVariant::List:
-		{
-			output->putChar('[');
-			const QVariantList& l = v.toList();
-			for (int i=0;i<l.size();++i)
-			{
-				// Break line if we start an JSON Object for nice looking
-				if (l.at(i).type()==QVariant::Map)
-					output->putChar('\n');
-				write(l.at(i), output, indentLevel);
-				if (i!=l.size()-1)
-					output->write(", ");
-			}
-			output->putChar(']');
-			break;
-		}
-		case QVariant::Map:
-		{
-			const QByteArray prepend(indentLevel, '\t');
-			output->write(prepend);
-			output->write("{\n");
-			++indentLevel;
-			const QVariantMap& m = v.toMap();
-			int j =0;
-			for (QVariantMap::ConstIterator i=m.constBegin();i!=m.constEnd();++i)
-			{
-				output->write(prepend);
-				output->write("\t\"");
-				output->write(i.key().toUtf8());
-				output->write("\": ");
-				// Break line if we start an JSON Object for nice looking
-				if (i.value().type()==QVariant::Map)
-					output->putChar('\n');
-				write(i.value(), output, indentLevel);
-				if (++j!=m.size())
-					output->putChar(',');
-				output->putChar('\n');
-			}
-			output->write(prepend);
-			output->write("}");
-			--indentLevel;
-			break;
-		}
-		default:
-			QHash<int, void (*)(const QVariant&, QIODevice*, int)>::ConstIterator iter = otherSerializer.find(v.userType());
-			if (iter!=otherSerializer.constEnd())
-			{
-				iter.value()(v, output, indentLevel);
-			}
-			else
-				output->write("null");
-			//qDebug() << v.type() << v.userType() << v.typeName();
-			//qWarning() << "Cannot serialize QVariant of type " << v.typeName() << " in JSON";
-			break;
-	}
+	QByteArray json = write(v, indentLevel);
+	output->write(json);
 }
 
 QByteArray StelJsonParser::write(const QVariant& jsonObject, int indentLevel)
 {
-	QByteArray ar;
-	QBuffer buf(&ar);
-	buf.open(QIODevice::WriteOnly);
-	StelJsonParser::write(jsonObject, &buf, indentLevel);
-	buf.close();
-	return ar;
+	QJsonDocument doc = QJsonDocument::fromVariant(jsonObject);
+	return doc.toJson();
 }
 
 QVariant StelJsonParser::parse(QIODevice* input)
 {
-	StelJsonParserInstance parser(input);
-	return parser.parse();
+	QByteArray data = input->readAll();
+	return parse(data);
 }
 
 QVariant StelJsonParser::parse(const QByteArray& aar)
 {
-	QByteArray ar = aar;
-	QBuffer buf(&ar);
-	buf.open(QIODevice::ReadOnly);
-	QVariant v = StelJsonParser::parse(&buf);
-	buf.close();
-	return v;
-}
-
-JsonListIterator::JsonListIterator(QIODevice* input)
-{
-	parser = new StelJsonParserInstance(input);
-	if (!parser->skipAndConsumeChar('['))
-	{
-		throw std::runtime_error("Expected '[' to start a list iterator");
-	}
-	ahasNext = !parser->skipAndConsumeChar(']');
-}
-
-JsonListIterator::~JsonListIterator()
-{
-	Q_ASSERT(parser);
-	delete parser;
-	parser=NULL;
-}
-
-QVariant JsonListIterator::next()
-{
-	QVariant ret = parser->parse();
-	ahasNext = parser->skipAndConsumeChar(',');
-	if (!ahasNext)
-	{
-		if (!parser->skipAndConsumeChar(']'))
-			throw std::runtime_error("Expected ']' to end a list iterator");
-	}
-	return ret;
+	QJsonParseError error;
+	QJsonDocument doc = QJsonDocument::fromJson(aar, &error);
+	if (error.error != QJsonParseError::NoError)
+	{
+		throw std::runtime_error(error.errorString().toLatin1().constData());
+	}
+	return doc.toVariant();
 }

=== modified file 'src/core/StelJsonParser.hpp'
--- src/core/StelJsonParser.hpp	2012-01-11 10:50:37 +0000
+++ src/core/StelJsonParser.hpp	2016-11-18 09:14:03 +0000
@@ -24,34 +24,6 @@
 #include <QByteArray>
 
 
-//! Qt-style iterator over a JSON array. An actual list is not kept in memory,
-//! so only forward iteration is supported and all methods, including the constructor,
-//! involve read() calls on the QIODevice. Because of this, do not modify the
-//! QIODevice between calls to JsonListIterator methods. Also, the toFront()
-//! method has a special function and reset() is provided for convenience. Only
-//! peekNext() is guaranteed not to modify the QIODevice.
-class JsonListIterator
-{
-public:
-	//! Sets up JsonListIterator to read an array. Swallows all whitespace
-	//! up to a beginning '[' character. If '[' is not the first non-whitespace
-	//! character encountered, reset() is called and an exception is thrown.
-	JsonListIterator(QIODevice* input);
-	~JsonListIterator();
-
-	//! Reads and parses the next object from input. Advances QIODevice to
-	//! just after the object.
-	//! @return the next object from the array
-	QVariant next();
-
-	//! Returns true if the next non-whitespace character is not a ']' character.
-	bool hasNext() const {return ahasNext;}
-
-private:
-	bool ahasNext;
-	class StelJsonParserInstance* parser;
-};
-
 //! @class StelJsonParser
 //! Qt-based simple JSON reader inspired by the one from <a href='http://zoolib.sourceforge.net/'>Zoolib</a>.
 //! JSON is JavaScript Object Notation. See http://www.json.org/
@@ -69,9 +41,6 @@
 class StelJsonParser
 {
 public:
-	//! Create a JsonListIterator from the given input device.
-	static JsonListIterator initListIterator(QIODevice* in) {return JsonListIterator(in);}
-
 	//! Parse the given input stream.
 	static QVariant parse(QIODevice* input);
 	static QVariant parse(const QByteArray& input);
@@ -82,10 +51,7 @@
 	//! Serialize the passed QVariant as JSON in a QByteArray.
 	static QByteArray write(const QVariant& jsonObject, int indentLevel=0);
 	
-	static void registerSerializerForType(int t, void (*func)(const QVariant&, QIODevice*, int)) {otherSerializer.insert(t, func);}
-	
-private:
-	static QHash<int, void (*)(const QVariant&, QIODevice*, int)> otherSerializer;
+	// static void registerSerializerForType(int t, void (*func)(const QVariant&, QIODevice*, int)) {otherSerializer.insert(t, func);}
 };
 
 #endif // _STELJSONPARSER_HPP_

=== modified file 'src/tests/testStelJsonParser.cpp'
--- src/tests/testStelJsonParser.cpp	2016-08-14 09:10:10 +0000
+++ src/tests/testStelJsonParser.cpp	2016-11-18 09:14:03 +0000
@@ -86,33 +86,6 @@
 	QVERIFY(ok==true);
 }
 
-void TestStelJsonParser::testIterator()
-{
-	QBuffer buf;
-	buf.setData(listJsonBuff);
-	buf.open(QIODevice::ReadOnly);
-
-	try
-	{
-		int tot = 0;
-		JsonListIterator iter = StelJsonParser::initListIterator(&buf);
-		while (iter.hasNext())
-		{
-			QVariant v = iter.next();
-			QVERIFY(v.canConvert<QVariantMap>());
-			++tot;
-		}
-		QVERIFY(tot==3);
-	}
-	catch (std::runtime_error& e)
-	{
-		QString msg("Exception while loading JSON: ");
-		msg+=e.what();
-		QFAIL(qPrintable(msg));
-	}
-	buf.close();
-}
-
 void TestStelJsonParser::testErrors()
 {
 	bool wasCatched = false;
@@ -139,6 +112,7 @@
 	buf.open(QIODevice::ReadOnly);
 	QVariant result;
 	QBENCHMARK {
+		buf.seek(0);
 		result = StelJsonParser::parse(&buf);
 	}
 }

=== modified file 'src/tests/testStelJsonParser.hpp'
--- src/tests/testStelJsonParser.hpp	2013-09-24 05:34:35 +0000
+++ src/tests/testStelJsonParser.hpp	2016-11-18 09:14:03 +0000
@@ -29,7 +29,6 @@
 private slots:
 	void initTestCase();
 	void testBase();
-	void testIterator();
 	void benchmarkParse();
 	void testErrors();
 private: