umco / umbraco-vorto

1:1 multilingual property editor wrapper for Umbraco
https://our.umbraco.org/projects/backoffice-extensions/vorto
MIT License
44 stars 42 forks source link

GetVortoValue returns null in 1.7.0 version #116

Closed zd-dalibor closed 5 years ago

zd-dalibor commented 5 years ago

After upgrading to 1.7.0 content.GetVortoValue("someAlias") or content.GetVortoValue<object>("someAlias") returns null, but content.GetVortoValue<string>("someAlias") working.

This content.GetVortoValue("someAlias") worked in 1.6.0.

NikRimington commented 5 years ago

Hi @dalibor983,

Do you have any more information on this? I'm getting an issue even when specifying a return type after upgrading my install to v 1.7.0 as well. This is running on an Umbraco 7.13.1 site.

Thanks

Nik

zd-dalibor commented 5 years ago

Hi @NikRimington,

I've tried to debug 1.7.0 version but there are no debugging symbols for this nuget package.

mattbrailsford commented 5 years ago

@dalibor983 you may need to pull the source down and compile it in debug to generate the PDBs and to set breakpoints into the codebase.

1.7.0 saw a big update to the internal API so it's possible something has changed.

zd-dalibor commented 5 years ago

@mattbrailsford I think I found the problem

https://github.com/umco/umbraco-vorto/blob/44b9af7bec24129d1a008a5f3d1f414fa8c67904/src/Our.Umbraco.Vorto/Extensions/IPublishedContentExtensions.cs#L102

For content.GetVortoValue("someAlias") type of T is object. In my case prop.Value is instance of VortoValue<string> so:

var vortoModel = prop.Value as VortoValue<T>; // casting VortoValue<string> to VortoValue<object>
// vortoModel is null

so private static T DoGetVortoValue<T>(this IPublishedContent content, string propertyAlias, string cultureName = null, recursive = false, T defaultValue = default(T)) will returns null because in my case recursive property is false.

We must find better way to cast prop.Value to VortoValue<T>.

mattbrailsford commented 5 years ago

I'm open to suggestions

mattbrailsford commented 5 years ago

Maybe we could add an implicit operator to VortoValue<T> to allow it to be cast as other types and it takes the generic arg and tries to cast it's generic arg type to that? Or maybe at minimum an implicit operator for VortoValue<object>

mattbrailsford commented 5 years ago

Maybe adding this to VortoValue<T> could work?

public static implicit operator VortoValue<object>(VortoValue<T> v)
{
    var newVortoValue = new VortoValue<object>() {
        Values = v.Values.ToDictionary(x => x.Key, x => x.Value as object)
            .Where(x => x.Value != null)
            .ToDictionary(x => x.Key, x => x.Value)
    };

    return newVortoValue.Values.Count > 0 ? newVortoValue : null;
}
mattbrailsford commented 5 years ago

Actually, this won't work. I think we might need a base class / interface that has an explicit casting method. You can then cast to the interface first to make sure it's a vorto value and then call the cast method to force it to the required type.

mattbrailsford commented 5 years ago

Potential fix here #117 if someone wants to try the nightly https://ci.appveyor.com/project/UMCO/umbraco-vorto/builds/22396168/artifacts

The PR has an annoying ammount of changes due to tabs and spaces, but the main change for this is here https://github.com/umco/umbraco-vorto/pull/117/files#diff-e3939f24b4a5c08135ae25b1a6faf5d5R102. I cast to IVortoValue first to make sure it's a vorto value, and then call a new CastToVortoValue<T>() method.

zd-dalibor commented 5 years ago

@mattbrailsford #117 this fixed the problem to me. It works now.

I tested the source code from wip/vorto-value-casting branch in my project.

ronaldbarendse commented 5 years ago

@mattbrailsford Looks like this should be fixed by using an interface with a covariant generic type parameter: https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/covariance-contravariance/creating-variant-generic-interfaces.

void Main()
{
    var stringValue = new TestValue<string>()
    {
        Value = "Test"
    };

    var objectValue = (stringValue as ITestValue<object>);
    if (objectValue != null)
    {
        // The value could be boxed to object, so return value
    }
    else
    {
        // No conversion exists, so return null
    }
}

public interface ITestValue<out T>
{
    T Value { get; }
}

public class TestValue<T> : ITestValue<T>
{
    public T Value { get; set; }
}