Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: basic object merging #650

Merged
merged 13 commits into from
Jan 14, 2022
22 changes: 19 additions & 3 deletions src/sentry_scope.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "sentry_scope.h"
#include "sentry.h"
#include "sentry_backend.h"
#include "sentry_core.h"
#include "sentry_database.h"
Expand All @@ -7,6 +8,7 @@
#include "sentry_string.h"
#include "sentry_symbolizer.h"
#include "sentry_sync.h"
#include "sentry_value.h"

#include <stdlib.h>

Expand Down Expand Up @@ -293,9 +295,23 @@ sentry__scope_apply_to_event(const sentry_scope_t *scope,
PLACE_STRING("transaction", scope->transaction);
PLACE_VALUE("sdk", scope->client_sdk);

// TODO: these should merge
PLACE_CLONED_VALUE("tags", scope->tags);
PLACE_CLONED_VALUE("extra", scope->extra);
sentry_value_t event_tags = sentry_value_get_by_key(event, "tags");
if (sentry_value_is_null(event_tags)) {
sentry_value_t tags = sentry__value_clone(scope->tags);
if (!sentry_value_is_null(tags)) {
sentry_value_set_by_key(event, "tags", tags);
}
} else {
sentry__value_merge_objects(event_tags, scope->tags);
}
sentry_value_t event_extra = sentry_value_get_by_key(event, "extra");
if (sentry_value_is_null(event_extra)) {
sentry_value_t extra = sentry__value_clone(scope->extra);
if (!sentry_value_is_null(extra)) {
sentry_value_set_by_key(event, "extra", extra);
}
}
sentry__value_merge_objects(event_extra, scope->extra);

#ifdef SENTRY_PERFORMANCE_MONITORING
// TODO: better, more thorough deep merging
Expand Down
36 changes: 36 additions & 0 deletions src/sentry_value.c
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include "sentry.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this include. this is being pulled in via sentry_boot.h, and that should also magically fix your compile error.

Don’t ask me to explain exactly whats going on. Just that in C include order matters, and defines that you set have an effect on includes, and sentry_boot.h sets some defines that are needed so the Windows SDK does not break on microsofts own compiler. Fun times! Rewrite it in Rust!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, thanks!

#include "sentry_boot.h"

#include <assert.h>
Expand Down Expand Up @@ -806,6 +807,41 @@ sentry_value_is_null(sentry_value_t value)
return value._bits == CONST_NULL;
}

int
sentry__value_merge_objects(sentry_value_t dst, sentry_value_t src)
{
if (sentry_value_is_null(src)) {
return 0;
}
if (sentry_value_get_type(dst) != SENTRY_VALUE_TYPE_OBJECT
|| sentry_value_get_type(src) != SENTRY_VALUE_TYPE_OBJECT
|| sentry_value_is_frozen(dst)) {
return 1;
flub marked this conversation as resolved.
Show resolved Hide resolved
}
thing_t *thing = value_as_thing(src);
if (!thing) {
return 1;
}
obj_t *obj = thing->payload._ptr;
for (size_t i = 0; i < obj->len; i++) {
char *key = obj->pairs[i].k;
sentry_value_t src_val = obj->pairs[i].v;
sentry_value_t dst_val = sentry_value_get_by_key(dst, key);
if (sentry_value_get_type(dst_val) == SENTRY_VALUE_TYPE_OBJECT
&& sentry_value_get_type(src_val) == SENTRY_VALUE_TYPE_OBJECT) {
if (sentry__value_merge_objects(dst_val, src_val) != 0) {
return 1;
}
} else {
flub marked this conversation as resolved.
Show resolved Hide resolved
if (sentry_value_set_by_key(dst, key, src_val) != 0) {
return 1;
}
sentry_value_incref(src_val);
}
}
return 0;
}

