Add undeclared variables to the Feed entity class

The aggregator module has been removed from Core in 10.0.x-dev and now lives on as a contrib module. Issues in the Core queue about the aggregator module, like this one, have been moved to the contrib module queue.

Closed #2198821: Split items and source_string out of feed object as a duplicate

+++ b/core/modules/aggregator/src/Entity/Feed.php
@@ -55,6 +55,14 @@
+  /**
+   * The last fetched content of the feed.
+   *
+   * @var string
+   *   The content of feed response.
+   */
+  public $source_string = '';

should we make this protected and then add a __get and trigger a deprecation warning?

As per #6

The aggregator module has been removed from Core in 10.0.x-dev and now lives on as a contrib module.

- this patch needs reroll.

Addressed #13
Rerolled patch against #4 for 2.x-dev in in 10.0.x-dev.

Kindly review.

Needs work for #12

Tests are fixed in HEAD for the 2.x branch

RE: #12

This also applies to the undeclared $feed->items property.

I'm sensing some code smell from these undeclared variables. It's more than just a bad practice in general. They obfuscate dependencies of the parser and processor plugins. Why doesn't fetch() return the body of the response instead of assigning it to $feed->source_string? Why doesn't parse() return the items instead of assigning it to $feed->items? I do not like using an entity class like this.

I want to remove the use of these variables altogether. But doing that would break backward compatibility by changing the plugin APIs. I don't know that I want to bother with releasing version 3 right now. Also, I feel like we should get a version that doesn't trigger deprecation notices in PHP 8.2 out sooner rather than later.

Suggested plan:

• Add the three variables as protected to the Feed class.
• Add the __get function to return the variables. Have the function trigger a deprecation notice stating that they will be removed in version 3.0.0.
• Open issues for removing the three variables and changing the plugin APIs to not use them. I suggest keeping the issues for each variable separate.

Please provide feedback.

Here's a patch for the plan I outlined in #16. Though I decided to not do anything with $http_headers and just let it be deleted as in the original patch. This may not be the correct thing to do. Feel free to comment.

An empty($feed->items); call in the ItemsImporter caused the failures above. empty() calls the __isset() magic method, which hadn't been implemented. I could have rewritten that line to check for the existence of items in the array a different way, but I figured a more complete solution is to implement __isset().

I also removed a couple of other dynamic variable assignments in AggregatorTestBase. One was for $feed->items, but used it in a completely different way than the importers. The other was for yet another new dynamic variable. Since it's only in a test I've chosen to not add it to the Feed class.

My change to remove those additional dynamic variables in the test base broke two of the tests. They were actually using that badly-assigned items variable. So I had to write it out of those other tests.

+++ b/src/Entity/Feed.php
@@ -55,6 +55,67 @@ use Drupal\aggregator\FeedInterface;
+    if ($name == 'items') {
+      @trigger_error('The $items property is deprecated in 2.1.0 and will be removed from 3.0.0. See https://www.drupal.org/project/aggregator/issues/3239690.', E_USER_DEPRECATED);
+      $this->items = $value;
+    }
+    elseif ($name == 'source_string') {
+      @trigger_error('The $source_string property is deprecated in 2.1.0 and will be removed from 3.0.0. See https://www.drupal.org/project/aggregator/issues/3239690.', E_USER_DEPRECATED);
+      $this->source_string = $value;
+    }
+    else {
+      parent::__set($name, $value);
+    }

Ubernit, but personally I see elseif (and else) as a code smell.

We can return early here and just use if.

I think we're supposed to link to a change record rather than an issue node - we can create one here: https://www.drupal.org/node/add/changenotice?field_project=3262413&field...

Not sure this is worth fixing, feel free to ignore on both counts

Here's the change record: https://www.drupal.org/node/3386012.

I also edited the __set() function as suggested.

Thank you to everyone who worked on this issue and the one that was closed as a duplicate!