From bd7f2f3a08b8b109922ca7213abf5a79dff110e1 Mon Sep 17 00:00:00 2001
From: Per von Zweigbergk <pvz@itassistans.se>
Date: Mon, 8 Aug 2016 17:14:30 +0200
Subject: [PATCH 01/12] First version of ApiMutableTableModelControllerBase
 class

---
 .../ApiMutableTableModelControllerBase.php    | 79 +++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100644 src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php

diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
new file mode 100644
index 000000000..3f94354d0
--- /dev/null
+++ b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
@@ -0,0 +1,79 @@
+<?php
+/**
+ *    Copyright (C) 2016 IT-assistans Sverige AB
+ *    Copyright (C) 2016 Deciso B.V.
+ *
+ *    All rights reserved.
+ *
+ *    Redistribution and use in source and binary forms, with or without
+ *    modification, are permitted provided that the following conditions are met:
+ *
+ *    1. Redistributions of source code must retain the above copyright notice,
+ *       this list of conditions and the following disclaimer.
+ *
+ *    2. Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in the
+ *       documentation and/or other materials provided with the distribution.
+ *
+ *    THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ *    INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ *    AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ *    AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
+ *    OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ *    SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ *    INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ *    CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ *    ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ *    POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+namespace OPNsense\Base;
+
+
+/**
+ * Class ApiMutableTableModelControllerBase, inherit this class to implement
+ * an API that exposes a model for tables.
+ * @package OPNsense\Base
+ */
+abstract class ApiMutableTableModelControllerBase extends ApiControllerBase
+{
+    static protected $modelPathPrefix = '';
+    private function getNodes() {
+        $ref = static::$modelPathPrefix
+             + static::$internalModelName;
+        return $this->getModel()->getNodeByReference($ref);
+    }
+    private function getNodeByUUID($uuid) {
+        return $this->getNodes()->$uuid;
+    }
+    public function getItemAction($uuid = null) {
+        $mdl = $this->getModel();
+        if ($uuid != null) {
+            $node = getNodeByUUID($uuid);
+            if ($node != null) {
+                // return node
+                return array(static::$internalModelName => $node->getNodes());
+            }
+        } else {
+            // generate new node, but don't save to disc
+            $node = getNodes()->add();
+            return array(static::$internalModelName => $node->getNodes());
+        }
+        return array();
+    }
+    public function setItemAction($uuid) {
+        // FIXME To be implemented
+    }
+    public function addItemAction() {
+        // FIXME To be implemented
+    }
+    public function delItemAction($uuid) {
+        // FIXME To be implemented
+    }
+    public function toggleItemAction($uuid, $enabled = null) {
+        // FIXME To be implemented
+    }
+    public function searchItemsAction() {
+        // FIXME To be implemented
+    }
+}

From 2d303f1573e781acf2aacf758de9696d4fb94b70 Mon Sep 17 00:00:00 2001
From: pv2b <pvz@pvz.pp.se>
Date: Mon, 8 Aug 2016 21:25:39 +0200
Subject: [PATCH 02/12] ApiMutableTableModelControllerBase implemented methods

---
 .../ApiMutableTableModelControllerBase.php    | 111 +++++++++++++++++-
 1 file changed, 105 insertions(+), 6 deletions(-)

diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
index 3f94354d0..d0a4f54c4 100644
--- a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
+++ b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
@@ -37,6 +37,8 @@ namespace OPNsense\Base;
  */
 abstract class ApiMutableTableModelControllerBase extends ApiControllerBase
 {
+	// FIXME What about validation?
+	
     static protected $modelPathPrefix = '';
     private function getNodes() {
         $ref = static::$modelPathPrefix
@@ -46,10 +48,16 @@ abstract class ApiMutableTableModelControllerBase extends ApiControllerBase
     private function getNodeByUUID($uuid) {
         return $this->getNodes()->$uuid;
     }
+
+	/**
+     * retrieve item or return defaults
+     * @param $uuid item unique id
+     * @return array
+     */
     public function getItemAction($uuid = null) {
         $mdl = $this->getModel();
         if ($uuid != null) {
-            $node = getNodeByUUID($uuid);
+            $node = $this->getNodeByUUID($uuid);
             if ($node != null) {
                 // return node
                 return array(static::$internalModelName => $node->getNodes());
@@ -61,19 +69,110 @@ abstract class ApiMutableTableModelControllerBase extends ApiControllerBase
         }
         return array();
     }
+	
+    /**
+     * update item with given properties
+     * @param $uuid item unique id
+     * @return array
+     */
     public function setItemAction($uuid) {
-        // FIXME To be implemented
+        if ($this->request->isPost() && $this->request->hasPost(static::$internalModelName)) {
+            $mdl = $this->getModel();
+            if ($uuid != null) {
+                $node = $this->getNodeByUUID($uuid);
+                if ($node != null) {
+                    $node->setNodes($this->request->getPost(static::$internalModelName));
+                    return $this->save($mdl, $node, static::$internalModelName);
+                }
+            }
+        }
+        return array("result"=>"failed");
     }
+
+    /**
+     * add new item and set with attributes from post
+     * @return array
+     */
     public function addItemAction() {
-        // FIXME To be implemented
+        $result = array("result"=>"failed");
+        if ($this->request->isPost() && $this->request->hasPost(static::$internalModelName))) {
+            $mdl = $this->getModel();
+			// FIXME Is this correct?
+			$node = $this->getNodes()->add();
+            $node->setNodes($this->request->getPost(static::$internalModelName)));
+			// FIXME What do we do with this? Is this traffic-shaper specific? Do we need a hook here?
+            $node->origin = "TrafficShaper"; // set origin to this component.
+            return $this->save($mdl, $node, static::$internalModelName));
+        }
+        return $result;
     }
+	
+    /**
+     * delete item by uuid
+     * @param $uuid item unique id
+     * @return array status
+     */
     public function delItemAction($uuid) {
-        // FIXME To be implemented
+        $result = array("result"=>"failed");
+        if ($this->request->isPost()) {
+            $mdl = $this->getModel();
+            if ($uuid != null) {
+                if (getNodes()->del($uuid)) {
+                    // if item is removed, serialize to config and save
+                    $mdlShaper->serializeToConfig();
+                    Config::getInstance()->save();
+                    $result['result'] = 'deleted';
+                } else {
+                    $result['result'] = 'not found';
+                }
+            }
+        }
+        return $result;
     }
+	
+    /**
+     * toggle item by uuid (enable/disable)
+     * @param $uuid item unique id
+     * @param $enabled desired state enabled(1)/disabled(1), leave empty for toggle
+     * @return array status
+     */
     public function toggleItemAction($uuid, $enabled = null) {
-        // FIXME To be implemented
+        $result = array("result" => "failed");
+        if ($this->request->isPost()) {
+            $mdl = $this->getModel();
+            if ($uuid != null) {
+                $node = $mdl->getNodeByUUID($uuid);
+                if ($node != null) {
+                    if ($enabled == "0" || $enabled == "1") {
+                        $node->enabled = (string)$enabled;
+                    } elseif ($node->enabled->__toString() == "1") {
+                        $node->enabled = "0";
+                    } else {
+                        $node->enabled = "1";
+                    }
+                    $result['result'] = $node->enabled;
+                    // if item has toggled, serialize to config and save
+                    $mdlShaper->serializeToConfig();
+                    Config::getInstance()->save();
+                }
+            }
+        }
+        return $result;
     }
+	
+    /**
+     * search items
+     * @return array
+     */
     public function searchItemsAction() {
-        // FIXME To be implemented
+        $this->sessionClose();
+        $mdl = $this->getModel();
+        $grid = new UIModelGrid($this->getNodes());
+        return $grid->fetchBindRequest(
+            $this->request,
+			// FIXME Where do we get this list from? Is this something to be supplied by class implementor?
+            array("enabled","number", "bandwidth","bandwidthMetric","burst","description","mask","origin"),
+            "number"
+        );
     }
 }

From fbe7cd2922d1a3bd2ba50d6dd51e9a8deafdeb43 Mon Sep 17 00:00:00 2001
From: pv2b <pvz@pvz.pp.se>
Date: Mon, 8 Aug 2016 21:29:07 +0200
Subject: [PATCH 03/12] Fixed typos

---
 .../OPNsense/Base/ApiMutableTableModelControllerBase.php      | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
