Register DisplayDrivers with ContentOptions

[deanmarcussen]

Far too late to be wanting to get this in now, but further to our conversation @sebastienros here's some ideas for registering DisplayDriver Types in the new ContentOptions, with a factory to resolve them from DI as required, rather than resolving all of them at once and iterating.

Just ideas for the future, but if you like the concept, then this would work for Handlers, as well.

Did think about using a dictionary of Func<> but the factory seemed better

Also will probably need to be changed to a Lookup to support multiple driver registrations against the same part, eg CommonPart Drivers

[deanmarcussen]

@jtkech Thanks for taking a look last night.

I had some time this morning so I refactored, based on your suggestions, and some other things that I wanted to try.

• Removed the lookup dictionary on ContentOptions, and implemented ContentDisplayOptions as a complement too it. Felt it was a bit too leaky, and as this is all about perf, better to have an actual property to use.
• Renamed factories to Resolvers. Much better convention
• Removed weird generic type checking. Better to just rely on where T : IContentPartDisplayDriver
• Made it support multiple drivers and handlers for the same part, including registering seperately, if for example, they are part of a feature.
• Implemented for all Fields, and Handlers as well, to see if it benchmarks any better.

I guess this PR is all about performance, so if @sebastienros feels like benchmarking it, that would be the only way to see if what I've done is even remotely useful?

[carlwoodhouse]

if nothing else its much more intuitive :)

[deanmarcussen]

@sebastienros Any feedback on this idea?

[deanmarcussen]

@jtkech I went through this again to add some logging, and am happy with how it looks, if you had any thoughts they'd be appreciated

[jtkech]

@deanmarcussen okay no problem, i will look at it asap, but so many things ;)

[deanmarcussen]

@sebastienros on your recent pr #4887 you've reduced the allocation pattern for using the invoke extensions.
Just noting that the current BuildDisplayAsync method doesn't use the
await partDisplayDrivers.InvokeAsync(async (driver, part, typePartDefinition, context) => technique at all, it is only the BuildEditorAsync that used it previously.
So if there is an opportunity to reduce allocations on BuildDisplayAsync (regardless of whether this PR goes through), perhaps we should use them?

[deanmarcussen]

ok @sebastienros I reimplemented this with explicit registrations for display modes and editors.

By default registration is * to indicate all display modes.

Other options include registering as

services.AddContentField<MarkdownField>()
    .WithDisplayDriver<MarkdownFieldDisplayDriver>(o =>
    {
        o.ForDisplayModes("*"); // use this driver for all display modes
        o.ForDisplayModes("standard", "my-custom-display-mode"); // use this driver for standard and custom
        o.ForDisplayModes("my-custom-display-mode"); // just my custom mode
});

I have left them as mutable HashSets so registration can be mutated by a user, being able to alter the default registrations if they are implementing a custom editor and want to change the default registrations.

Note parts only have custom editors , not display modes.

Edited

[deanmarcussen]

Updated this after our conversation Seb

Here are some examples of how registration could be done for fields

// Taxonomy Field - Standard registration, will automatically add a Func return true to editor and displaymode
services.AddContentField<TaxonomyField>()
  .UseDisplayDriver<TaxonomyFieldDisplayDriver>();

// Alter display mode in third party module. Run normal driver for standard mode. Run Tag driver for Tags
services.AddContentField<TaxonomyField>() //can be called multiple times to access builder again
  .ForDisplayMode<TaxonomyFieldDisplayDriver>(d => String.IsNullOrEmpty(d) ? true : false)
  .ForDisplayMode<TagFieldDisplayDriver>(d => String.Equals(d, "Tags", StringComparison.OrdinalIgnoreCase) ? true : false);

// Alter editor in third party module
services.AddContentField<TaxonomyField>()
  .ForEditor<TaxonomyFieldDisplayDriver>(e => String.IsNullOrEmpty(e) ? true : false)
  .ForEditor<TagFieldDisplayDriver>(e => String.Equals(e, "Tags", StringComparison.OrdinalIgnoreCase) ? true : false);

// Alter both display mode and editor in third party module
services.AddContentField<TaxonomyField>()
  .UseDisplayDriver<TaxonomyFieldDisplayDriver>(f => String.IsNullOrEmpty(f) ? true : false)
  .UseDisplayDriver<TagFieldDisplayDriver>(d => String.Equals(d, "Tags", StringComparison.OrdinalIgnoreCase) ? true : false);

// Totally remove the default driver (saves running a func against it), and use a custom driver.
services.AddContentField<TaxonomyField>()
  .RemoveDisplayDriver<TaxonomyFieldDisplayDriver>()
  .UseDisplayDriver<MyCustomFieldDisplayDriver>();

Handlers I moved to .Add and .Remove

// Taxonomy Part
  services.AddContentPart<TaxonomyPart>()
    .UseDisplayDriver<TaxonomyPartDisplayDriver>()
    .AddHandler<TaxonomyHandler>();

// Remove handler in third party module and replace with a new handler
services.AddContentPart<TaxonomyPart>()
  .UseDisplayDriver<TaxonomyPartDisplayDriver>()
  .RemoveHandler<TaxonomyHandler>()
  .AddHandler<CustomTaxonomyHandler>();  

So let me know if you agree with the design and like the naming conventions (easy to change)