umbraco-community / umbraco-graphql

An implementation of GraphQL for Umbraco 8 using GraphQL for .NET.
MIT License
64 stars 32 forks source link

Custom fields not outputted in umbraco8 branch #47

Closed jsommr closed 5 years ago

jsommr commented 5 years ago

On the demo site, when executing

{
  content {
    atRoot {
      Home {
        _children {
          items {
            ... on Blog {
              _name
              _children {
                items {
                  ... on Blogpost {
                    _name
                    pageTitle
                  }
                }
              }
            }
          }
        }
      }
    }
  }
}

this is returned

{
  "data": {
    "content": {
      "atRoot": {
        "Home": [
          {
            "_children": {
              "items": [
                {},
                {},
                {},
                {
                  "_name": "Blog",
                  "_children": {
                    "items": [
                      {
                        "_name": "My Blog Post",
                        "pageTitle": ""
                      },
                      {
                        "_name": "Another one",
                        "pageTitle": ""
                      },
                      {
                        "_name": "This will be great",
                        "pageTitle": ""
                      }
                    ]
                  }
                },
                {}
              ]
            }
          }
        ]
      }
    }
  }
}

I expected pageTitle to not be an empty string, because Umbraco shows that the fields have data in the UI. This is a fresh Umbraco installation with the demo website installed.

I traced the issue to https://github.com/rasmusjp/umbraco-graphql/blob/09ea3aafc4d738a820f3c8d2b57501391cd924bd/src/Our.Umbraco.GraphQL/Types/PublishedPropertyFieldType.cs#L29-L34 where publishedProperty.GetValue(userContext.Culture) returns "" for pageTitle because culture is passed as argument. Removing it returns the correct value, but we've lost localization support. userContext.Culture = "en-US".

I'm in no way an expert in Umbraco, but the problem seems to be in Umbraco.Web.PublishedCache.NuCache.Property where (my comments in / /)

public override object GetValue(string culture = null, string segment = null)
{
    _content.VariationContextAccessor.ContextualizeVariation(_variations, ref culture, ref segment);

    object value;
    lock (_locko)
    {
        var cacheValues = GetCacheValues(PropertyType.CacheLevel).For(culture, segment);

        // initial reference cache level always is .Content
        const PropertyCacheLevel initialCacheLevel = PropertyCacheLevel.Element;

                 /* cacheValues.ObjectInitialized is false on first run, so not returning here */
        if (cacheValues.ObjectInitialized) return cacheValues.ObjectValue;

                /* GetInterValue(culture, segment) is the interesting call here, see next code snippet */
        cacheValues.ObjectValue = PropertyType.ConvertInterToObject(_content, initialCacheLevel, GetInterValue(culture, segment), _isPreviewing);
        cacheValues.ObjectInitialized = true;
        value = cacheValues.ObjectValue;
    }

    return value;
}
private object GetInterValue(string culture, string segment)
{
    if (culture == "" && segment == "")
    {
        /* Not relevant */
    }

        /* (_sourceValues == null) == true! */
    if (_sourceValues == null)
        _sourceValues = new Dictionary<CompositeStringStringKey, SourceInterValue>();

    var k = new CompositeStringStringKey(culture, segment);

        /* Because _sourceValues have just been initialized, nothing is found and the statement below evaluates to true */
    if (!_sourceValues.TryGetValue(k, out var vvalue))
        {
                /* SourceValue is set to null, which might be the problem, next code snippet shows why */
        _sourceValues[k] = vvalue = new SourceInterValue { Culture = culture, Segment = segment, SourceValue = GetSourceValue(culture, segment) };
        }

        /* This will be false on first run */
    if (vvalue.InterInitialized) return vvalue.InterValue;

        /* Since vvalue.SourceValue == null, PropertyType.ConvertSourceToInter returns null */
    vvalue.InterValue = PropertyType.ConvertSourceToInter(_content, vvalue.SourceValue, _isPreviewing);
    vvalue.InterInitialized = true;
    return vvalue.InterValue;
}
public override object GetSourceValue(string culture = null, string segment = null)
{
    _content.VariationContextAccessor.ContextualizeVariation(_variations, ref culture, ref segment);

    if (culture == "" && segment == "")
        return _sourceValue;

    lock (_locko)
    {
        if (_sourceValues == null) return null;

                /* _sourceValues was just initialized in GetInterValue, so it's going to be empty. No SouceValue will be found, and thus null will be returned */
        if (_sourceValues.TryGetValue(new CompositeStringStringKey(culture, segment), out var sourceValue))
            return sourceValue.SourceValue;
        else
            return null; /* This is returned */
    }
}

It might be an Umbraco bug, or me doing something wrong, but filing it here because I'm clueless and just experimenting with your GraphQL api.

jsommr commented 5 years ago

Enabling language invariants seems to help. So changing https://github.com/rasmusjp/umbraco-graphql/blob/09ea3aafc4d738a820f3c8d2b57501391cd924bd/src/Our.Umbraco.GraphQL/Types/PublishedPropertyFieldType.cs#L29-L34 to

Resolver = new FuncFieldResolver<IPublishedContent, object>(context =>
{
    var userContext = (UmbracoGraphQLContext)context.UserContext;
    IPublishedProperty publishedProperty = context.Source.GetProperty(publishedPropertyType.Alias);

    object value = null;
    var variations = publishedPropertyType.Variations;
    if (variations == ContentVariation.Nothing)
        value = publishedProperty.GetValue();
    else if (variations == ContentVariation.Culture)
        value = publishedProperty.GetValue(userContext.Culture);
    else if (variations == ContentVariation.Segment)
        value = "ContentVariation.Segment: TODO";
    else if (variations == ContentVariation.CultureAndSegment)
        value = "ContentVariation.CultureAndSegment: TODO";

    return foundResolver.Resolve(context.Source, publishedPropertyType, value);
});

should in theory work, but my setup is pretty broken after getting a manually compiled Umbraco up and running. I'm getting some slightly better results, but will have to create a fresh Umbraco install and investigate further.

rasmusjp commented 5 years ago

Not sure if this is still relevant since I'v rewritten most of the code.

Feel free to reopen if you still have problems with the code in the develop branch