[rygel/wip/basic-management: 75/95] core: Handle BasicManagement argument errors consistently



commit e8b70d7a761d2237faaa1aae9a6a35bb4b62c0ce
Author: Jussi Kukkonen <jussi kukkonen intel com>
Date:   Wed Jun 5 14:05:28 2013 +0300

    core: Handle BasicManagement argument errors consistently
    
    Return Error_Other if the arguments are not within implementation
    specific limits. Test initialization is no longer handled by specific
    init() function: instead the tests constructed() checks argument
    values and sets a enum InitState to proper value.
    
    The InitState enum exists so that ExecutionState could map 1:1 to the
    UPnP state variable without ugly hacks.
    
    This approach is simpler for BasicManagement (one less method call) and
    produces arguably better results: at least the caller of the UPnP api
    now gets meaningful error messages.

 .../rygel-basic-management-test-nslookup.vala      |   69 ++++++---
 .../rygel-basic-management-test-ping.vala          |  170 +++++++++++++-------
 .../rygel-basic-management-test-traceroute.vala    |  161 ++++++++++++-------
 src/librygel-core/rygel-basic-management-test.vala |   55 ++++---
 src/librygel-core/rygel-basic-management.vala      |   44 ++----
 5 files changed, 306 insertions(+), 193 deletions(-)
