brick / schema

Schema.org library for PHP
MIT License
49 stars 6 forks source link

ThingConvert ignores the @Context #5

Open juanparati opened 2 years ago

juanparati commented 2 years ago

It seems that ThingConvert ignores @context, so the library will fails to read schemas like the following ones:

{
   "@context":"https://schema.org",
   "@graph":[
      {
         "@type":"Organization",
         "@id":"https://example.org/#organization",
         "name":"Acme LTD",
         "url":"https://example.org/",
         "sameAs":[

         ]
      }
   ]
}

The reason is that ThingConverts expects a @type that starts with https://schema.org.

jdrucey commented 2 years ago

Just started using this as it seems perfect for what we need, so thanks for the great work.

Flat schemas worked perfectly until I tried running on a site that uses @graph to include multiple schemas, where I get no results back.

Any input on this one? @graph seems to be commonly used by Yoast, a popular WordPress plugin, so these types of implementation seem ripe across the internet. This structure also validates fine in validator.schema.org so handling it in this library seems sensible.

jdrucey commented 2 years ago

This is seemingly the same issue as @juanparati issue, in that the context is set at the top, so inside the @graph in my example, types are not prefixed with https://schema.org.

What was your solution in the end @juanparati?

goellner commented 1 year ago

Also running into this issue, would be great if anyone could share a solution to this

goellner commented 1 year ago

brick/schema can extract the data, if the context is set on a graph item:

{
   "@context":"https://schema.org",
   "@graph":[
      {
         "@context": "http://schema.org/"
         "@type":"Organization",
         "@id":"https://example.org/#organization",
         "name":"Acme LTD",
         "url":"https://example.org/",
         "sameAs":[

         ]
      }
   ]
}

older versions of the wp plugin yoast don't include this context, newer versions do. since we can't update external websites yoast version, it would be awesome, if brick could add support for this.

@BenMorel would you be able to help if we sponsor the project with a one time sponsorship?

jhard commented 1 year ago

Seems like the easiest way is to add the @context in the @graph array so you end up with what @goellner mentioned.

Quickfix for me was to change brick/structured-data/src/Reader/JsonLdReader.php in the function readJson and change $data = $data->{'@graph'}; to

                $context = $data->{'@context'} ?? null;
                $data = $data->{'@graph'};
                if($context) {
                   foreach($data as $i => $item) {
                      if($context && !($item->{'@context'} ?? null)) {
                         $data[$i]->{'@context'} = $context;
                      }
                   }
                }

It will preserve existing @context attributes on the items and otherwise add the parent one if one exists. I'm a total noob when it comes to schema.org so I don't know if they could potentially have nested @graph that would require additional treatment, but that shouldn't be too hard to add.

@BenMorel If you'd accept a pull request for brick/structured-data I could create on over there.

goaround commented 10 months ago

I run into the same issue with the Rank Math WordPress Plugin. There is also the @schema on the @graph items missing @jhard could you create a PR?

jhard commented 10 months ago

I run into the same issue with the Rank Math WordPress Plugin. There is also the @Schema on the @graph items missing @jhard could you create a PR?

Sorry, I've long moved on from this and it seems so has everyone else unfortunately. Feel free to submit the PR with the code yourself though :)