void
sentry__jsonwriter_write_value(sentry_jsonwriter_t *jw, sentry_value_t value)
{
Expand Down
17 changes: 17 additions & 0 deletions src/sentry_value.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#ifndef SENTRY_VALUE_H_INCLUDED
#define SENTRY_VALUE_H_INCLUDED

#include "sentry.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here I guess… Not sure why you added this in the first place?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i didn't, clangd did

#include "sentry_boot.h"

/**
Expand Down Expand Up @@ -96,6 +97,22 @@ sentry_value_t sentry__value_clone(sentry_value_t value);
int sentry__value_append_bounded(
sentry_value_t value, sentry_value_t v, size_t max);

/**
* Deep-merges object src into dst.
*
* For each key-value pair in the src object the same key in the dst object
* will be set to the value from src. If both the dst value and the src value
* are objects themselves they are stepped into recursively instead of
* overriding the entire dst object.
*
* If src is null nothing needs to be merged and this is handled gracefully,
* otherwise if dst is any other type than an object or src is neither an
* object nor null an error is returned.
*
* Returns 0 on success.
*/
int sentry__value_merge_objects(sentry_value_t dst, sentry_value_t src);

/**
* Parse the given JSON string into a new Value.
*/
Expand Down
54 changes: 54 additions & 0 deletions tests/unit/test_value.c
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,60 @@ SENTRY_TEST(value_object)
sentry_value_decref(val);
}

SENTRY_TEST(value_object_merge)
{
sentry_value_t dst = sentry_value_new_object();
sentry_value_set_by_key(dst, "a", sentry_value_new_int32(1));
sentry_value_set_by_key(dst, "b", sentry_value_new_int32(2));

sentry_value_t src = sentry_value_new_object();
sentry_value_set_by_key(src, "b", sentry_value_new_int32(20));
sentry_value_set_by_key(src, "c", sentry_value_new_int32(30));

int rv = sentry__value_merge_objects(dst, src);
TEST_CHECK_INT_EQUAL(rv, 0);
sentry_value_decref(src);

sentry_value_t a = sentry_value_get_by_key(dst, "a");
sentry_value_t b = sentry_value_get_by_key(dst, "b");
sentry_value_t c = sentry_value_get_by_key(dst, "c");
TEST_CHECK_INT_EQUAL(sentry_value_as_int32(a), 1);
TEST_CHECK_INT_EQUAL(sentry_value_as_int32(b), 20);
TEST_CHECK_INT_EQUAL(sentry_value_as_int32(c), 30);
sentry_value_decref(dst);
}

SENTRY_TEST(value_object_merge_nested)
{
sentry_value_t dst = sentry_value_new_object();
sentry_value_set_by_key(dst, "a", sentry_value_new_int32(1));
sentry_value_t dst_nested = sentry_value_new_object();
sentry_value_set_by_key(dst_nested, "ba", sentry_value_new_int32(1));
sentry_value_set_by_key(dst_nested, "bb", sentry_value_new_int32(2));
sentry_value_set_by_key(dst, "b", dst_nested);

sentry_value_t src = sentry_value_new_object();
sentry_value_t src_nested = sentry_value_new_object();
sentry_value_set_by_key(src_nested, "bb", sentry_value_new_int32(20));
sentry_value_set_by_key(src_nested, "bc", sentry_value_new_int32(30));
sentry_value_set_by_key(src, "b", src_nested);

int rv = sentry__value_merge_objects(dst, src);
TEST_CHECK_INT_EQUAL(rv, 0);
sentry_value_decref(src);

sentry_value_t a = sentry_value_get_by_key(dst, "a");
sentry_value_t nested = sentry_value_get_by_key(dst, "b");
sentry_value_t ba = sentry_value_get_by_key(nested, "ba");
sentry_value_t bb = sentry_value_get_by_key(nested, "bb");
sentry_value_t bc = sentry_value_get_by_key(nested, "bc");
TEST_CHECK_INT_EQUAL(sentry_value_as_int32(a), 1);
TEST_CHECK_INT_EQUAL(sentry_value_as_int32(ba), 1);
TEST_CHECK_INT_EQUAL(sentry_value_as_int32(bb), 20);
TEST_CHECK_INT_EQUAL(sentry_value_as_int32(bc), 30);
sentry_value_decref(dst);
}

SENTRY_TEST(value_freezing)
{
sentry_value_t val = sentry_value_new_list();
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/tests.inc
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ XX(value_json_surrogates)
XX(value_list)
XX(value_null)
XX(value_object)
XX(value_object_merge)
XX(value_object_merge_nested)
XX(value_string)
XX(value_unicode)
XX(value_wrong_type)
Expand Down