index d0a4f54c4..b9c1d9944 100644
--- a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
+++ b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
@@ -119,7 +119,7 @@ abstract class ApiMutableTableModelControllerBase extends ApiControllerBase
             if ($uuid != null) {
                 if (getNodes()->del($uuid)) {
                     // if item is removed, serialize to config and save
-                    $mdlShaper->serializeToConfig();
+                    $mdl->serializeToConfig();
                     Config::getInstance()->save();
                     $result['result'] = 'deleted';
                 } else {
@@ -152,7 +152,7 @@ abstract class ApiMutableTableModelControllerBase extends ApiControllerBase
                     }
                     $result['result'] = $node->enabled;
                     // if item has toggled, serialize to config and save
-                    $mdlShaper->serializeToConfig();
+                    $mdl->serializeToConfig();
                     Config::getInstance()->save();
                 }
             }

From 58bfcf6e4ae4e9d48abd7f47ae23abc03bd854aa Mon Sep 17 00:00:00 2001
From: pv2b <pvz@pvz.pp.se>
Date: Mon, 8 Aug 2016 21:32:45 +0200
Subject: [PATCH 04/12] style fix

---
 .../ApiMutableTableModelControllerBase.php    | 41 +++++++++++--------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
index b9c1d9944..f23be4670 100644
--- a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
+++ b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
@@ -29,7 +29,6 @@
  */
 namespace OPNsense\Base;
 
