[libgdata/libgdata-0-10: 12/15] Bug 659016 â @rel not mandatory in website references

[Philip Withnall <pwithnall src gnome org>]

commit 6c369e44ede54bf17c5433029133264e9efac632
Author: Philip Withnall <philip tecnocode co uk>
Date:   Wed Sep 14 19:19:38 2011 +0100

    Bug 659016 â @rel not mandatory in website references
    
    Allow gContact:website elements to include a label attribute but no rel
    attribute. Includes new test cases.
    
    I've filed
    http://code.google.com/a/google.com/p/apps-api-issues/issues/detail?id=2769
    about the lack of clarity in Google's protocol specification regarding this.
    
    Closes: bgo#659016

 gdata/gcontact/gdata-gcontact-website.c |   46 +++++++++++++++++++++++-------
 gdata/tests/general.c                   |   32 +++++++++++++++++++++-
 2 files changed, 66 insertions(+), 12 deletions(-)
---
diff --git a/gdata/gcontact/gdata-gcontact-website.c b/gdata/gcontact/gdata-gcontact-website.c
index de14948..edfcbdb 100644
--- a/gdata/gcontact/gdata-gcontact-website.c
+++ b/gdata/gcontact/gdata-gcontact-website.c
@@ -100,6 +100,7 @@ gdata_gcontact_website_class_init (GDataGContactWebsiteClass *klass)
 	 * GDataGContactWebsite:relation-type:
 	 *
 	 * A programmatic value that identifies the type of website. Examples are %GDATA_GCONTACT_WEBSITE_HOME_PAGE or %GDATA_GCONTACT_WEBSITE_FTP.
+	 * It is mutually exclusive with #GDataGContactWebsite:label.
 	 *
 	 * For more information, see the
 	 * <ulink type="http" url="http://code.google.com/apis/contacts/docs/3.0/reference.html#gcWebsite";>gContact specification</ulink>.
@@ -116,6 +117,7 @@ gdata_gcontact_website_class_init (GDataGContactWebsiteClass *klass)
 	 * GDataGContactWebsite:label:
 	 *
 	 * A simple string value used to name this website. It allows UIs to display a label such as "Work", "Travel blog", "Personal blog", etc.
+	 * It is mutually exclusive with #GDataGContactWebsite:relation-type.
 	 *
 	 * For more information, see the
 	 * <ulink type="http" url="http://code.google.com/apis/contacts/docs/3.0/reference.html#gcWebsite";>gContact specification</ulink>.
