[Core][Hosting] Resolve handler services through registered service types by mdh1418 · Pull Request #20298 · dotnet/maui

mdh1418 · 2024-02-01T20:13:30Z

Description of Change

Reflection techniques used in MAUI's dynamic resolution of a virtual view's handler are neither AOT friendly nor trimming friendly. It involves iterating through the view's base types (GetServiceBaseTypes) and checking if any registered handler service types match (GetServiceDescriptor).

GetServiceBaseTypes(Type serviceType)

{
	var types = new List<Type> { serviceType };

	Type? baseType = serviceType.BaseType;

	while (baseType != null)
	{
		types.Add(baseType);
		baseType = baseType.BaseType;
	}

	foreach (var interfac in serviceType.GetInterfaces()) // Not AOT friendly, requires trimming annotations
	{
		if (typeof(IView).IsAssignableFrom(interfac))
			types.Add(interfac);
	}

	return types;
}

GetServiceDescriptor(Type serviceType)

{
	if (serviceType == null)
		throw new ArgumentNullException(nameof(serviceType));

	var types = GetServiceBaseTypes(serviceType);

	foreach (var type in types)
	{
		if (_collection.TryGetService(type, out var descriptor) && descriptor != null)
			return descriptor;
	}

	return null;
}

Instead of using reflection to resolve a virtual view's corresponding registered handler, this PR takes inspiration from #20058 and leverages a set of registered handler service types to find a virtual view's corresponding handler.

With the addition of ImageSource service type mappings and Handler service type set, MauiFactory's AOT unfriendly and trimming unsafe logic is no longer needed. This PR opts to simplify MauiFactory, which may be substituted out for the default ServiceProvider in another PR.

This PR does the following:

Modifies MauiHandlersFactory service registration to emulate ImageSourceServiceProvider service registration
Introduces a RegisteredHandlerServiceTypeSet to keep track of handler service types registered through AddHandler and to resolve a virtual view's corresponding handler service type
Cleans up existing tests and adds new tests
Simplifies MauiFactory and removes AOT unfriendly + trimming unsafe logic
Removes MauiFactory build warnings from BuildWarningsUtilities test

Issues Fixed

Contributes to #19397
Fixes #1298 tested via HostBuilderResolvesToHandlerRegisteredUnderMostDerivedBaseInterfaceType

simonrozsival

Looks pretty good!

src/Core/src/Hosting/Internal/RegisteredHandlerServiceTypeSet.cs

src/Core/src/Hosting/MauiHandlersCollectionExtensions.cs

simonrozsival · 2024-02-02T10:25:47Z

src/Core/tests/DeviceTests/Stubs/ElementHandlerStub.cs


 		protected override object CreatePlatformElement() => new Object();
+
+		public override void SetVirtualView(IElement view) => base.SetVirtualView(view);


Is this override necessary?

Yes, there is an exception propagation test

maui/src/Core/tests/UnitTests/HandlerTests.cs

Lines 24 to 77 in 8ba0551

[Fact]

public void ExceptionPropagatesFromHandler()

{

var mauiApp = MauiApp.CreateBuilder()

.ConfigureMauiHandlers(handlers => handlers.AddHandler<IViewStub, ViewHandlerWithChildException>())

.ConfigureMauiHandlers(handlers => handlers.AddHandler<ExceptionThrowingStub, ExceptionThrowingViewHandler>())

.Build();

var mauiContext = new MauiContext(mauiApp.Services);

Assert.Throws<PlatformViewStubCreatePlatformViewException>(() =>

new ViewStub().ToPlatform(mauiContext)

);

}

class ViewHandlerWithChildException : ViewHandlerStub

{

public ViewHandlerWithChildException()

{

}

protected override PlatformViewStub CreatePlatformView()

{

return new PlatformViewStub();

}

public override void SetVirtualView(IView view)

{

base.SetVirtualView(view);

new ExceptionThrowingStub().ToPlatform(MauiContext);

}

}

class ExceptionThrowingStub : ViewStub

{

}

class ExceptionThrowingViewHandler : ViewHandlerStub

{

public ExceptionThrowingViewHandler()

{

}

protected override PlatformViewStub CreatePlatformView()

{

throw new PlatformViewStubCreatePlatformViewException();

}

}

class PlatformViewStubCreatePlatformViewException : Exception

{

}

that tests whether a view/element can throw an exception for another view/element.

I originally moved all HandlerTests to HostBuilderHandlerTests because I didn't see why we really needed the separation, both were in the same testing namespace anyways and both used HostBuilding (MauiApp.CreateBuilder().Build)

