Daniel P. Berrange wrote:
> Comparing JSON docs using strcmp is simple, but is not flexible
> as it is sensitive to whitespace used in the doc generation.
> When comparing objects it may also be desirable to treat the
> existance of keys in the actual object but not expected object
> as non-fatal. Introduce a virJSONStringCompare function which
> takes two strings representing expected and actual JSON docs
> and then does a DOM comparison.
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> src/libvirt_private.syms | 1 +
> src/util/virjson.c | 185 +++++++++++++++++++++++++++++++++++++++++++++++
>
I'm not too familiar with this file, but didn't notice any problems with
this patch beyond a little nit below.
> src/util/virjson.h | 22 ++++++
> 3 files changed, 208 insertions(+)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 4c1f61c..5f2b9d9 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1406,6 +1406,7 @@ virISCSIScanTargets;
>
>
> # util/virjson.h
> +virJSONStringCompare;
> virJSONValueArrayAppend;
> virJSONValueArrayGet;
> virJSONValueArraySize;
> diff --git a/src/util/virjson.c b/src/util/virjson.c
> index 35a8252..0a6d1be 100644
> --- a/src/util/virjson.c
> +++ b/src/util/virjson.c
> @@ -47,6 +47,11 @@
>
> VIR_LOG_INIT("util.json");
>
> +VIR_ENUM_DECL(virJSONType)
> +VIR_ENUM_IMPL(virJSONType, VIR_JSON_TYPE_LAST,
> + "object", "array", "string",
> + "number", "boolean", "null")
> +
> typedef struct _virJSONParserState virJSONParserState;
> typedef virJSONParserState *virJSONParserStatePtr;
> struct _virJSONParserState {
> @@ -90,6 +95,7 @@ void virJSONValueFree(virJSONValuePtr value)
> break;
> case VIR_JSON_TYPE_BOOLEAN:
> case VIR_JSON_TYPE_NULL:
> + case VIR_JSON_TYPE_LAST:
> break;
> }
>
> @@ -1152,3 +1158,182 @@ char *virJSONValueToString(virJSONValuePtr object ATTRIBUTE_UNUSED,
> return NULL;
> }
> #endif
> +
> +
> +static bool
> +virJSONValueCompare(virJSONValuePtr expect,
> + virJSONValuePtr actual,
> + const char *context,
> + unsigned int flags)
> +{
> + size_t i, j;
> + if (expect->type != actual->type) {
>
Super nit: seems most of libvirt code would have a line between local
variable declarations and start of code, but this file is not consistent
in that regard anyhow.
ACK.
Regards,
Jim
> + if (expect->type == VIR_JSON_TYPE_NULL &&
> + (flags & VIR_JSON_COMPARE_IGNORE_EXPECTED_NULL))
> + return true;
> +
> + const char *expectType = virJSONTypeTypeToString(expect->type);
> + const char *actualType = virJSONTypeTypeToString(actual->type);
> +
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Expected value type '%s' but got value type '%s' at '%s'"),
> + expectType, actualType, context);
> + return false;
> + }
> +
> + switch (expect->type) {
> + case VIR_JSON_TYPE_OBJECT:
> + for (i = 0; i < expect->data.object.npairs; i++) {
> + bool found = false;
> + char *childcontext;
> + for (j = 0; j < actual->data.object.npairs; j++) {
> + if (STREQ(expect->data.object.pairs[i].key,
> + actual->data.object.pairs[j].key)) {
> + found = true;
> + break;
> + }
> + }
> + if (!found) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Expected object key '%s' not found in actual object at '%s'"),
> + expect->data.object.pairs[i].key, context);
> + return false;
> + }
> +
> + if (virAsprintf(&childcontext, "%s%s%s",
> + context,
> + STREQ(context, "/") ? "" : "/",
> + expect->data.object.pairs[i].key) < 0)
> + return false;
> +
> + if (!virJSONValueCompare(expect->data.object.pairs[i].value,
> + actual->data.object.pairs[j].value,
> + childcontext,
> + flags)) {
> + VIR_FREE(childcontext);
> + return false;
> + }
> + VIR_FREE(childcontext);
> + }
> + if (!(flags & VIR_JSON_COMPARE_IGNORE_EXTRA_OBJECT_KEYS)) {
> + for (i = 0; i < actual->data.object.npairs; i++) {
> + bool found = false;
> + char *childcontext;
> + for (j = 0; j < expect->data.object.npairs; j++) {
> + if (STREQ(actual->data.object.pairs[i].key,
> + expect->data.object.pairs[j].key)) {
> + found = true;
> + break;
> + }
> + }
> + if (!found) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Actual object key '%s' not found in expected object at '%s'"),
> + actual->data.object.pairs[i].key, context);
> + return false;
> + }
> +
> + if (virAsprintf(&childcontext, "%s%s%s",
> + STREQ(context, "/") ? "" : "/",
> + context, actual->data.object.pairs[i].key) < 0)
> + return false;
> +
> + if (!virJSONValueCompare(actual->data.object.pairs[i].value,
> + expect->data.object.pairs[j].value,
> + childcontext,
> + flags)) {
> + VIR_FREE(childcontext);
> + return false;
> + }
> + VIR_FREE(childcontext);
> + }
> + }
> + break;
> + case VIR_JSON_TYPE_ARRAY:
> + if (expect->data.array.nvalues !=
> + actual->data.array.nvalues) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Expected array size '%zu' but got size '%zu' at '%s'"),
> + expect->data.array.nvalues,
> + actual->data.array.nvalues,
> + context);
> + return false;
> + }
> +
> + for (i = 0; i < expect->data.array.nvalues; i++) {
> + char *childcontext;
> + if (virAsprintf(&childcontext, "%s%s[%zu]",
> + STREQ(context, "/") ? "" : "/",
> + context, i) < 0)
> + return false;
> +
> + if (!virJSONValueCompare(expect->data.array.values[i],
> + actual->data.array.values[i],
> + childcontext,
> + flags)) {
> + VIR_FREE(childcontext);
> + return false;
> + }
> + VIR_FREE(childcontext);
> + }
> + break;
> + case VIR_JSON_TYPE_STRING:
> + if (STRNEQ(expect->data.string,
> + actual->data.string)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Expected string value '%s' but got '%s' at '%s'"),
> + expect->data.string, actual->data.string, context);
> + return false;
> + }
> + break;
> + case VIR_JSON_TYPE_NUMBER:
> + if (STRNEQ(expect->data.number,
> + actual->data.number)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Expected number value '%s' but got '%s' at '%s'"),
> + expect->data.number, actual->data.number, context);
> + return false;
> + }
> + break;
> + case VIR_JSON_TYPE_BOOLEAN:
> + if (expect->data.boolean !=
> + actual->data.boolean) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Expected bool value '%d' but got '%d' at '%s'"),
> + expect->data.boolean, actual->data.boolean, context);
> + return false;
> + }
> + break;
> + case VIR_JSON_TYPE_NULL:
> + /* trivially equal */
> + break;
> +
> + case VIR_JSON_TYPE_LAST:
> + /* nothing */
> + break;
> + }
> +
> + return true;
> +}
> +
> +
> +bool virJSONStringCompare(const char *expect,
> + const char *actual,
> + unsigned int flags)
> +{
> + virJSONValuePtr expectVal = NULL;
> + virJSONValuePtr actualVal = NULL;
> + int ret = false;
> +
> + if (!(expectVal = virJSONValueFromString(expect)))
> + goto cleanup;
> + if (!(actualVal = virJSONValueFromString(actual)))
> + goto cleanup;
> +
> + ret = virJSONValueCompare(expectVal, actualVal, "/", flags);
> +
> + cleanup:
> + virJSONValueFree(expectVal);
> + virJSONValueFree(actualVal);
> + return ret;
> +}
> diff --git a/src/util/virjson.h b/src/util/virjson.h
> index db11396..b1f96d3 100644
> --- a/src/util/virjson.h
> +++ b/src/util/virjson.h
> @@ -34,6 +34,8 @@ typedef enum {
> VIR_JSON_TYPE_NUMBER,
> VIR_JSON_TYPE_BOOLEAN,
> VIR_JSON_TYPE_NULL,
> +
> + VIR_JSON_TYPE_LAST,
> } virJSONType;
>
> typedef struct _virJSONValue virJSONValue;
> @@ -139,4 +141,24 @@ virJSONValuePtr virJSONValueFromString(const char *jsonstring);
> char *virJSONValueToString(virJSONValuePtr object,
> bool pretty);
>
> +typedef enum {
> + /*
> + * When comparing a pair of Objects, if the 'actual' object
> + * has keys that are not present in the 'expected' object,
> + * the existance of these extra keys will be non-fatal.
> + */
> + VIR_JSON_COMPARE_IGNORE_EXTRA_OBJECT_KEYS = (1 << 0),
> +
> + /*
> + * when comparing two values, if their types are different,
> + * and the 'expected' value type is 'null', then this will
> + * be considered non-fatal.
> + */
> + VIR_JSON_COMPARE_IGNORE_EXPECTED_NULL = (1 << 1),
> +} virJSONCompareFlags;
> +
> +bool virJSONStringCompare(const char *expect,
> + const char *actual,
> + unsigned int flags);
> +
> #endif /* __VIR_JSON_H_ */
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list