@@ -233,7 +235,7 @@ gdata_gcontact_website_set_property (GObject *object, guint property_id, const G
 static gboolean
 pre_parse_xml (GDataParsable *parsable, xmlDoc *doc, xmlNode *root_node, gpointer user_data, GError **error)
 {
-	xmlChar *uri, *rel;
+	xmlChar *uri, *rel, *label;
 	gboolean primary_bool;
 	GDataGContactWebsitePrivate *priv = GDATA_GCONTACT_WEBSITE (parsable)->priv;
 
@@ -247,16 +249,19 @@ pre_parse_xml (GDataParsable *parsable, xmlDoc *doc, xmlNode *root_node, gpointe
 		return gdata_parser_error_required_property_missing (root_node, "href", error);
 	}
 
+	/* NOTE: We allow both rel and label to be present when we should probably be asserting that they're mutually exclusive. See the comment in
+	 * pre_get_xml() for details. */
 	rel = xmlGetProp (root_node, (xmlChar*) "rel");
-	if (rel == NULL || *rel == '\0') {
-		xmlFree (uri);
+	label = xmlGetProp (root_node, (xmlChar*) "label");
+	if ((rel == NULL || *rel == '\0') && (label == NULL || *label == '\0')) {
 		xmlFree (rel);
+		xmlFree (label);
 		return gdata_parser_error_required_property_missing (root_node, "rel", error);
 	}
 
-	priv->uri = (gchar*) uri;
 	priv->relation_type = (gchar*) rel;
-	priv->label = (gchar*) xmlGetProp (root_node, (xmlChar*) "label");
+	priv->label = (gchar*) label;
+	priv->uri = (gchar*) uri;
 	priv->is_primary = primary_bool;
 
 	return TRUE;
@@ -268,10 +273,18 @@ pre_get_xml (GDataParsable *parsable, GString *xml_string)
 	GDataGContactWebsitePrivate *priv = GDATA_GCONTACT_WEBSITE (parsable)->priv;
 
 	gdata_parser_string_append_escaped (xml_string, " href='", priv->uri, "'");
-	gdata_parser_string_append_escaped (xml_string, " rel='", priv->relation_type, "'");
 
-	if (priv->label != NULL)
+	/* NOTE: We previously allowed both rel and label to be set, making rel mandatory. Since bgo#659016, we treat rel and label as mutually
+	 * exclusive attributes when parsing. We should treat them as mutually exclusive here as well, and g_assert_not_reached() if neither or
+	 * both are set (as in gdata-gcontact-event.c:pre_get_xml()), but in order to maintain backwards compatibility, we don't.
+	 * Sigh, Google. */
+	if (priv->relation_type != NULL) {
+		gdata_parser_string_append_escaped (xml_string, " rel='", priv->relation_type, "'");
+	}
+
+	if (priv->label != NULL) {
 		gdata_parser_string_append_escaped (xml_string, " label='", priv->label, "'");
+	}
 
 	if (priv->is_primary == TRUE)
 		g_string_append (xml_string, " primary='true'");
@@ -288,13 +301,15 @@ get_namespaces (GDataParsable *parsable, GHashTable *namespaces)
 /**
  * gdata_gcontact_website_new:
  * @uri: the website URI
- * @relation_type: the relationship between the website and its owner
+ * @relation_type: the relationship between the website and its owner, or %NULL
  * @label: (allow-none): a human-readable label for the website, or %NULL
  * @is_primary: %TRUE if this website is its owner's primary website, %FALSE otherwise
  *
  * Creates a new #GDataGContactWebsite. More information is available in the <ulink type="http"
  * url="http://code.google.com/apis/contacts/docs/3.0/reference.html#gcWebsite";>gContact specification</ulink>.
  *
+ * Exactly one of @relation_type and @label should be provided; the other must be %NULL.
+ *
  * Return value: a new #GDataGContactWebsite; unref with g_object_unref()
  *
  * Since: 0.7.0
@@ -303,7 +318,10 @@ GDataGContactWebsite *
 gdata_gcontact_website_new (const gchar *uri, const gchar *relation_type, const gchar *label, gboolean is_primary)
 {
 	g_return_val_if_fail (uri != NULL && *uri != '\0', NULL);
-	g_return_val_if_fail (relation_type != NULL && *relation_type != '\0', NULL);
+	g_return_val_if_fail ((relation_type != NULL && *relation_type != '\0'/* && label == NULL*/) ||
+	                      (relation_type == NULL && label != NULL && *label != '\0'), NULL);
+	/* NOTE: As in pre_get_xml(), we should treat rel and label as mutually exclusive here, but we can't for backwards compatibility reasons. */
+
 	return g_object_new (GDATA_TYPE_GCONTACT_WEBSITE, "uri", uri, "relation-type", relation_type, "label", label, "is-primary", is_primary, NULL);
 }
 
@@ -369,13 +387,16 @@ gdata_gcontact_website_get_relation_type (GDataGContactWebsite *self)
  * Sets the #GDataGContactWebsite:relation-type property to @relation_type
  * such as %GDATA_GCONTACT_WEBSITE_HOME_PAGE or %GDATA_GCONTACT_WEBSITE_FTP.
  *