Then I figured that the test didn't fit in either, as it wasn't testing the HostBuilder's ability to register and retrieve services, rather it would be an ElementTest, so now its there as ElementToHandlerPropagatesThrownException.

The reason ElementToHandlerPropagatesThrownException needs that declared override void SetVirtualView from what I'm guessing because I don't fully understand the intent of the original test, was to give it some easy access method to throw an exception in the nested view/element after grabbing the view/element's handler in the ToHandler extensions method

maui/src/Core/src/Platform/ElementExtensions.cs

Lines 81 to 99 in 8ba0551

handler = context.Handlers.GetHandler(viewType);

}

catch (MissingMethodException)

{

handler = viewType.CreateTypeWithInjection(context);

if (handler != null)

handlersWithConstructors.Add(view.GetType());

}

}

if (handler == null)

throw new HandlerNotFoundException(view);

handler.SetMauiContext(context);

view.Handler = handler;

if (handler.VirtualView != view)

handler.SetVirtualView(view);

. So once the view/element (ViewWithExceptionThrowingChildStub) grabbed the nested child view/element which throws the exception (ExceptionThrowingViewStub) we just check that the exception propagates through. Originally it was using CreatePlatformView to throw the exception but I figured if we just wanted to propagate an exception being thrown, using SetVirtualView would achieve the same results. But moving from UnitTests to DeviceTests meant that different stubs were available ViewStubHandler vs ElementStubHandler, but I couldn't override SetVirtualView without adding this to the ElementHandlerStub. Maybe I was missing something simple?

