#4179: Dataset ComplexDataFacade returning wrong empty value

[stefan-korn]

fixes #4179

[dafeder]

@stefan-korn I'm not sure about this -- can you give an example case where $dist->{$matches[2]} would be unset?

[stefan-korn]

@dafeder : Have you read the description in #4179 ? I cannot reproduce this on plain DKANv2, only with our customized schema. But it should not return an empty array as value there, this is wrong in my opinion.

[dafeder]

@stefan-korn I did read it, I'd like to observe the behavior before and after the fix to properly review. Can you post an example schema that would let me reproduce the issue?

[stefan-korn]

@dafeder : I am attaching an example of dataset.json which can produce the issue. There is new property "dctconformsto" in the distribution (as an array of objects).
The steps to reproduce the issue are:

• add a dataset with a distribution that has this property filled (once or more).
• add the property to the search index DKAN.
• Reindex the search index
• indexing will fail with Error: Object of class stdClass could not be converted to string in Drupal\search_api\Plugin\search_api\data_type\StringDataType->getValue() (line 23 of /var/www/plain-dkan/docroot/modules/contrib/search_api/src/Plugin/search_api/data_type/StringDataType.php).

dataset.json

[dafeder]

Hmmm @stefan-korn I have been trying to test... with your branch I get the same error on indexing, but now I also get a new error at the top of every form on the site:

Warning: Attempt to read property "value" on null in data_dictionary_widget_form_alter() (line 81 of /var/www/html/dkan/modules/data_dictionary_widget/data_dictionary_widget.module).

data_dictionary_widget_form_alter(Array, Object, 'search_api_index_fields') (Line: 545)

Here is the dataset I tried indexing:

{
    "title": "Test dataset",
    "description": "This is a test dataset",
    "accessLevel": "public",
    "modified": "2019-01-01",
    "keyword": ["test", "dataset"],
    "distribution": [
        {
            "title": "Test distribution",
            "format": "text/csv",
            "downloadURL": "https://example.com/test.csv",
            "dctconformsto": [
                {
                    "dct_standard": "https://www.w3.org/TR/vocab-dcat-2/"
                }
            ]
        }
    ]
}

[stefan-korn]

@dafeder . i suppose this error was because the branch was out of sync with current head. I have synced with latest changes from head now.

fix for #4178 was missing.

[dafeder]

CodeClimate no longer shows what the issue was, and I rebased and pushed it as PR #4236 and it passed, so going to ignore that :).

Re-testing.

[dafeder]

@stefan-korn OK I have tested this and I get the same error message on index, "Object of class stdClass could not be converted to string", whether I use your branch or not. But I don't think this has anything to do with empty values?

This error seems to be more about how well it can handle nested objects within the dataset schema. Which is maybe just a different issue, this field handling should be more recursive so that it's possible to index fields more than 2 levels deep in the JSON tree.

[dafeder]

@stefan-korn the code in your PR does just look more inherently correct and is so small I am ok with merging it. I would have liked to be able to reproduce an actual problem here and see your PR resolving it, so if I am testing wrong let me know. I don't want to drag this out too much especially if this resolves an issue you're having. Again, I do think we've identified a second problem here which I may address myself as I'm on a new project that probably requires deeper levels of nesting in the schema than we've used before.

[stefan-korn]

@dafeder : Hmm, at this moment I can also not reproduce the original issue we had. I suppose by providing the dataset.json that produces an error, I was somehow let wrong. It's true that this error relates to deeper nesting and happens with or without. but the original issue was different.

I was pretty sure about this change then, but now I am a bit baffled with this.

So, it's up to you if you want to get this in, but I do not want to push you if you do not feel comfortable with this.

.. I am currently not feeling comfortable with this whole issue ...

[dafeder]

Yes - I think if I understand the original issue correctly, just creating a dataset with no theme, and creating a theme filtering by theme IS NULL, should not show that dataset right? Because the index would not record "NULL" but whatever would be the equivalent of an empty array in a search index. But I tried this (on 2.x branch) and filtering by theme IS NULL actually produces the correct hit. So.. yes until we can actually demonstrate the effect of this bug in a reproducible way I think we should close this and create a new issue for the nesting problem.