core: Warn on passing empty values to AbstractFieldDetails subclasses

There are no situations where passing an empty value (string or object) to
an AbstractFieldDetails subclass makes sense. It just introduces potential
bugs with mishandling empty strings (etc.) in code which consumes the
AbstractFieldDetails.

This commit adds warnings to all the AbstractFieldDetails subclasses’
constructors which check for empty inputs. We can't use assert()s here
because that would be an API break.

Helps: bgo#666540

diff --git a/folks/email-details.vala b/folks/email-details.vala
index 81bc542..4ed3e15 100644 (file)
--- a/folks/email-details.vala
+++ b/folks/email-details.vala
@@ -37,12 +37,12 @@ public class Folks.EmailFieldDetails : AbstractFieldDetails<string>
   /**
    * Create a new EmailFieldDetails.
    *
-   * @param value the value of the field
+   * @param value the value of the field, which should be a valid, non-empty
+   * e-mail address
    * @param parameters initial parameters. See
    * {@link AbstractFieldDetails.parameters}. A `null` value is equivalent to an
    * empty map of parameters.
    *
-   *
    * @return a new EmailFieldDetails
    *
    * @since 0.6.0
@@ -50,6 +50,11 @@ public class Folks.EmailFieldDetails : AbstractFieldDetails<string>
   public EmailFieldDetails (string value,
       MultiMap<string, string>? parameters = null)
     {
+      if (value == "")
+        {
+          warning ("Empty e-mail address passed to EmailFieldDetails.");
+        }
+
       this.value = value;
       if (parameters != null)
         this.parameters = parameters;

diff --git a/folks/im-details.vala b/folks/im-details.vala
index 864894e..66becd5 100644 (file)
--- a/folks/im-details.vala
+++ b/folks/im-details.vala
@@ -46,7 +46,8 @@ public class Folks.ImFieldDetails : AbstractFieldDetails<string>
   /**
    * Create a new ImFieldDetails.
    *
-   * @param value the value of the field
+   * @param value the value of the field, which should be a valid, non-empty
+   * IM address
    * @param parameters initial parameters. See
    * {@link AbstractFieldDetails.parameters}. A `null` value is equivalent to an
    * empty map of parameters.
@@ -58,6 +59,11 @@ public class Folks.ImFieldDetails : AbstractFieldDetails<string>
   public ImFieldDetails (string value,
       MultiMap<string, string>? parameters = null)
     {
+      if (value == "")
+        {
+          warning ("Empty IM address passed to ImFieldDetails.");
+        }
+
       this.value = value;
       if (parameters != null)
         this.parameters = parameters;

diff --git a/folks/note-details.vala b/folks/note-details.vala
index 838191a..cb0333f 100644 (file)
--- a/folks/note-details.vala
+++ b/folks/note-details.vala
@@ -56,7 +56,8 @@ public class Folks.NoteFieldDetails : AbstractFieldDetails<string>
   /**
    * Create a new NoteFieldDetails.
    *
-   * @param value the value of the field
+   * @param value the value of the field, which should be a non-empty free-form
+   * UTF-8 string as entered by the user
    * @param parameters initial parameters. See
    * {@link AbstractFieldDetails.parameters}. A `null` value is equivalent to a
    * empty map of parameters.
@@ -69,6 +70,11 @@ public class Folks.NoteFieldDetails : AbstractFieldDetails<string>
       MultiMap<string, string>? parameters = null,
       string? uid = null)
     {
+      if (value == "")
+        {
+          warning ("Empty note passed to NoteFieldDetails.");
+        }
+
       Object (value: value,
               id: uid,
               parameters: parameters);

diff --git a/folks/phone-details.vala b/folks/phone-details.vala
index f71f34f..bc525ff 100644 (file)
--- a/folks/phone-details.vala
+++ b/folks/phone-details.vala
@@ -54,7 +54,8 @@ public class Folks.PhoneFieldDetails : AbstractFieldDetails<string>
   /**
    * Create a new PhoneFieldDetails.
    *
-   * @param value the value of the field
+   * @param value the value of the field, which should be a non-empty phone
+   * number (no particular format is mandated)
    * @param parameters initial parameters. See
    * {@link AbstractFieldDetails.parameters}. A `null` value is equivalent to a
    * empty map of parameters.
@@ -66,6 +67,11 @@ public class Folks.PhoneFieldDetails : AbstractFieldDetails<string>
   public PhoneFieldDetails (string value,
       MultiMap<string, string>? parameters = null)
     {
+      if (value == "")
+        {
+          warning ("Empty phone number passed to PhoneFieldDetails.");
+        }
+
       Object (value: value,
               parameters: parameters);
     }

diff --git a/folks/postal-address-details.vala b/folks/postal-address-details.vala
index 6a6c8cc..b575a76 100644 (file)
--- a/folks/postal-address-details.vala
+++ b/folks/postal-address-details.vala
@@ -174,6 +174,26 @@ public class Folks.PostalAddress : Object
     }
 
   /**
+   * Whether none of the components is set.
+   *
+   * @return `true` if all the components are the empty string, `false`
+   * otherwise.
+   *
+   * @since UNRELEASED
+   */
+  public bool is_empty ()
+    {
+      return this.po_box == "" &&
+             this.extension == "" &&
+             this.street == "" &&
+             this.locality == "" &&
+             this.region == "" &&
+             this.postal_code == "" &&
+             this.country == "" &&
+             this.address_format == "";
+    }
+
+  /**
    * Compare if two postal addresses are equal. Addresses are equal if all their
    * components are equal (where `null` compares equal only with `null`) and
    * they have the same set of types (or both have no types).
@@ -246,7 +266,7 @@ public class Folks.PostalAddressFieldDetails :
   /**
    * Create a new PostalAddressFieldDetails.
    *
-   * @param value the value of the field
+   * @param value the value of the field, a non-empty {@link PostalAddress}
    * @param parameters initial parameters. See
    * {@link AbstractFieldDetails.parameters}. A `null` value is equivalent to an
    * empty map of parameters.
@@ -259,6 +279,11 @@ public class Folks.PostalAddressFieldDetails :
   public PostalAddressFieldDetails (PostalAddress value,
       MultiMap<string, string>? parameters = null)
     {
+      if (value.is_empty ())
+        {
+          warning ("Empty postal address passed to PostalAddressFieldDetails.");
+        }
+
       /* We keep id and value.uid synchronised in both directions. */
       Object (value: value,
               parameters: parameters,

diff --git a/folks/role-details.vala b/folks/role-details.vala
index a910c92..b720148 100644 (file)
--- a/folks/role-details.vala
+++ b/folks/role-details.vala
@@ -98,6 +98,21 @@ public class Folks.Role : Object
     }
 
   /**
+   * Whether none of the components is set.
+   *
+   * @return `true` if all the components are the empty string, `false`
+   * otherwise.
+   *
+   * @since UNRELEASED
+   */
+  public bool is_empty ()
+    {
+      return this.organisation_name == "" &&
+             this.title == "" &&
+             this.role == "";
+    }
+
+  /**
    * Compare if two roles are equal. Roles are equal if their titles and
    * organisation names are equal.
    *
@@ -165,7 +180,7 @@ public class Folks.RoleFieldDetails : AbstractFieldDetails<Role>
   /**
    * Create a new RoleFieldDetails.
    *
-   * @param value the {@link Role} of the field
+   * @param value the non-empty {@link Role} of the field
    * @param parameters initial parameters. See
    * {@link AbstractFieldDetails.parameters}. A `null` value is equivalent to an
    * empty map of parameters.
@@ -177,6 +192,11 @@ public class Folks.RoleFieldDetails : AbstractFieldDetails<Role>
   public RoleFieldDetails (Role value,
       MultiMap<string, string>? parameters = null)
     {
+      if (value.is_empty ())
+        {
+          warning ("Empty role passed to RoleFieldDetails.");
+        }
+
       /* We keep id and value.uid synchronised in both directions. */
       Object (value: value,
               parameters: parameters,

diff --git a/folks/url-details.vala b/folks/url-details.vala
index 501588d..8b44eb3 100644 (file)
--- a/folks/url-details.vala
+++ b/folks/url-details.vala
@@ -74,7 +74,7 @@ public class Folks.UrlFieldDetails : AbstractFieldDetails<string>
   /**
    * Create a new UrlFieldDetails.
    *
-   * @param value the value of the field
+   * @param value the value of the field, a non-empty URI
    * @param parameters initial parameters. See
    * {@link AbstractFieldDetails.parameters}. A `null` value is equivalent to a
    * empty map of parameters.
@@ -86,6 +86,11 @@ public class Folks.UrlFieldDetails : AbstractFieldDetails<string>
   public UrlFieldDetails (string value,
       MultiMap<string, string>? parameters = null)
     {
+      if (value == "")
+        {
+          warning ("Empty URI passed to UrlFieldDetails.");
+        }
+
       Object (value: value,
               parameters: parameters);
     }

diff --git a/folks/web-service-details.vala b/folks/web-service-details.vala
index 4717183..c37bbb8 100644 (file)
--- a/folks/web-service-details.vala
+++ b/folks/web-service-details.vala
@@ -35,7 +35,7 @@ public class Folks.WebServiceFieldDetails : AbstractFieldDetails<string>
   /**
    * Create a new WebServiceFieldDetails.
    *
-   * @param value the value of the field
+   * @param value the value of the field, a non-empty web service address
    * @param parameters initial parameters. See
    * {@link AbstractFieldDetails.parameters}. A `null` value is equivalent to an
    * empty map of parameters.
@@ -47,6 +47,12 @@ public class Folks.WebServiceFieldDetails : AbstractFieldDetails<string>
   public WebServiceFieldDetails (string value,
       MultiMap<string, string>? parameters = null)
     {
+      if (value == "")
+        {
+          warning ("Empty web service address passed to " +
+              "WebServiceFieldDetails.");
+        }
+
       Object (value: value,
               parameters: parameters);
     }