-
 /**
  * Class ApiMutableTableModelControllerBase, inherit this class to implement
  * an API that exposes a model for tables.
@@ -37,8 +36,8 @@ namespace OPNsense\Base;
  */
 abstract class ApiMutableTableModelControllerBase extends ApiControllerBase
 {
-	// FIXME What about validation?
-	
+    // FIXME What about validation?
+
     static protected $modelPathPrefix = '';
     private function getNodes() {
         $ref = static::$modelPathPrefix
@@ -49,12 +48,13 @@ abstract class ApiMutableTableModelControllerBase extends ApiControllerBase
         return $this->getNodes()->$uuid;
     }
 
-	/**
+    /**
      * retrieve item or return defaults
      * @param $uuid item unique id
      * @return array
      */
-    public function getItemAction($uuid = null) {
+    public function getItemAction($uuid = null)
+    {
         $mdl = $this->getModel();
         if ($uuid != null) {
             $node = $this->getNodeByUUID($uuid);
@@ -69,13 +69,14 @@ abstract class ApiMutableTableModelControllerBase extends ApiControllerBase
         }
         return array();
     }
-	
+
     /**
      * update item with given properties
      * @param $uuid item unique id
      * @return array
      */
-    public function setItemAction($uuid) {
+    public function setItemAction($uuid)
+    {
         if ($this->request->isPost() && $this->request->hasPost(static::$internalModelName)) {
             $mdl = $this->getModel();
             if ($uuid != null) {
@@ -93,26 +94,28 @@ abstract class ApiMutableTableModelControllerBase extends ApiControllerBase
      * add new item and set with attributes from post
      * @return array
      */
-    public function addItemAction() {
+    public function addItemAction()
+    {
         $result = array("result"=>"failed");
         if ($this->request->isPost() && $this->request->hasPost(static::$internalModelName))) {
             $mdl = $this->getModel();
-			// FIXME Is this correct?
-			$node = $this->getNodes()->add();
+            // FIXME Is this correct?
+            $node = $this->getNodes()->add();
             $node->setNodes($this->request->getPost(static::$internalModelName)));
-			// FIXME What do we do with this? Is this traffic-shaper specific? Do we need a hook here?
+            // FIXME What do we do with this? Is this traffic-shaper specific? Do we need a hook here?
             $node->origin = "TrafficShaper"; // set origin to this component.
             return $this->save($mdl, $node, static::$internalModelName));
         }
         return $result;
     }
-	
+
     /**
      * delete item by uuid
      * @param $uuid item unique id
      * @return array status
      */
-    public function delItemAction($uuid) {
+    public function delItemAction($uuid)
+    {
         $result = array("result"=>"failed");
         if ($this->request->isPost()) {
             $mdl = $this->getModel();
@@ -129,14 +132,15 @@ abstract class ApiMutableTableModelControllerBase extends ApiControllerBase
         }
         return $result;
     }
-	
+
     /**
      * toggle item by uuid (enable/disable)
      * @param $uuid item unique id
      * @param $enabled desired state enabled(1)/disabled(1), leave empty for toggle
      * @return array status
      */
-    public function toggleItemAction($uuid, $enabled = null) {
+    public function toggleItemAction($uuid, $enabled = null)
+    {
         $result = array("result" => "failed");
         if ($this->request->isPost()) {
             $mdl = $this->getModel();
@@ -159,18 +163,19 @@ abstract class ApiMutableTableModelControllerBase extends ApiControllerBase
         }
         return $result;
     }
-	
+
     /**
      * search items
      * @return array
      */
-    public function searchItemsAction() {
+    public function searchItemsAction()
+    {
         $this->sessionClose();
         $mdl = $this->getModel();
         $grid = new UIModelGrid($this->getNodes());
         return $grid->fetchBindRequest(
             $this->request,
-			// FIXME Where do we get this list from? Is this something to be supplied by class implementor?
+            // FIXME Where do we get this list from? Is this something to be supplied by class implementor?
             array("enabled","number", "bandwidth","bandwidthMetric","burst","description","mask","origin"),
             "number"
         );

From a45758ff8430695108847185ad1d09956317b526 Mon Sep 17 00:00:00 2001
From: Per von Zweigbergk <pvz@itassistans.se>
Date: Tue, 9 Aug 2016 10:29:27 +0200
Subject: [PATCH 05/12] (base) Properly check for null in getNodeByUUID

---
 .../OPNsense/Base/ApiMutableTableModelControllerBase.php       | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
index f23be4670..e058634b8 100644
--- a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
+++ b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
@@ -45,7 +45,8 @@ abstract class ApiMutableTableModelControllerBase extends ApiControllerBase
         return $this->getModel()->getNodeByReference($ref);
     }
     private function getNodeByUUID($uuid) {
-        return $this->getNodes()->$uuid;
+        $nodes = $this->getNodes();
+        return !empty($nodes) ? $nodes->$uuid : null;
     }
 
     /**

From ea9be4a9ee78caddd80c33a922a4100a11143709 Mon Sep 17 00:00:00 2001
From: Per von Zweigbergk <pvz@itassistans.se>
Date: Tue, 9 Aug 2016 10:32:32 +0200
Subject: [PATCH 06/12] (base) Get rid of spurious origin code

---
 .../OPNsense/Base/ApiMutableTableModelControllerBase.php        | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
index e058634b8..7e50f34f4 100644
--- a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
+++ b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
@@ -103,8 +103,6 @@ abstract class ApiMutableTableModelControllerBase extends ApiControllerBase
             // FIXME Is this correct?
             $node = $this->getNodes()->add();
             $node->setNodes($this->request->getPost(static::$internalModelName)));
-            // FIXME What do we do with this? Is this traffic-shaper specific? Do we need a hook here?
-            $node->origin = "TrafficShaper"; // set origin to this component.
             return $this->save($mdl, $node, static::$internalModelName));
         }
         return $result;

From 051947869f4f319dc5497e8932152b6a1f79425c Mon Sep 17 00:00:00 2001
From: Per von Zweigbergk <pvz@itassistans.se>
Date: Tue, 9 Aug 2016 16:12:03 +0200
Subject: [PATCH 07/12] (base) Fixups of ApiMutableTableModelControllerBase

Changed base class from ApiControllerBase to ApiMutableModelControllerBase
Fixed incorrect string concatenation (+ instead of .) in getNodes()
Removed stray parentheses
Extracted static fields for gridFields and gridDefaultSort
---
 .../ApiMutableTableModelControllerBase.php    | 20 ++++++++-----------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
index 7e50f34f4..60f2e071e 100644
--- a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
+++ b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
@@ -34,14 +34,15 @@ namespace OPNsense\Base;
  * an API that exposes a model for tables.
  * @package OPNsense\Base
  */
-abstract class ApiMutableTableModelControllerBase extends ApiControllerBase
+abstract class ApiMutableTableModelControllerBase extends ApiMutableModelControllerBase
 {
     // FIXME What about validation?
 
     static protected $modelPathPrefix = '';
+    static protected $gridFields = array();
+    static protected $gridDefaultSort = null;
     private function getNodes() {
-        $ref = static::$modelPathPrefix
-             + static::$internalModelName;
+        $ref = static::$modelPathPrefix . static::$internalModelName;
         return $this->getModel()->getNodeByReference($ref);
     }
     private function getNodeByUUID($uuid) {
@@ -98,12 +99,12 @@ abstract class ApiMutableTableModelControllerBase extends ApiControllerBase
     public function addItemAction()
     {
         $result = array("result"=>"failed");
-        if ($this->request->isPost() && $this->request->hasPost(static::$internalModelName))) {
+        if ($this->request->isPost() && $this->request->hasPost(static::$internalModelName)) {
             $mdl = $this->getModel();
             // FIXME Is this correct?
             $node = $this->getNodes()->add();
-            $node->setNodes($this->request->getPost(static::$internalModelName)));
-            return $this->save($mdl, $node, static::$internalModelName));
+            $node->setNodes($this->request->getPost(static::$internalModelName));
+            return $this->save($mdl, $node, static::$internalModelName);
         }
         return $result;
     }
@@ -172,11 +173,6 @@ abstract class ApiMutableTableModelControllerBase extends ApiControllerBase
         $this->sessionClose();
         $mdl = $this->getModel();
         $grid = new UIModelGrid($this->getNodes());
-        return $grid->fetchBindRequest(
-            $this->request,
-            // FIXME Where do we get this list from? Is this something to be supplied by class implementor?
-            array("enabled","number", "bandwidth","bandwidthMetric","burst","description","mask","origin"),
-            "number"
-        );
+        return $grid->fetchBindRequest($this->request, static::$gridFields, static::$gridDefaultSort);
     }
 }

From d48d41910677d60079861ca856c1df7913b5a3ae Mon Sep 17 00:00:00 2001
From: Per von Zweigbergk <pvz@itassistans.se>
Date: Tue, 9 Aug 2016 19:27:15 +0200
Subject: [PATCH 08/12] (Base) Refactor to split out saving/validation logic

---
 .../Base/ApiMutableModelControllerBase.php    | 49 +++++++++++++------
 1 file changed, 33 insertions(+), 16 deletions(-)

diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableModelControllerBase.php b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableModelControllerBase.php
index 828d20d1b..7289a612a 100644
--- a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableModelControllerBase.php
+++ b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableModelControllerBase.php
@@ -107,6 +107,38 @@ abstract class ApiMutableModelControllerBase extends ApiControllerBase
         return $this->modelHandle;
     }
 
+    /**
+     * validate and save model after update or insertion.
+     * Use the reference node and tag to rename validation output for a specific node to a new offset, which makes
+     * it easier to reference specific uuids without having to use them in the frontend descriptions.
+     * @param $node reference node, to use as relative offset
+     * @return array result / validation output
+     */
+    protected function save($node = null)
+    {
+        $mdl = $this->getModel();
+        $result = array("result"=>"failed","validations" => array());
+        // perform validation
+        $valMsgs = $mdl->performValidation();
+        foreach ($valMsgs as $field => $msg) {
+            // replace absolute path to attribute for relative one at uuid.
+            if ($node != null) {
+                $fieldnm = str_replace($node->__reference, static:$internalModelName, $msg->getField());
+                $result["validations"][$fieldnm] = $msg->getMessage();
+            } else {
+                $result["validations"][$msg->getField()] = $msg->getMessage();
+            }
+        }
+        // serialize model to config and save when there are no validation errors
+        if (count($result['validations']) == 0) {
+            // save config if validated correctly
+            $mdl->serializeToConfig();
+            Config::getInstance()->save();
+            $result = array("result" => "saved");
+        }
+        return $result;
+    }
+
     /**
      * update model settings
      * @return array status / validation errors
@@ -118,22 +150,7 @@ abstract class ApiMutableModelControllerBase extends ApiControllerBase
             // load model and update with provided data
             $mdl = $this->getModel();
             $mdl->setNodes($this->request->getPost(static::$internalModelName));
-
-            // perform validation
-            $valMsgs = $mdl->performValidation();
-            foreach ($valMsgs as $field => $msg) {
-                if (!array_key_exists("validations", $result)) {
-                    $result["validations"] = array();
-                }
-                $result["validations"][static::$internalModelName.".".$msg->getField()] = $msg->getMessage();
-            }
-
-            // serialize model to config and save
-            if ($valMsgs->count() == 0) {
-                $mdl->serializeToConfig();
-                Config::getInstance()->save();
-                $result["result"] = "saved";
-            }
+            $result = $this->save();
         }
         return $result;
     }

From 95c39134d95b4cfb8518bb5b543c3a08ffed6680 Mon Sep 17 00:00:00 2001
From: Per von Zweigbergk <pvz@itassistans.se>
Date: Tue, 9 Aug 2016 19:27:37 +0200
Subject: [PATCH 09/12] (Base) Properly update argument list for save after
 implementing

---
 .../OPNsense/Base/ApiMutableTableModelControllerBase.php     | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
index 60f2e071e..86d5bcc3e 100644
--- a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
+++ b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
@@ -85,7 +85,7 @@ abstract class ApiMutableTableModelControllerBase extends ApiMutableModelControl
                 $node = $this->getNodeByUUID($uuid);
                 if ($node != null) {
                     $node->setNodes($this->request->getPost(static::$internalModelName));
-                    return $this->save($mdl, $node, static::$internalModelName);
+                    return $this->save($node);
                 }
             }
         }
@@ -101,10 +101,9 @@ abstract class ApiMutableTableModelControllerBase extends ApiMutableModelControl
         $result = array("result"=>"failed");
         if ($this->request->isPost() && $this->request->hasPost(static::$internalModelName)) {
             $mdl = $this->getModel();
-            // FIXME Is this correct?
             $node = $this->getNodes()->add();
             $node->setNodes($this->request->getPost(static::$internalModelName));
-            return $this->save($mdl, $node, static::$internalModelName);
+            return $this->save($node);
         }
         return $result;
     }

From 7600727f6bd6a84deb234553c29ede96c0eb7801 Mon Sep 17 00:00:00 2001
From: Per von Zweigbergk <pvz@itassistans.se>
Date: Tue, 9 Aug 2016 19:35:44 +0200
Subject: [PATCH 10/12] (base) Fixed a mistyped Paamayim Nekudotayim

---
 .../controllers/OPNsense/Base/ApiMutableModelControllerBase.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableModelControllerBase.php b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableModelControllerBase.php
index 7289a612a..541bf4a5f 100644
--- a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableModelControllerBase.php
+++ b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableModelControllerBase.php
@@ -123,7 +123,7 @@ abstract class ApiMutableModelControllerBase extends ApiControllerBase
         foreach ($valMsgs as $field => $msg) {
             // replace absolute path to attribute for relative one at uuid.
             if ($node != null) {
-                $fieldnm = str_replace($node->__reference, static:$internalModelName, $msg->getField());
+                $fieldnm = str_replace($node->__reference, static::$internalModelName, $msg->getField());
                 $result["validations"][$fieldnm] = $msg->getMessage();
             } else {
                 $result["validations"][$msg->getField()] = $msg->getMessage();

From 44eb7531f9df9b00221bfb94ac998d5f33699c74 Mon Sep 17 00:00:00 2001
From: Per von Zweigbergk <pvz@itassistans.se>
Date: Tue, 9 Aug 2016 19:43:24 +0200
Subject: [PATCH 11/12] (base) Added missing $this->

---
 .../OPNsense/Base/ApiMutableTableModelControllerBase.php        | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
index 86d5bcc3e..9c01333d8 100644
--- a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
+++ b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
@@ -66,7 +66,7 @@ abstract class ApiMutableTableModelControllerBase extends ApiMutableModelControl
             }
         } else {
             // generate new node, but don't save to disc
-            $node = getNodes()->add();
+            $node = $this->getNodes()->add();
             return array(static::$internalModelName => $node->getNodes());
         }
         return array();

From 053f7f89cdea57095e938d29355a79bb6b3b3578 Mon Sep 17 00:00:00 2001
From: Per von Zweigbergk <pvz@itassistans.se>
Date: Wed, 10 Aug 2016 14:39:32 +0200
Subject: [PATCH 12/12] (base) Added hooks that can be used by controllers to
 take actions at the appropriate time during an action

---
 .../Base/ApiMutableModelControllerBase.php    | 36 ++++++---
 .../ApiMutableTableModelControllerBase.php    | 75 +++++++++++++++----
 2 files changed, 87 insertions(+), 24 deletions(-)

diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableModelControllerBase.php b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableModelControllerBase.php
index 541bf4a5f..2f1053c1a 100644
--- a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableModelControllerBase.php
+++ b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableModelControllerBase.php
@@ -111,12 +111,13 @@ abstract class ApiMutableModelControllerBase extends ApiControllerBase
      * validate and save model after update or insertion.
      * Use the reference node and tag to rename validation output for a specific node to a new offset, which makes
      * it easier to reference specific uuids without having to use them in the frontend descriptions.
+     * @param $mdl The model to save
      * @param $node reference node, to use as relative offset
+     * @param $actionHook A closure containing an action to do after validation but before saving. Must return null if successful, or an error message if not.
      * @return array result / validation output
      */
-    protected function save($node = null)
+    protected function save($mdl, $node = null, $actionHook = null)
     {
-        $mdl = $this->getModel();
         $result = array("result"=>"failed","validations" => array());
         // perform validation
         $valMsgs = $mdl->performValidation();
@@ -129,16 +130,31 @@ abstract class ApiMutableModelControllerBase extends ApiControllerBase
                 $result["validations"][$msg->getField()] = $msg->getMessage();
             }
         }
-        // serialize model to config and save when there are no validation errors
-        if (count($result['validations']) == 0) {
-            // save config if validated correctly
-            $mdl->serializeToConfig();
-            Config::getInstance()->save();
-            $result = array("result" => "saved");
+        if (count($result['validations']) != 0) {
+            return $result;
         }
-        return $result;
+        // run the action hook (default one just always returns null)
+        $errorMessage = $actionHook ? $actionHook($mdl) : null;
+        if ($errorMessage != null) {
+            return array("result"=>failed, "error"=>$errorMessage);
+        }
+        // serialize model to config and save when there are no validation errors
+        // and the action hook completed successfully
+        $mdl->serializeToConfig();
+        Config::getInstance()->save();
+        return array("result"=>"saved");
     }
 
+    /**
+     * hook to be overridden if the controller is to take an action when
+     * setAction is called. This hook is called after a model has been
+     * constructed and validated but before it serialized to the configuration
+     * and written to disk
+     * @param $mdl The validated model containing the new state of the model
+     * @return Error message on error, or null/void on success
+     */
+    protected function setActionHook($mdl) { }
+
     /**
      * update model settings
      * @return array status / validation errors
@@ -150,7 +166,7 @@ abstract class ApiMutableModelControllerBase extends ApiControllerBase
             // load model and update with provided data
             $mdl = $this->getModel();
             $mdl->setNodes($this->request->getPost(static::$internalModelName));
-            $result = $this->save();
+            $result = $this->save($mdl, null, $this->setActionHook);
         }
         return $result;
     }
diff --git a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
index 9c01333d8..420b0a7c6 100644
--- a/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
+++ b/src/opnsense/mvc/app/controllers/OPNsense/Base/ApiMutableTableModelControllerBase.php
@@ -36,8 +36,6 @@ namespace OPNsense\Base;
  */
 abstract class ApiMutableTableModelControllerBase extends ApiMutableModelControllerBase
 {
-    // FIXME What about validation?
-
     static protected $modelPathPrefix = '';
     static protected $gridFields = array();
     static protected $gridDefaultSort = null;
@@ -72,6 +70,16 @@ abstract class ApiMutableTableModelControllerBase extends ApiMutableModelControl
         return array();
     }
 
+    /**
+     * hook to be overridden if the controller is to take an action when
+     * setItemAction is called. This hook is called after a model has been
+     * constructed and validated but before it serialized to the configuration
+     * and written to disk
+     * @param $mdl The validated model containing the new state of the model
+     * @return Error message on error, or null/void on success
+     */
+    protected function setItemActionHook($mdl) { }
+
     /**
      * update item with given properties
      * @param $uuid item unique id
@@ -85,13 +93,23 @@ abstract class ApiMutableTableModelControllerBase extends ApiMutableModelControl
                 $node = $this->getNodeByUUID($uuid);
                 if ($node != null) {
                     $node->setNodes($this->request->getPost(static::$internalModelName));
-                    return $this->save($node);
+                    return $this->save($mdl, $node, $this->setItemActionHook);
                 }
             }
         }
         return array("result"=>"failed");
     }
 
+    /**
+     * hook to be overridden if the controller is to take an action when
+     * addItemAction is called. This hook is called after a model has been
+     * constructed and validated but before it serialized to the configuration
+     * and written to disk
+     * @param $mdl The validated model containing the state of the new model
+     * @return Error message on error, or null/void on success
+     */
+    protected function addItemActionHook($mdl) { }
+
     /**
      * add new item and set with attributes from post
      * @return array
@@ -103,11 +121,21 @@ abstract class ApiMutableTableModelControllerBase extends ApiMutableModelControl
             $mdl = $this->getModel();
             $node = $this->getNodes()->add();
             $node->setNodes($this->request->getPost(static::$internalModelName));
-            return $this->save($node);
+            return $this->save($mdl, $node, $this->addItemActionHook);
         }
         return $result;
     }
 
+    /**
+     * hook to be overridden if the controller is to take an action when
+     * delItemAction is called. This hook is called after a model has been
+     * constructed and validated but before it serialized to the configuration
+     * and written to disk
+     * @param $uuid The UUID of the item to be deleted
+     * @return Error message on error, or null/void on succes s
+     */
+    protected function delItemActionHook($uuid) { }
+
     /**
      * delete item by uuid
      * @param $uuid item unique id
@@ -119,7 +147,10 @@ abstract class ApiMutableTableModelControllerBase extends ApiMutableModelControl
         if ($this->request->isPost()) {
             $mdl = $this->getModel();
             if ($uuid != null) {
-                if (getNodes()->del($uuid)) {
+                $errorMessage = delItemActionHook($uuid);
+                if ($errorMessage) {
+                    $result['error'] = $errorMessage;
+                } else if (getNodes()->del($uuid)) {
                     // if item is removed, serialize to config and save
                     $mdl->serializeToConfig();
                     Config::getInstance()->save();
@@ -132,6 +163,17 @@ abstract class ApiMutableTableModelControllerBase extends ApiMutableModelControl
         return $result;
     }
 
+    /**
+     * hook to be overridden if the controller is to take an action when
+     * toggleItemAction is called. This hook is called after a model has been
+     * constructed and validated but before it serialized to the configuration
+     * and written to disk
+     * @param $uuid The UUID of the item to be toggled
+     * @param $enabled desired state enabled(1)/disabled(1), leave empty for toggle
+     * @return Error message on error, or null/void on succes s
+     */
+    protected function toggleItemActionHook($uuid, $enabled) { }
+
     /**
      * toggle item by uuid (enable/disable)
      * @param $uuid item unique id
@@ -146,17 +188,22 @@ abstract class ApiMutableTableModelControllerBase extends ApiMutableModelControl
             if ($uuid != null) {
                 $node = $mdl->getNodeByUUID($uuid);
                 if ($node != null) {
-                    if ($enabled == "0" || $enabled == "1") {
-                        $node->enabled = (string)$enabled;
-                    } elseif ($node->enabled->__toString() == "1") {
-                        $node->enabled = "0";
+                    $errorMessage = toggleItemActionHook($uuid, $enabled);
+                    if ($errorMessage) {
+                        $result['error'] = $errorMessage;
                     } else {
-                        $node->enabled = "1";
+                        if ($enabled == "0" || $enabled == "1") {
+                            $node->enabled = (string)$enabled;
+                        } elseif ($node->enabled->__toString() == "1") {
+                            $node->enabled = "0";
+                        } else {
+                            $node->enabled = "1";
+                        }
+                        $result['result'] = $node->enabled;
+                        // if item has toggled, serialize to config and save
+                        $mdl->serializeToConfig();
+                        Config::getInstance()->save();
                     }
-                    $result['result'] = $node->enabled;
-                    // if item has toggled, serialize to config and save
-                    $mdl->serializeToConfig();
-                    Config::getInstance()->save();
                 }
             }
         }