MDL-57273 persistent: protected custom getters

(and setters).
moodle · Jan 20, 2017 · 599acbe · 599acbe
1 parent 9c91a95
commit 599acbe
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 7 deletions.
diff --git a/admin/tool/lp/classes/form/competency_framework.php b/admin/tool/lp/classes/form/competency_framework.php
@@ -149,7 +149,7 @@ protected function extra_validation($data, $files, array &$errors) {
      */
     protected function get_default_data() {
         $data = parent::get_default_data();
-        $data->taxonomies = $this->get_persistent()->get_taxonomies();
+        $data->taxonomies = $this->get_persistent()->get('taxonomies');
         return $data;
     }
 

diff --git a/admin/tool/lpimportcsv/classes/framework_exporter.php b/admin/tool/lpimportcsv/classes/framework_exporter.php
@@ -83,7 +83,7 @@ public function export() {
             '',
             '',
             true,
-            implode(',', $this->framework->get_taxonomies())
+            implode(',', $this->framework->get('taxonomies'))
         );
         $writer->add_data($row);
 

diff --git a/competency/classes/competency_framework.php b/competency/classes/competency_framework.php
@@ -178,7 +178,7 @@ public function get_taxonomy($level) {
      *
      * @return array Contains the list of taxonomy constants indexed by level.
      */
-    public function get_taxonomies() {
+    protected function get_taxonomies() {
         $taxonomies = explode(',', $this->raw_get('taxonomies'));
 
         // Indexing first level at 1.
@@ -212,7 +212,7 @@ public function has_user_competencies() {
      *
      * @param string|array $taxonomies A string, or an array where the values are the term constants.
      */
-    public function set_taxonomies($taxonomies) {
+    protected function set_taxonomies($taxonomies) {
         if (is_array($taxonomies)) {
             $taxonomies = implode(',', $taxonomies);
         }

diff --git a/competency/classes/evidence.php b/competency/classes/evidence.php
@@ -124,7 +124,7 @@ public function get_context() {
      *
      * @return mixed
      */
-    public function get_desca() {
+    protected function get_desca() {
         $value = $this->raw_get('desca');
         if ($value !== null) {
             $value = json_decode($value);
@@ -147,7 +147,7 @@ public function get_description() {
      * @param mixed $value
      * @return mixed
      */
-    public function set_desca($value) {
+    protected function set_desca($value) {
         if ($value !== null) {
             if (!is_scalar($value) && !is_array($value) && !($value instanceof stdClass)) {
                 throw new coding_exception('$a format not supported.');
@@ -162,7 +162,7 @@ public function set_desca($value) {
      *
      * @param null|string|moodle_url $url The URL.
      */
-    public function set_url($url) {
+    protected function set_url($url) {
         if ($url instanceof \moodle_url) {
             $url = $url->out(false);
         }

diff --git a/lib/classes/persistent.php b/lib/classes/persistent.php
@@ -58,13 +58,45 @@ abstract class persistent {
      * @param stdClass $record If set will be passed to {@link self::from_record()}.
      */
     public function __construct($id = 0, stdClass $record = null) {
+        global $CFG;
+
         if ($id > 0) {
             $this->raw_set('id', $id);
             $this->read();
         }
         if (!empty($record)) {
             $this->from_record($record);
         }
+        if ($CFG->debugdeveloper) {
+            $this->verify_protected_methods();
+        }
+    }
+
+    /**
+     * This function is used to verify that custom getters and setters are declared as protected.
+     *
+     * Persistent properties should always be accessed via get('property') and set('property', 'value') which
+     * will call the custom getter or setter if it exists. We do not want to allow inconsistent access to the properties.
+     */
+    final protected function verify_protected_methods() {
+        $properties = static::properties_definition();
+
+        foreach ($properties as $property => $definition) {
+            $method = 'get_' . $property;
+            if (method_exists($this, $method)) {
+                $reflection = new ReflectionMethod($this, $method);
+                if (!$reflection->isProtected()) {
+                    throw new coding_exception('The method ' . get_class($this) . '::'. $method . ' should be protected.');
+                }
+            }
+            $method = 'set_' . $property;
+            if (method_exists($this, $method)) {
+                $reflection = new ReflectionMethod($this, $method);
+                if (!$reflection->isProtected()) {
+                    throw new coding_exception('The method ' . get_class($this) . '::'. $method . ' should be protected.');
+                }
+            }
+        }
     }
 
     /**