modelsbuilder / ModelsBuilder.Original

The Community Models Builder for Umbraco
MIT License
114 stars 49 forks source link

Generate metadata #215

Open zpqrtbnk opened 5 years ago

zpqrtbnk commented 5 years ago

This is a feature that handles issues #157, #145... and PR #122. Idea is to expose a MB static class (name can be configured) that gives access to models metadata.

Usage:

// get the alias of a content type
var typeAlias = MB.ContentAlias.MyType;

// gets the alias of a property type
var propertyAlias = MB.PropertyAlias.MyType.MyProperty;

// gets the published content type of a content type
var type = MB.ContentType.MyType;

// gets the published property type of a property type
var type = MB.PropertyType.MyType.MyProperty;

Aliases are generated as string constants, and thus can be used in code with minimal overhead.

This feature is open for comment.

zpqrtbnk commented 5 years ago

Extended with:

var infos = MB.Models;
var info = MB.Model("contentTypeAlias");

Where info contains the alias, the CLR name and CLR type of the content type - and properties, which in turn contain the alias, the CLR name and CLR type of the property. This provides full models discovery and could be used by query builders.

hfloyd commented 5 years ago

In terms of naming this "Metadata class" - I think a second customization option set in the MB config.cs file would make the most sense.

Example:

[assembly: ModelsNamespace("MySite.Models")]
[assembly: ModelsMetaNamespace("MySite.ModelsInfo")]
zpqrtbnk commented 5 years ago

Sure. It's prob going to be

[assembly:ModelsBuilderConfigure(Namespace="MySite.Models", MBClassName="ModelsInfo"))]

Looks good?

hfloyd commented 5 years ago

Sure :-)

ronaldbarendse commented 5 years ago

I would recommend changing the default static class name to Constants to keep it inline with Umbraco (core). The property on the ModelsBuilderConfigureAttribute could then also be renamed from MBClassName to ConstantsClassName, which seems a little more descriptive to me.

And having it generated in the same namespace as the models should be a good default, although if you want change it, a ConstantsNamespace property could also be added.

zpqrtbnk commented 5 years ago

Totally love the Constants name - just hoping the collision with Core won't be annoying? Anyways, the name of the class is configurable, so Constants is a fine default I guess.

As for namespaces, there's another PR for allowing way more configuration of models namespaces, so... also being able to configure the namespace of the constants class make sense.

ronaldbarendse commented 5 years ago

I have some issues with splitting the content and property aliasses (ContentAlias and PropertyAlias): having them within the same parent class seems more logical to me.

Also, returning the published types as properties doesn't seem right, as it invokes a method that does quite some heavy lifting (and requires an UmbracoContext).

Something like this incorporates these remarks and allows extending with CLR types:

public static partial class Constants
{
    public static partial class Project
    {
        public const string Alias = "project";
        public const PublishedItemType ItemType = PublishedItemType.Content;

        public static IPublishedContentType GetModelContentType() => PublishedModelUtility.GetModelContentType(ItemType, Alias);

        public static partial class Properties
        {
            public const string ProductsAlias = "products";

            public static IPublishedPropertyType GetModelPropertyType(string alias) => GetModelContentType().GetPropertyType(alias);
        }
    }

    public static partial class Image
    {
        public const string Alias = "image";
        public const PublishedItemType ItemType = PublishedItemType.Media;

        public static IPublishedContentType GetModelContentType() => PublishedModelUtility.GetModelContentType(ItemType, Alias);

        public static partial class Properties
        {
            public const string UmbracoFileAlias = "umbracoFile";
            public const string UmbracoWidthAlias = "umbracoWidth";
            public const string UmbracoHeightAlias = "umbracoHeight";
            public const string UmbracoBytesAlias = "umbracoBytes";
            public const string UmbracoExtensionAlias = "umbracoExtension";

            public static IPublishedPropertyType GetModelPropertyType(string alias) => GetModelContentType().GetPropertyType(alias);
        }
    }

    public static partial class Member
    {
        // ...
    }
}

The CLR types of both the model and properties could be added as:

public static partial class Constants
{
    public static partial class Project
    {
        public static Type Type => typeof([Namespace].Project);

        public static partial class Properties
        {
            public static Type ProductsType => typeof(...);
        }
    }
}
zpqrtbnk commented 5 years ago

Super happy to have feedback on this!

I see what you've done. Grouping things in a more hierarchical way instead of splitting right after Constants. I thought about it but was afraid it would be more verbose. Yet... maybe it makes more sense indeed. What about this:

var contentTypeAlias = Constants.SomeType.Alias;
var contentTypeItemType = Constants.SomeType.ItemType;
var contentTypeType = Constants.SomeType.GetContentType();
var propertyTypeAlias = Constants.SomeType.Properties.SomeProperty.Alias;
var propertyTypeType = Constants.SomeType.Properties.SomeProperty.GetPropertyType();

We could also have Constants.SomeType.Properties.SomeProperty.ClrType return the CLR type of the property...

Now I remember why I avoided "Constants" - because GetContentType() or GetPropertyType() are not really constants - but MB is ugly, I totally agree. Maybe "Constants` is not "pure" but makes plenty of sense.

Now... take in account that we may want to model the dictionary, too, eventually. What about turning Constants.SomeType into Constants.ModelTypes.SomeType so we can always do Constants.Dictionary later on?