Problem/Motivation

A media item always references a source, and we frequently want to access field values from that source in MediaSource plugins. Instead of repeating similar code in many places, it will be easier to write, easier to read, and more reliable to provide a method for this common task.

The code for this was already introduced in #2831944: Implement media source plugin for remote video via oEmbed. We are splitting off this issue in the hope of making the new method available a little sooner.

Proposed resolution

Add the method getSourceFieldValue() to MediaSourceInterface and implement it in MediaSourceBase.

Completed tasks

  1. Add the method to the interface.
  2. Add the method to the base class.
  3. Add a test.

Remaining tasks

  1. Write a change record.

User interface changes

None

API changes

Add a new method to MediaSourceInterface.

Most classes that extend MediaSourceBase will not have to make any changes.

  1. Any class that implements the interface without extending the base class will have to implement this method.
  2. If the source value is not the main property of the source field, as returned by mainPropertyName(), then the MediaSource plugin will need to override this method.

Without this method, we use code like this in MediaSource plugins:

    $source_field = $this->getSourceFieldDefinition($media->bundle->entity)->getName();
    /** @var \Drupal\Core\Field\FieldItemInterface $source_field_item */
    $source_field_item = $media->get($source_field)->first();

    $main_property = $source_field_item->mainPropertyName();
    $resource_url = $source_field_item->get($main_property)->getString();

After adding this method, that can be simplified:

    $resource_url = $this->getSourceFieldValue($media);

Data model changes

None

Comments

benjifisher created an issue. See original summary.

benjifisher’s picture

Issue summary: View changes
Status: Active » Needs work
StatusFileSize
new1.83 KB

The attached patch is based on the code from #2831944: Implement media source plugin for remote video via oEmbed, with minor changes to the comments.

I will start working on a test.

benjifisher’s picture

Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new1.31 KB
new2.78 KB

The attached patch fixes the method: return $field_item->{$field_item->mainPropertyName()}; instead of return $field_item->get($field_item->mainPropertyName());.

It also adds a test, so I think this patch is ready for review.

phenaproxima’s picture

Status: Needs review » Needs work
Issue tags: +Media Initiative

Thanks, @benjifisher! This looks fantastic.

+++ b/core/modules/media/src/MediaSourceInterface.php
@@ -177,4 +177,18 @@ public function prepareViewDisplay(MediaTypeInterface $type, EntityViewDisplayIn
+   * @return \Drupal\Core\TypedData\TypedDataInterface
+   *   The source value.

This will return mixed, not TypedDataInterface.

benjifisher’s picture

Status: Needs work » Needs review
StatusFileSize
new668 bytes
new2.75 KB

Thanks for checking! The attached patch changes the @return to "mixed" and also tweaks the summary line of the docblock.

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +DX (Developer Experience)

Looks great! RTBC once Drupal CI passes it. This is also a big win for developers who are using Media, so I'm tagging this as a DX improvement.

benjifisher’s picture

Assigned: benjifisher » Unassigned
larowlan’s picture

+++ b/core/modules/media/src/MediaSourceBase.php
@@ -319,6 +319,22 @@ protected function getSourceFieldName() {
+    // The source value is stored as the main property of the source field in
+    // 99% of the cases, so return that. The other 1% can override this function
+    // and tweak the logic.

I don't think we should speculate on the % or elude to basic language features like inheritance.

So tl;dr - not sure this comment is needed

starshaped’s picture

StatusFileSize
new2.63 KB
new716 bytes

Removed the comment as noted in #8.

  • larowlan committed bfe514a on 8.5.x
    Issue #2934966 by benjifisher, starshaped: Make it easier to get source...
larowlan’s picture

Status: Reviewed & tested by the community » Fixed

Committed as bfe514a and pushed to 8.5.x.

Do you want to add a tag here for tracking progress in this weekend's sprint?

Publishing change record.

Status: Fixed » Closed (fixed)

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

james hawthorn-byng’s picture

Just in case someone else comes here is is as equally as confused as I was as to how we can use this on a Media object...

If you have a media object and you want to get the source field value you need to do $mediaSource = $media->getSource() then $mediaSourceValue = $mediaSource->getSourceFieldValue()

Every time I tried to see what was coming back in just $media->getSource() my site crashed or I wasn't allowed to kint out the protected properties so I just wanted to put a little explanation here.

benjifisher’s picture

@James Byng:

That sort of usage information is in the change record (CR). If you check the right sidebar for this issue, you will find a link to the CR getSourceFieldValue() method added to MediaSourceInterface, which says pretty much the same thing as your comment.