+ * If @relation_type is %NULL, the relation type will be unset. When the #GDataGContactWebsite is used in a query, however,
+ * exactly one of #GDataGContactWebsite:relation-type and #GDataGContactWebsite:label must be %NULL.
+ *
  * Since: 0.7.0
  **/
 void
 gdata_gcontact_website_set_relation_type (GDataGContactWebsite *self, const gchar *relation_type)
 {
 	g_return_if_fail (GDATA_IS_GCONTACT_WEBSITE (self));
-	g_return_if_fail (relation_type != NULL && *relation_type != '\0');
+	g_return_if_fail (relation_type == NULL || *relation_type != '\0');
 
 	g_free (self->priv->relation_type);
 	self->priv->relation_type = g_strdup (relation_type);
@@ -406,7 +427,8 @@ gdata_gcontact_website_get_label (GDataGContactWebsite *self)
  *
  * Sets the #GDataGContactWebsite:label property to @label.
  *
- * Set @label to %NULL to unset the property in the website.
+ * If @label is %NULL, the label will be unset. When the #GDataGContactWebsite is used in a query, however,
+ * exactly one of #GDataGContactWebsite:relation-type and #GDataGContactWebsite:label must be %NULL.
  *
  * Since: 0.7.0
  **/
@@ -414,6 +436,8 @@ void
 gdata_gcontact_website_set_label (GDataGContactWebsite *self, const gchar *label)
 {
 	g_return_if_fail (GDATA_IS_GCONTACT_WEBSITE (self));
+	/* NOTE: We should be validating using the code below, but we can't. See pre_get_xml() for details. */
+	g_return_if_fail (label == NULL || *label != '\0');
 
 	g_free (self->priv->label);
 	self->priv->label = g_strdup (label);
diff --git a/gdata/tests/general.c b/gdata/tests/general.c
index 4fe2b01..c0b2e18 100644
--- a/gdata/tests/general.c
+++ b/gdata/tests/general.c
@@ -3627,6 +3627,35 @@ test_gcontact_website (void)
 }
 
 static void
+test_gcontact_website_label (void)
+{
+	GDataGContactWebsite *website;
+	GError *error = NULL;
+
+	g_test_bug ("659016");
+
+	/* Parse a website with a label but no rel. */
+	website = GDATA_GCONTACT_WEBSITE (gdata_parsable_new_from_xml (GDATA_TYPE_GCONTACT_WEBSITE,
+		"<gContact:website xmlns:gContact='http://schemas.google.com/contact/2008' href='http://test.com/' label='Custom'/>", -1, &error));
+	g_assert_no_error (error);
+	g_assert (GDATA_IS_GCONTACT_WEBSITE (website));
+	g_clear_error (&error);
+
+	/* Check the properties */
+	g_assert_cmpstr (gdata_gcontact_website_get_uri (website), ==, "http://test.com/";);
+	g_assert_cmpstr (gdata_gcontact_website_get_relation_type (website), ==, NULL);
+	g_assert_cmpstr (gdata_gcontact_website_get_label (website), ==, "Custom");
+	g_assert (gdata_gcontact_website_is_primary (website) == FALSE);
+
+	/* Check the outputted XML is still OK */
+	gdata_test_assert_xml (website,
+		"<?xml version='1.0' encoding='UTF-8'?>"
+		"<gContact:website xmlns='http://www.w3.org/2005/Atom' xmlns:gContact='http://schemas.google.com/contact/2008' "
+		                  "href='http://test.com/' label='Custom' primary='false'/>");
+	g_object_unref (website);
+}
+
+static void
 test_gcontact_website_error_handling (void)
 {
 	GDataGContactWebsite *website;
@@ -3642,7 +3671,7 @@ test_gcontact_website_error_handling (void)
 
 	TEST_XML_ERROR_HANDLING ("rel='work'"); /* no href */
 	TEST_XML_ERROR_HANDLING ("rel='work' href=''"); /* empty href */
-	TEST_XML_ERROR_HANDLING ("href='http://example.com/'"); /* no rel */
+	TEST_XML_ERROR_HANDLING ("href='http://example.com/'"); /* no rel or label */
 	TEST_XML_ERROR_HANDLING ("href='http://example.com/' rel=''"); /* empty rel */
 	TEST_XML_ERROR_HANDLING ("href='http://example.com/' rel='profile' primary='not a boolean'"); /* invalid primary */
 
@@ -3760,6 +3789,7 @@ main (int argc, char *argv[])
 	g_test_add_func ("/gcontact/relation/error_handling", test_gcontact_relation_error_handling);
 	g_test_add_func ("/gcontact/relation/escaping", test_gcontact_relation_escaping);
 	g_test_add_func ("/gcontact/website", test_gcontact_website);
+	g_test_add_func ("/gcontact/website/label", test_gcontact_website_label);
 	g_test_add_func ("/gcontact/website/error_handling", test_gcontact_website_error_handling);
 	g_test_add_func ("/gcontact/website/escaping", test_gcontact_website_escaping);