dee

Merge lp://qastaging/~mhr3/dee/easy-hints into lp://qastaging/dee

easy-hints
Merge into trunk

Proposed by Michal Hruby on 2012-11-20

Status:	Merged
Approved by:	Michal Hruby on 2012-11-23
Approved revision:	408
Merged at revision:	388
Proposed branch:	lp://qastaging/~mhr3/dee/easy-hints
Merge into:	lp://qastaging/dee
Prerequisite:	lp://qastaging/~mhr3/dee/named-columns
Diff against target:	1117 lines (+638/-75) 9 files modified bindings/python/Dee.py (+38/-12) debian/libdee-1.0-4.symbols (+2/-0) src/dee-model.c (+147/-52) src/dee-model.h (+16/-0) src/dee-proxy-model.c (+35/-1) src/dee-serializable-model.c (+173/-4) src/dee-transaction.c (+39/-3) tests/test-model-rows.c (+183/-0) vapi/dee-1.0.vapi (+5/-3)
To merge this branch:	bzr merge lp://qastaging/~mhr3/dee/easy-hints
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot (community)	continuous-integration		Needs Fixing on 2012-11-23
Paweł Stołowski (community)		2012-11-20	Approve on 2012-11-22
Review via email: mp+135197@code.qastaging.launchpad.net

Commit message

Implement methods to allow easy manipulation of a{sv} columns

Description of the change

Add a new method to register schema of fields in columns with 'a{sv}' schema plus transparent support for that in build_named_row() and get_value_by_name() methods.

Revision history for this message

Paweł Stołowski (stolowski) wrote on 2012-11-21:

285 + g_variant_builder_add (builders[col_idx], "{sv}",
286 + col_name, // FIXME: "::"
287 + collect_variant (col_schema, args));

I think "::" needs to be handled?

Question: what happens if there are two a{sv} fields in the model, and both have some common fields (e.g. "id")?
Would something like:
dee_model_build_named_row (fix->model, "object-path", "/org/example", "field1::id", 8123, "field2::id", 1111, NULL) just work?

review: Needs Fixing

Revision history for this message

Michal Hruby (mhr3) wrote on 2012-11-21:

> 285 + g_variant_builder_add (builders[col_idx], "{sv}",
> 286 + col_name, // FIXME: "::"
> 287 + collect_variant (col_schema,
> args));
>
> I think "::" needs to be handled?

Indeed, I'm getting very good at forgetting my own FIXMEs :)

> Question: what happens if there are two a{sv} fields in the model, and both
> have some common fields (e.g. "id")?
> Would something like:
> dee_model_build_named_row (fix->model, "object-path", "/org/example",
> "field1::id", 8123, "field2::id", 1111, NULL) just work?

That was the idea.

Revision history for this message

Paweł Stołowski (stolowski) wrote on 2012-11-22:

628 + g_hash_table_insert (priv->field_schemas, g_strdup (key), info);
629 + full_name = g_strdup_printf ("%s::%s", priv->column_names[column],
630 + (gchar*) key);
631 + /* transfering ownership of full_name to hashtable */
632 + g_hash_table_insert (priv->field_schemas, full_name,

Maybe I'm missing something, but I think 628 will silently overwrite schema for a{sv} field if there is another field with same name in a different a{sv} column. This will cause issues if you don't pass fully-qualified field name to build_row. If this is the case, then perhaps we should only allow fully-qualified field names for a{sv}?

Whether I'm right or wrong, could you please add a test case for two a{sv} columns with some common field names in them?

review: Needs Fixing

Revision history for this message

Paweł Stołowski (stolowski) wrote on 2012-11-22:

Cool!

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2012-11-22:

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/dee-mbs-autolanding/16/
Executed test runs:
SUCCESS: http://jenkins.qa.ubuntu.com/job/dee-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=amd64/16/console
FAILURE: http://jenkins.qa.ubuntu.com/job/dee-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=i386/16/console