---
diff --git a/src/librygel-core/rygel-basic-management-test-nslookup.vala 
b/src/librygel-core/rygel-basic-management-test-nslookup.vala
index c8f0bca..a440f3f 100644
--- a/src/librygel-core/rygel-basic-management-test-nslookup.vala
+++ b/src/librygel-core/rygel-basic-management-test-nslookup.vala
@@ -44,7 +44,8 @@ internal class Rygel.BasicManagementTestNSLookup : BasicManagementTest {
     private enum GenericStatus {
         SUCCESS,
         ERROR_DNS_SERVER_NOT_RESOLVED,
-        ERROR_INTERNAL;
+        ERROR_INTERNAL,
+        ERROR_OTHER;
         
         public string to_string() {
             switch (this) {
@@ -54,6 +55,8 @@ internal class Rygel.BasicManagementTestNSLookup : BasicManagementTest {
                     return "Error_DNSServerNotResolved";
                 case ERROR_INTERNAL:
                     return "Error_Internal";
+                case ERROR_OTHER:
+                    return "Error_Other";
                 default:
                     assert_not_reached();
             }
@@ -146,6 +149,10 @@ internal class Rygel.BasicManagementTestNSLookup : BasicManagementTest {
         }
     }
 
+    public string host_name { construct; private get; default = ""; }
+    public string? name_server { construct; private get; default = null; }
+    public uint interval_time_out { construct; private get; default = MIN_INTERVAL_TIMEOUT; }
+
     private Result[] results;
     private GenericStatus generic_status;
     private string additional_info;
@@ -154,29 +161,43 @@ internal class Rygel.BasicManagementTestNSLookup : BasicManagementTest {
     public override string method_type { get { return "NSLookup"; } }
     public override string results_type { get { return "GetNSLookupResult"; } }
 
-    public void init(string host_name, string? name_server, uint repetitions,
-                     uint32 interval_time_out) throws BasicManagementTestError {
-        this.command = { "nslookup" };
+    public BasicManagementTestNSLookup(string host_name,
+                                       string? name_server,
+                                       uint repetitions,
+                                       uint32 interval_time_out) {
+        Object (host_name: host_name,
+                name_server: name_server,
+                repetitions: repetitions,
+                interval_time_out: interval_time_out);
+    }
+
+    public override void constructed () {
+        base.constructed ();
+
         this.generic_status = GenericStatus.ERROR_INTERNAL;
         this.additional_info = "";
         this.results = {};
 
-        /* TODO should invalid values fail now instead of just limit ? */
-        repetitions = uint.max (1, repetitions);
-        this.repetitions = uint.min (repetitions, MAX_REPEAT_COUNT);
-
-        interval_time_out = uint.max (MIN_INTERVAL_TIMEOUT, interval_time_out);
-        interval_time_out = uint.min (interval_time_out, MAX_INTERVAL_TIMEOUT);
-        this.command += ("-timeout=%u").printf (interval_time_out/1000);
-
-        if (host_name == null || host_name.length < 1)
-            throw new BasicManagementTestError.INIT_FAILED
-                                                ("Host name is required");
+        this.command = { "nslookup" };
+        this.command += ("-timeout=%u").printf (this.interval_time_out/1000);
         this.command += host_name;
-
         if (name_server != null && name_server.length > 0)
             this.command += name_server;
 
+        /* Fail early if internal parameter limits are violated */
+        if (this.repetitions > MAX_REPEAT_COUNT) {
+            init_state = InitState.INVALID_PARAMETER;
+            var msg = "NumberOfRepetitions %u is not in allowed range [0, %u]";
+            this.additional_info = msg.printf (this.repetitions,
+                                               MAX_REPEAT_COUNT);
+        } else if (this.interval_time_out < MIN_INTERVAL_TIMEOUT ||
+                   this.interval_time_out > MAX_INTERVAL_TIMEOUT) {
+            init_state = InitState.INVALID_PARAMETER;
+            var msg = "Timeout %u is not in allowed range [%u, %u]";
+            this.additional_info = msg.printf (this.interval_time_out,
+                                               MIN_INTERVAL_TIMEOUT,
+                                               MAX_INTERVAL_TIMEOUT);
+        }
     }
 
     protected override void init_iteration () {
@@ -196,19 +217,25 @@ internal class Rygel.BasicManagementTestNSLookup : BasicManagementTest {
     }
 
     protected override void finish_iteration () {
-        switch (this.execution_state) {
-            case ExecutionState.SPAWN_FAILED:
+        switch (this.init_state) {
+            case InitState.SPAWN_FAILED:
+                /* quitting early */
                 this.generic_status = GenericStatus.ERROR_INTERNAL;
-                this.additional_info = "Failed spawn nslookup";
+                this.additional_info = "Failed to spawn nslookup";
+                this.results[results.length - 1].status = 
+                                        ResultStatus.ERROR_OTHER;
+                break;
+            case InitState.INVALID_PARAMETER:
+                /* quitting early */
+                /* constructed() has set info already */
+                this.generic_status = GenericStatus.ERROR_OTHER;
                 this.results[results.length - 1].status = 
                                         ResultStatus.ERROR_OTHER;
-
                 break;
             default:
                 var elapsed_msec = this.timer.elapsed (null) * 1000;
                 var execution_time = (uint)Math.round(elapsed_msec);
                 this.results[results.length - 1].execution_time = execution_time;
-
                 break;
         }
 
diff --git a/src/librygel-core/rygel-basic-management-test-ping.vala 
b/src/librygel-core/rygel-basic-management-test-ping.vala
index bd20efb..6c0877c 100644
--- a/src/librygel-core/rygel-basic-management-test-ping.vala
+++ b/src/librygel-core/rygel-basic-management-test-ping.vala
@@ -25,6 +25,18 @@ using GLib;
 
 // Helper class for BasicManagementTestPing.
 internal class Rygel.BasicManagementTestPing : BasicManagementTest {
+    private static const uint MAX_REPEAT_COUNT = 100;
+    private static const uint DEFAULT_REPEAT_COUNT = 1;
+    private static const uint DEFAULT_REPLY_TIMEOUT = 10000;
+    private static const uint MIN_REQUEST_INTERVAL_TIMEOUT = 1000;
+    private static const uint MAX_REQUEST_INTERVAL_TIMEOUT = 30000;
+    private static const uint DEFAULT_REQUEST_INTERVAL_TIMEOUT = 10000;
+    private static const uint MIN_DATA_BLOCK_SIZE = 20;
+    private static const uint MAX_DATA_BLOCK_SIZE = 2048;
+    private static const uint DEFAULT_DATA_BLOCK_SIZE = 32;
+    private static const uint MAX_DSCP = 64;
+    private static const uint DEFAULT_DSCP = 30;
+
     private enum ProcessState {
         INIT,
         STATISTICS,
@@ -53,6 +65,53 @@ internal class Rygel.BasicManagementTestPing : BasicManagementTest {
         }
     }
 
+    public string host { construct; get; default = ""; }
+
+    private uint _repeat_count;
+    public uint repeat_count {
+        construct {
+            this._repeat_count = value;
+            if (this._repeat_count == 0)
+                this._repeat_count = DEFAULT_REPEAT_COUNT;
+        }
+        get { return this._repeat_count; }
+        default = DEFAULT_REPEAT_COUNT;
+    }
+
+    private uint _data_block_size;
+    public uint data_block_size {
+        construct {
+            this._data_block_size = value;
+            if (this._data_block_size == 0)
+                this._data_block_size = DEFAULT_DATA_BLOCK_SIZE;
+        }
+        get { return this._data_block_size; }
+        default = DEFAULT_DATA_BLOCK_SIZE;
+    }
+
+    private uint _dscp;
+    public uint dscp {
+        construct {
+            this._dscp = value;
+            if (this._dscp == 0)
+                this._dscp = DEFAULT_DSCP;
+        }
+        get { return this._dscp; }
+        default = DEFAULT_DSCP;
+    }
+
+    private uint32 _interval_time_out;
+    public uint32 interval_time_out {
+        construct {
+            this._interval_time_out = value;
+            if (this._interval_time_out == 0)
+                this._interval_time_out = DEFAULT_REQUEST_INTERVAL_TIMEOUT;
+        }
+        get { return _interval_time_out; }
+        default = DEFAULT_REQUEST_INTERVAL_TIMEOUT;
+    }
+
+
     private ProcessState state;
     private Status status;
     private string additional_info;
@@ -62,28 +121,26 @@ internal class Rygel.BasicManagementTestPing : BasicManagementTest {
     private uint32 min_response_time;
     private uint32 max_response_time;
 
-    private static const uint MIN_REPEAT_COUNT = 1;
-    private static const uint MAX_REPEAT_COUNT = 100;
-    private static const uint DEFAULT_REPEAT_COUNT = 1;
-    private static const uint DEFAULT_REPLY_TIMEOUT = 10000;
-    private static const uint MIN_REQUEST_INTERVAL_TIMEOUT = 1000;
-    private static const uint MAX_REQUEST_INTERVAL_TIMEOUT = 30000;
-    private static const uint DEFAULT_REQUEST_INTERVAL_TIMEOUT = 10000;
-    private static const uint MIN_DATA_BLOCK_SIZE = 20;
-    private static const uint MAX_DATA_BLOCK_SIZE = 2048;
-    private static const uint DEFAULT_DATA_BLOCK_SIZE = 32;
-    private static const uint MIN_DSCP = 1;
-    private static const uint MAX_DSCP = 64;
-    private static const uint DEFAULT_DSCP = 30;
-
     public override string method_type { get { return "Ping"; } }
     public override string results_type { get { return "GetPingResult"; } }
 
-    public void init(string host, uint repeat_count, uint data_block_size,
-                     uint dscp, uint32 interval_time_out)
-                     throws BasicManagementTestError {
-        this.command = { "ping" };
+    public BasicManagementTestPing (string host,
+                                    uint repeat_count,
+                                    uint data_block_size,
+                                    uint dscp,
+                                    uint32 interval_time_out) {
+        Object (host: host,
+                repeat_count: repeat_count,
+                data_block_size: data_block_size,
+                dscp: dscp,
+                interval_time_out: interval_time_out);
+    }
+
+    public override void constructed () {
+        base.constructed ();
+
         this.status = Status.ERROR_INTERNAL;
+        this.state = ProcessState.INIT;
         this.additional_info = "";
         this.success_count = 0;
         this.failure_count = 0;
@@ -91,62 +148,51 @@ internal class Rygel.BasicManagementTestPing : BasicManagementTest {
         this.min_response_time = 0;
         this.max_response_time = 0;
 
-        if (repeat_count == 0) {
-            repeat_count = DEFAULT_REPEAT_COUNT;
-        } else if (repeat_count < MIN_REPEAT_COUNT &&
-                   repeat_count > MAX_REPEAT_COUNT) {
-            throw new BasicManagementTestError.INIT_FAILED
-                                                ("Invalid repeat count");
-        }
+        this.command = { "ping" };
         this.command += ("-c %u").printf (repeat_count);
-
         this.command += ("-W %u").printf (DEFAULT_REPLY_TIMEOUT/1000);
-
-        if (interval_time_out == 0) {
-            interval_time_out = DEFAULT_REQUEST_INTERVAL_TIMEOUT;
-        } else if (interval_time_out < MIN_REQUEST_INTERVAL_TIMEOUT &&
-                   interval_time_out > MAX_REQUEST_INTERVAL_TIMEOUT) {
-            throw new BasicManagementTestError.INIT_FAILED
-                                                ("Invalid interval timeout");
-        }
         this.command += ("-i %u").printf (interval_time_out/1000);
-
-        if (data_block_size == 0) {
-            data_block_size = DEFAULT_DATA_BLOCK_SIZE;
-        } else if (data_block_size < MIN_DATA_BLOCK_SIZE &&
-                   data_block_size > MAX_DATA_BLOCK_SIZE) {
-            throw new BasicManagementTestError.INIT_FAILED
-                                                ("Invalid data block size");
-        }
         this.command += ("-s %u").printf (data_block_size);
-
-        if (dscp == 0) {
-            dscp = DEFAULT_DSCP;
-        } else if (dscp < MIN_DSCP && dscp > MAX_DSCP) {
-            throw new BasicManagementTestError.INIT_FAILED
-                                                ("Invalid DSCP");
-        }
         this.command += ("-Q %u").printf (dscp >> 2);
-
-        if (host == null || host.length < 1) {
-            throw new BasicManagementTestError.INIT_FAILED
-                                                ("Host name is required");
-        }
-
         this.command += host;
-    }
 
-    protected override void init_iteration () {
-        base.init_iteration ();
+        if (this.repeat_count > MAX_REPEAT_COUNT) {
+            this.init_state = InitState.INVALID_PARAMETER;
+            this.status = Status.ERROR_OTHER;
+            var msg = "NumberOfRepetitions %u is not in allowed range [0, %u]";
+            this.additional_info = msg.printf (this.repeat_count,
+                                               MAX_REPEAT_COUNT);
 
-        this.state = ProcessState.INIT;
+        } else if (this.interval_time_out < MIN_REQUEST_INTERVAL_TIMEOUT ||
+                   this.interval_time_out > MAX_REQUEST_INTERVAL_TIMEOUT) {
+            this.init_state = InitState.INVALID_PARAMETER;
+            this.status = Status.ERROR_OTHER;
+            var msg = "Timeout %u is not in allowed range [%u, %u]";
+            this.additional_info = msg.printf (this.interval_time_out,
+                                               MIN_REQUEST_INTERVAL_TIMEOUT,
+                                               MAX_REQUEST_INTERVAL_TIMEOUT);
+
+        } else if (this.data_block_size < MIN_DATA_BLOCK_SIZE ||
+                   this.data_block_size > MAX_DATA_BLOCK_SIZE) {
+            this.init_state = InitState.INVALID_PARAMETER;
+            this.status = Status.ERROR_OTHER;
+            var msg = "DataBlockSize %u is not in allowed range [%u, %u]";
+            this.additional_info = msg.printf (this.data_block_size,
+                                               MIN_DATA_BLOCK_SIZE,
+                                               MAX_DATA_BLOCK_SIZE);
+        } else if (this.dscp > MAX_DSCP) {
+            this.init_state = InitState.INVALID_PARAMETER;
+            this.status = Status.ERROR_OTHER;
+            var msg = "DSCP %u is not in allowed range [0, %u]";
+            this.additional_info = msg.printf (this.dscp, MAX_DSCP);
+        }
     }
 
     protected override void finish_iteration () {
-        switch (this.execution_state) {
-            case ExecutionState.SPAWN_FAILED:
+        switch (this.init_state) {
+            case InitState.SPAWN_FAILED:
                 this.status = Status.ERROR_INTERNAL;
-                this.additional_info = "Failed spawn ping";
+                this.additional_info = "Failed to spawn ping";
                 break;
             default:
                 break;
diff --git a/src/librygel-core/rygel-basic-management-test-traceroute.vala 
b/src/librygel-core/rygel-basic-management-test-traceroute.vala
index aa968b7..f575025 100644
--- a/src/librygel-core/rygel-basic-management-test-traceroute.vala
+++ b/src/librygel-core/rygel-basic-management-test-traceroute.vala
@@ -25,78 +25,119 @@ using GLib;
 
 // Helper class for BasicManagementTestTraceroute.
 internal class Rygel.BasicManagementTestTraceroute : BasicManagementTest {
+    private static const uint MIN_TIMEOUT = 1000;
+    private static const uint MAX_TIMEOUT = 30000;
+    private static const uint DEFAULT_TIMEOUT = 5000;
+    private static const uint MIN_DATA_BLOCK_SIZE = 20;
+    private static const uint MAX_DATA_BLOCK_SIZE = 2048;
+    private static const uint DEFAULT_DATA_BLOCK_SIZE = 32;
+    private static const uint MAX_DSCP = 64;
+    private static const uint DEFAULT_DSCP = 30;
+    private static const uint MAX_HOPS = 64;
+    private static const uint DEFAULT_HOPS = 30;
+    private static const uint MAX_HOSTS = 2048;
+    private static const uint MAX_RESULT_SIZE = 4;
+
+    public string host { construct; get; default = ""; }
+
+    private uint32 _wait_time_out;
+    public uint32 wait_time_out {
+        construct {
+            this._wait_time_out = value;
+            if (this._wait_time_out == 0)
+                this._wait_time_out = DEFAULT_TIMEOUT;
+        }
+        get { return this._wait_time_out; }
+        default = DEFAULT_TIMEOUT;
+    }
 
-    private string host;
-    private uint32 wait_time_out;
-    private uint data_block_size;
-    private uint max_hop_count;
-    private uint dscp;
+    private uint _data_block_size;
+    public uint data_block_size {
+        construct {
+            this._data_block_size = value;
+            if (this._data_block_size == 0)
+                this._data_block_size = DEFAULT_DATA_BLOCK_SIZE;
+        }
+        get { return this._data_block_size; }
+        default = DEFAULT_DATA_BLOCK_SIZE;
+    }
+
+    private uint _max_hop_count;
+    public uint max_hop_count {
+        construct {
+            this._max_hop_count = value;
+            if (this._max_hop_count == 0)
+                this._max_hop_count = DEFAULT_HOPS;
+        }
+        get { return this._max_hop_count; }
+        default = DEFAULT_HOPS;
+    }
+
+    private uint _dscp;
+    public uint dscp {
+        construct {
+            this._dscp = value;
+            if (this._dscp == 0)
+                this._dscp = DEFAULT_DSCP;
+        }
+        get { return this._dscp; }
+        default = DEFAULT_DSCP;
+    }
 
     private string status;
     private string additional_info;
     private uint32 response_time;
     private string hop_hosts;
 
-    private static const uint TRACEROUTE_MIN_REQUEST_TIMEOUT = 1000;
-    private static const uint TRACEROUTE_MAX_REQUEST_TIMEOUT = 30000;
-    private static const uint TRACEROUTE_DEFAULT_REQUEST_TIMEOUT = 5000;
-    private static const uint TRACEROUTE_MIN_DATA_BLOCK_SIZE = 20;
-    private static const uint TRACEROUTE_MAX_DATA_BLOCK_SIZE = 2048;
-    private static const uint TRACEROUTE_DEFAULT_DATA_BLOCK_SIZE = 32;
-    private static const uint TRACEROUTE_MIN_DSCP = 1;
-    private static const uint TRACEROUTE_MAX_DSCP = 64;
-    private static const uint TRACEROUTE_DEFAULT_DSCP = 30;
-    private static const uint TRACEROUTE_MIN_HOPS = 1;
-    private static const uint TRACEROUTE_MAX_HOPS = 64;
-    private static const uint TRACEROUTE_DEFAULT_HOPS = 30;
-    private static const uint TRACEROUTE_MAX_HOSTS = 2048;
-    private static const uint TRACEROUTE_MAX_RESULT_SIZE = 4;
-
     public override string method_type { get { return "Traceroute"; } }
     public override string results_type { get { return "GetTracerouteResult"; } }
 
-    public bool init(string host, uint32 wait_time_out, uint data_block_size,
-                     uint max_hop_count, uint dscp) {
-        stdout.printf("*Traceroute* init()\n");
-
-        if (host == null || host == "") {
-            return false;
-        }
-
-        if (wait_time_out == 0) {
-            wait_time_out = TRACEROUTE_DEFAULT_REQUEST_TIMEOUT;
-        } else if (wait_time_out < TRACEROUTE_MIN_REQUEST_TIMEOUT &&
-                   wait_time_out > TRACEROUTE_MAX_REQUEST_TIMEOUT) {
-            return false;
-        }
-
-        if (data_block_size == 0) {
-            data_block_size = TRACEROUTE_DEFAULT_DATA_BLOCK_SIZE;
-        } else if (data_block_size < TRACEROUTE_MIN_DATA_BLOCK_SIZE &&
-                   data_block_size > TRACEROUTE_MAX_DATA_BLOCK_SIZE) {
-            return false;
-        }
-
-        if (max_hop_count == 0) {
-            max_hop_count = TRACEROUTE_DEFAULT_HOPS;
-        } else if (max_hop_count < TRACEROUTE_MIN_HOPS &&
-                 max_hop_count > TRACEROUTE_MAX_HOPS) {
-            return false;
-        }
+    public BasicManagementTestTraceroute (string host,
+                                          uint32 wait_time_out,
+                                          uint data_block_size,
+                                          uint max_hop_count,
+                                          uint dscp) {
+        Object (host: host,
+                wait_time_out: wait_time_out,
+                data_block_size: data_block_size,
+                max_hop_count: max_hop_count,
+                dscp: dscp);
+    }
 
-        if (dscp == 0) {
-            dscp = TRACEROUTE_DEFAULT_DSCP;
-        } else if (dscp < TRACEROUTE_MIN_DSCP && dscp > TRACEROUTE_MAX_DSCP) {
-            return false;
+    public override void constructed () {
+        base.constructed ();
+
+        stdout.printf("*Traceroute* constructed()\n");
+
+        /* Fail early if internal parameter limits are violated */
+        if (this.wait_time_out < MIN_TIMEOUT ||
+            this.wait_time_out > MAX_TIMEOUT) {
+
+            this.init_state = InitState.INVALID_PARAMETER;
+            var msg = "Timeout %u is not in allowed range [%u, %u]";
+            this.additional_info = msg.printf (this.wait_time_out,
+                                               MIN_TIMEOUT,
+                                               MAX_TIMEOUT);
+
+        } else if (this.data_block_size < MIN_DATA_BLOCK_SIZE ||
+                   this.data_block_size > MAX_DATA_BLOCK_SIZE) {
+            this.init_state = InitState.INVALID_PARAMETER;
+            var msg = "DataBlockSize %u is not in allowed range [%u, %u]";
+            this.additional_info = msg.printf (this.data_block_size,
+                                               MIN_DATA_BLOCK_SIZE,
+                                               MAX_DATA_BLOCK_SIZE);
+
+        } else if (this.max_hop_count > MAX_HOPS) {
+            this.init_state = InitState.INVALID_PARAMETER;
+            var msg = "MaxHopCount %u is not in allowed range [0, %u]";
+            this.additional_info = msg.printf (this.max_hop_count,
+                                               MAX_HOPS);
+
+        } else if (this.dscp > MAX_DSCP) {
+            this.init_state = InitState.INVALID_PARAMETER;
+            var msg = "DSCP %u is not in allowed range [0, %u]";
+            this.additional_info = msg.printf (this.dscp, MAX_DSCP);
         }
-
-        this.host = host;
-        this.wait_time_out = wait_time_out;
-        this.data_block_size = data_block_size;
-        this.max_hop_count = max_hop_count;
-        this.dscp = dscp;
-
-        return true;
     }
 
     public void get_results(out string status, out string additional_info,
diff --git a/src/librygel-core/rygel-basic-management-test.vala 
b/src/librygel-core/rygel-basic-management-test.vala
index bcde4d5..eca44d3 100644
--- a/src/librygel-core/rygel-basic-management-test.vala
+++ b/src/librygel-core/rygel-basic-management-test.vala
@@ -25,16 +25,21 @@ using GLib;
 
 
 internal errordomain Rygel.BasicManagementTestError {
-    NOT_POSSIBLE,
-    INIT_FAILED,
+    NOT_POSSIBLE
 }
 
 internal abstract class Rygel.BasicManagementTest : Object {
+    protected enum InitState {
+        OK,
+        SPAWN_FAILED,
+        INVALID_PARAMETER,
+    }
+    protected InitState init_state;
+
     public enum ExecutionState {
         REQUESTED,
         IN_PROGRESS,
         COMPLETED,
-        SPAWN_FAILED,
         CANCELED;
 
         /* Return values fit for A_ARG_TYPE_TestState */
@@ -46,8 +51,6 @@ internal abstract class Rygel.BasicManagementTest : Object {
                     return "InProgress";
                 case COMPLETED:
                     return "Completed";
-                case SPAWN_FAILED:
-                    return "Completed";
                 case CANCELED:
                     return "Canceled";
                 default:
@@ -56,8 +59,11 @@ internal abstract class Rygel.BasicManagementTest : Object {
         }
     }
 
-    public ExecutionState execution_state;
-    
+    public ExecutionState execution_state {
+        get;
+        protected set; 
+        default = ExecutionState.REQUESTED;
+    }
     public string id;
 
     /* properties implementations need to provide */
@@ -65,10 +71,10 @@ internal abstract class Rygel.BasicManagementTest : Object {
     public abstract string results_type { get; }
 
     /* properties for implementations to access */
+    public uint repetitions { construct; protected get; default = 1; }
     protected SpawnFlags flags = SpawnFlags.SEARCH_PATH |
                                  SpawnFlags.LEAVE_DESCRIPTORS_OPEN;
     protected string[] command;
-    protected uint repetitions;
 
     private uint eof_count;
     private int std_out;
@@ -90,9 +96,15 @@ internal abstract class Rygel.BasicManagementTest : Object {
     }
     protected virtual void finish_iteration () {
         this.iteration++;
-        if (this.execution_state != ExecutionState.IN_PROGRESS) {
-            this.async_callback ();
-        } else if (this.iteration >= this.repetitions) {
+
+        if (this.init_state != InitState.OK ||
+            this.execution_state == ExecutionState.COMPLETED ||
+            this.iteration >= this.repetitions) {
+            /* No more iterations if 
+             *  - init failed, recovery is impossible or
+             *  - execution has ended (remaining iterations should be skipped)
+             *  - the specified nr of iterations have been executed already
+             */
             this.execution_state = ExecutionState.COMPLETED;
             this.async_callback ();
         } else {
@@ -111,8 +123,16 @@ internal abstract class Rygel.BasicManagementTest : Object {
     }
 
     private void run_iteration () {
+        this.init_iteration ();
+
+        /*if we failed to initialize, skip spawning */
+        if (this.init_state != InitState.OK) {
+            Idle.add ((SourceFunc)this.finish_iteration);
+            return;
+        }
+
         try {
-            this.init_iteration ();
+
             this.eof_count = 0;
             Process.spawn_async_with_pipes (null,
                                             this.command,
@@ -132,8 +152,9 @@ internal abstract class Rygel.BasicManagementTest : Object {
             err_channel.add_watch (IOCondition.OUT | IOCondition.HUP,
                                    this.err_watch);
         } catch (SpawnError e) {
-            /* Let the async function yeild, then error out */
-            this.execution_state = ExecutionState.SPAWN_FAILED;
+            /* Let the async function yeild, then let the Test 
+             * implementation handle this in finish_iteration */
+            this.init_state = InitState.SPAWN_FAILED;
             Idle.add ((SourceFunc)this.finish_iteration);
         }
     }
@@ -188,12 +209,6 @@ internal abstract class Rygel.BasicManagementTest : Object {
         return true;
     }
 
-
-    public BasicManagementTest() {
-        this.execution_state = ExecutionState.REQUESTED;
-        this.id = null;
-    }
-
     public async virtual void execute () throws BasicManagementTestError {
         if (this.execution_state != ExecutionState.REQUESTED)
             throw new BasicManagementTestError.NOT_POSSIBLE
diff --git a/src/librygel-core/rygel-basic-management.vala b/src/librygel-core/rygel-basic-management.vala
index 538c88b..7436b00 100644
--- a/src/librygel-core/rygel-basic-management.vala
+++ b/src/librygel-core/rygel-basic-management.vala
@@ -118,8 +118,7 @@ public class Rygel.BasicManagement : Service {
             this.notify ("ActiveTestIDs", typeof (string),
                          create_test_ids_list (true));
         } else if ((execution_state == BasicManagementTest.ExecutionState.CANCELED) ||
-                   (execution_state == BasicManagementTest.ExecutionState.COMPLETED) ||
-                   (execution_state == BasicManagementTest.ExecutionState.SPAWN_FAILED)) {
+                   (execution_state == BasicManagementTest.ExecutionState.COMPLETED)) {
             this.active_tests_map.unset (bm_test.id);
             this.notify ("ActiveTestIDs", typeof (string),
                          create_test_ids_list (true));
@@ -277,16 +276,10 @@ public class Rygel.BasicManagement : Service {
                         typeof (uint),
                         out dscp);
 
-        var ping = new BasicManagementTestPing();
-        try {
-            ping.init (host, repeat_count, interval_time_out, data_block_size,
-                       dscp);
-
-            this.add_test_and_return_action (ping as BasicManagementTest,
-                                             action);
-        } catch (BasicManagementTestError e) {
-            action.return_error (402, _("Invalid argument"));
-        }
+        var ping = new BasicManagementTestPing(host, repeat_count,
+                                               interval_time_out,
+                                               data_block_size, dscp);
+        this.add_test_and_return_action (ping as BasicManagementTest, action);
     }
 
     private void ping_result_cb (Service             cm,
@@ -366,16 +359,12 @@ public class Rygel.BasicManagement : Service {
                         typeof (uint32),
                         out interval_time_out);
 
-        var nslookup = new BasicManagementTestNSLookup();
-        try {
-            nslookup.init (hostname, dns_server, repeat_count,
-                           interval_time_out);
-
-            this.add_test_and_return_action (nslookup as BasicManagementTest,
-                                             action);
-        } catch (BasicManagementTestError e) {
-            action.return_error (402, _("Invalid argument"));
-        }
+        var nslookup = new BasicManagementTestNSLookup(hostname,
+                                                       dns_server,
+                                                       repeat_count,
+                                                       interval_time_out);
+        this.add_test_and_return_action (nslookup as BasicManagementTest,
+                                         action);
     }
 
     private void nslookup_result_cb (Service             cm,
@@ -444,14 +433,9 @@ public class Rygel.BasicManagement : Service {
                         typeof (uint),
                         out dscp);
 
-        var traceroute = new BasicManagementTestTraceroute();
-        if (!traceroute.init (host, wait_time_out, data_block_size,
-                              max_hop_count, dscp)) {
-            action.return_error (402, _("Invalid argument"));
-
-            return;
-        }
-
+        var traceroute = new BasicManagementTestTraceroute(host, wait_time_out,
+                                                           data_block_size,
+                                                           max_hop_count, dscp);
         this.add_test_and_return_action (traceroute as BasicManagementTest,
                                          action);
     }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]