So the propagation pattern now looks like
ViewWithExceptionThrowingChildStub().ToHandler -> GetHandler -> ViewWithExceptionThrowingChildHandler.SetVirtualView -> ExceptionThrowingViewStub().ToHandler -> GetHandler + ExceptionThrowingViewHandler.SetVirtualView -> throw HandlerPropagatesException.
Compared to the original test that had
ViewStub().ToHandler -> GetHandler -> ViewHandlerWithChildException.SetVirtualView -> ExceptionThrowingStub().ToHandler->GetHandler->ExceptionThrowingViewHandler.SetVirtualView->ExceptionThrowingViewHandler.CreatePlatformView-> throwPlatformViewStubCreatePlatformViewException`

@PureWeen do you happen to know if I'm on the right track for the original intent of the test?

src/Core/tests/UnitTests/Hosting/RegisteredHandlerServiceTypeSetTests.cs

rmarinho · 2024-02-12T01:53:50Z

/rebase

The logic to dynamically resolve the service associated with a type's inheritance tree is no longer needed with the addition of ImageSourceServiceType mappings and HandlerServiceType set. AOT unfriendy and trimming unsafe logic can be removed.

samhouts added the area-core-hosting Extensions / Hosting / AppBuilder / Startup label Feb 2, 2024

mdh1418 force-pushed the add_handler_service_type_resolver branch from bb78a7a to d31eb32 Compare February 2, 2024 01:30

simonrozsival reviewed Feb 2, 2024

View reviewed changes

mdh1418 force-pushed the add_handler_service_type_resolver branch from d31eb32 to b4a30ec Compare February 2, 2024 17:10

mdh1418 marked this pull request as ready for review February 5, 2024 17:12

mdh1418 requested a review from a team as a code owner February 5, 2024 17:12

mdh1418 requested review from PureWeen, jfversluis, jonathanpeppers, mattleibow and rmarinho February 5, 2024 17:12

mdh1418 added the do-not-merge Don't merge this PR label Feb 5, 2024

mdh1418 force-pushed the add_handler_service_type_resolver branch 2 times, most recently from 23cfb82 to c8da981 Compare February 8, 2024 20:59

simonrozsival mentioned this pull request Feb 9, 2024

[iOS] Resolving Trimming Warnings for dotnet new maui #19397

Closed

19 tasks

mdh1418 added 9 commits February 12, 2024 09:44

[handlers] Model handlers factory after image sources

42140b8

[UnitTests] Fix MauiHandlersFactory service registration

20bec80

[Handlers] Leverage registered handler service types to resolve services

85917b8

Cleanup existing Handler tests

aac155b

[Core][UnitTests] Add more handler test scenarios

3137f89

[UnitTests][Hosting] Add RegisteredHandlerServiceTypeSet Tests

4f4da86

[UnitTests][Hosting] Add GetHandlerType tests

533e2a7

[Hosting] Simplify MauiFactory

4eb8864

The logic to dynamically resolve the service associated with a type's inheritance tree is no longer needed with the addition of ImageSourceServiceType mappings and HandlerServiceType set. AOT unfriendy and trimming unsafe logic can be removed.

[IntegrationTests] Remove MauiFactory warnings

7b1e9c5

mdh1418 force-pushed the add_handler_service_type_resolver branch from c8da981 to 7b1e9c5 Compare February 12, 2024 14:46

mdh1418 removed the do-not-merge Don't merge this PR label Feb 12, 2024

simonrozsival previously approved these changes Feb 12, 2024

View reviewed changes

jonathanpeppers requested a review from StephaneDelcroix February 13, 2024 16:21

Merge branch 'net9.0' into add_handler_service_type_resolver

58167ff

simonrozsival dismissed their stale review via 58167ff February 14, 2024 12:57

rmarinho requested review from simonrozsival and removed request for jfversluis February 19, 2024 18:25

simonrozsival approved these changes Feb 20, 2024

View reviewed changes

StephaneDelcroix approved these changes Feb 20, 2024

View reviewed changes

jonathanpeppers approved these changes Feb 20, 2024

View reviewed changes

rmarinho merged commit e2245e0 into dotnet:net9.0 Feb 20, 2024

rmarinho mentioned this pull request Feb 20, 2024

[Bug] Derived control renders with the handler of the base class #1298

Closed

mdh1418 deleted the add_handler_service_type_resolver branch February 20, 2024 16:43

github-actions bot locked and limited conversation to collaborators Mar 22, 2024

samhouts added fixed-in-9.0.0-preview.2.10247 fixed-in-9.0.0-preview.2.10293 labels Aug 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Core][Hosting] Resolve handler services through registered service types#20298

[Core][Hosting] Resolve handler services through registered service types#20298
rmarinho merged 10 commits intodotnet:net9.0from
mdh1418:add_handler_service_type_resolver

mdh1418 commented Feb 1, 2024 •

edited

Loading

Uh oh!

simonrozsival left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

simonrozsival Feb 2, 2024

Uh oh!

mdh1418 Feb 2, 2024

Uh oh!

Uh oh!

Uh oh!

rmarinho commented Feb 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants


		protected override object CreatePlatformElement() => new Object();

		public override void SetVirtualView(IElement view) => base.SetVirtualView(view);

	[Fact]
	public void ExceptionPropagatesFromHandler()
	{
	var mauiApp = MauiApp.CreateBuilder()
	.ConfigureMauiHandlers(handlers => handlers.AddHandler<IViewStub, ViewHandlerWithChildException>())
	.ConfigureMauiHandlers(handlers => handlers.AddHandler<ExceptionThrowingStub, ExceptionThrowingViewHandler>())
	.Build();

	var mauiContext = new MauiContext(mauiApp.Services);

	Assert.Throws<PlatformViewStubCreatePlatformViewException>(() =>
	new ViewStub().ToPlatform(mauiContext)
	);
	}

	class ViewHandlerWithChildException : ViewHandlerStub
	{
	public ViewHandlerWithChildException()
	{
	}

	protected override PlatformViewStub CreatePlatformView()
	{
	return new PlatformViewStub();
	}

	public override void SetVirtualView(IView view)
	{
	base.SetVirtualView(view);
	new ExceptionThrowingStub().ToPlatform(MauiContext);
	}
	}

	class ExceptionThrowingStub : ViewStub
	{

	}

	class ExceptionThrowingViewHandler : ViewHandlerStub
	{
	public ExceptionThrowingViewHandler()
	{
	}

	protected override PlatformViewStub CreatePlatformView()
	{
	throw new PlatformViewStubCreatePlatformViewException();
	}
	}

	class PlatformViewStubCreatePlatformViewException : Exception
	{

	}

	handler = context.Handlers.GetHandler(viewType);
	}
	catch (MissingMethodException)
	{
	handler = viewType.CreateTypeWithInjection(context);
	if (handler != null)
	handlersWithConstructors.Add(view.GetType());
	}
	}

	if (handler == null)
	throw new HandlerNotFoundException(view);

	handler.SetMauiContext(context);

	view.Handler = handler;

	if (handler.VirtualView != view)
	handler.SetVirtualView(view);

Conversation

mdh1418 commented Feb 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of Change

Issues Fixed

Uh oh!

simonrozsival left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

simonrozsival Feb 2, 2024

Choose a reason for hiding this comment

Uh oh!

mdh1418 Feb 2, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rmarinho commented Feb 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mdh1418 commented Feb 1, 2024 •

edited

Loading