Searchpage class variables should not be accessed directly. Functions should be use to access the variable. For instance use getDescription() and setDescription($description) for the protected class variable description. For a boolean variable the getter function becomes isVariableName(). In object-oriented programming this is called encapsulation.

Remaining tasks

  • Update the class variables and make them protected.
  • Create getters and setters for frequently used get and set functionality.
  • Update drupal to use the getters and setters instead of accessing variables directly.
  • There are no tests required because the added functions are only getters and setters.

For more info over what should be done see the issue summary of #2016679: Expand Entity Type interfaces to provide methods, protect the properties.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because properties should not be public, API methods should not be allowed to be sidestepped.
Issue priority Major because this meta goes across the entire system. But each child will be a normal bug.
Prioritized changes Prioritized since it is a bug and it reduces fragility.
Disruption Somewhat disruptive for core as well as contributed and custom modules:
  • BC break for anything using the public properties: code will need to convert to the methods
  • BC break for anything (mis)using properties that should not really be public: will require minor refactoring
  • BC break for alternate implementations of a given entity interface (rare/probably nonexistent): they will need to implement the new methods

But impact will be greater than the disruption, so it is allowed in the beta.

CommentFileSizeAuthor
#5 2384537-5.patch588 bytesrpayanm
#3 protect_searchpage-2384537-3.patch1.23 KBareke

Comments

daffie’s picture

daffie commented

I would like to get this fixed. So I will do a good review for posted patches.

daffie’s picture

daffie commented
Issue summary: View changes

The class variables $id and $label need to become protected.
You can use the functions id() as a getter function for the $id variable and label() as a getter function for the variable $label.

areke’s picture

areke commented
Status: Active » Needs review
StatusFileSize
new protect_searchpage-2384537-3.patch1.23 KB

Protect variables for searchpage.

daffie’s picture

daffie commented
Status: Needs review » Needs work 
+++ b/core/modules/search/src/Entity/SearchPage.php
@@ -136,6 +136,36 @@ public function getPluginCollections() {
+  public function label() {
+    return $this->get('label');
+  }
...
+  public function id() {
+    return $this->get('id');
+  }

These functions are inherited from the Entity class. So they can be removed.

+++ b/core/modules/search/src/Entity/SearchPage.php
@@ -136,6 +136,36 @@ public function getPluginCollections() {
+  public function setLabel($label) {
+    $this->set('label', $label);
+    return $this;
+  }
...
+  public function setId($id) {
+    $this->set('id', $id);
+    return $this;
   }

These functions are not used. You can remove them and use set('id', $value) and set('label', $value).

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
commented
Status: Needs work » Needs review
StatusFileSize
new 2384537-5.patch588 bytes
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Title: Make the class variables protected for Searchpage » Make the class variables protected for SearchPage entity
Status: Needs review » Reviewed & tested by the community

Well, that apparently passed the tests. It seems simple enough, and since the core ConfigEntityBase class already has methods for these, it seems like that's all we need to do. I confirmed there are not other public class variables on the class.

Issue summary looks good and includes beta eval.

Do we need a change notice? The other issues for config entities on the parent Meta do not seem to have them, so I guess not.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed a9a3b15 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation for to the issue summary.

  • alexpott committed a9a3b15 on 8.0.x 
    Issue #2384537 by rpayanm, areke: Make the class variables protected for...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.