Fix: ServiceAttribute is missing foregroundServiceType by gugavaro · Pull Request #3810 · dotnet/android

gugavaro · 2019-10-17T13:20:11Z

Our tools currently do not automatically add new manifest properties.
Manually adding ForegroundServiceType property to ServiceAttribute class

Tracking issue: #3712

jonpryor · 2019-10-24T21:44:36Z

This should also add a test, probably by just using the new attribute in src/Mono.Android/Test, but something in src/Xamarin.Android.Build.Tasks/Tests may also be desirable.

Regardless, there's a fundamental problem: the //service/@android:foregroundServiceType attribute wants a limited set of strings:

["connectedDevice" | "dataSync" |
    "location" | "mediaPlayback" | "mediaProjection" |
    "phoneCall"]

These do map (poorly) with the ForegroundService enum members, but nothing actually converts between the two. Thus, two things need to be done:

Write a conversion method between the ForegroundService enum and a string:

		static string ToString (ForegroundService value)
		{
			switch (value) {
				case ForegroundService.TypeConnectedDevice:   return "connectedDevice";
				...
				default:
					throw new ArgumentException ($"Unsupported ForegroundService value '{value}'.", nameof(value));
			}
		}

Add this method to the dictionary at:

https://github.com/xamarin/xamarin-android/blob/39bcbb85d662c18e5cdbd4bed091980f68288028/src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocumentElement.cs#L180

static readonly Dictionary<Type, Func<object, ICustomAttributeProvider, IAssemblyResolver, int, string>> ValueConverters = new Dictionary<Type, Func<object, ICustomAttributeProvider, IAssemblyResolver, int, string>> () {
    // ...
    { typeof (ForegroundService),     (value, p, r, v) => ToString ((ForegroundService) value) },
};

This should ensure that the generated //service/@android:foregroundServiceType is one of the expected & correct string values, not...a number, or the enum member name, or whatever the current code will be generating.

jonpryor · 2019-11-07T19:33:45Z

+				case ForegroundService.TypeConnectedDevice:	return "connectedDevice";
+				case ForegroundService.TypeDataSync:		return "dataSync";
+				case ForegroundService.TypeLocation:		return "location";
+				case ForegroundService.TypeManifest:		return "manifest";


Oddly, manifest isn't a documented value at: https://developer.android.com/guide/topics/manifest/service-element

I'm not sure what to make of that. Perhaps it shouldn't be supported, and should hit the default case?

Good catch!
I opened Android Studio and it is not an option on the given list. So I guess it is not supported and should hit default case.

This is something to keep in mind if/when you get around to auto-producing these custom attributes based on the manifest XML: the XML may contain values which are not documented and which Android Studio doesn't "support".

I'm not sure if this can be fully automated.

(Though even partial automation would presumably be an improvement.)

jahmai-ca · 2019-11-13T05:57:58Z

Does this allow for multiple foreground service types?

android:foregroundServiceType
Specify that the service is a foreground service that satisfies a particular use case. For example, a foreground service type of "location" indicates that an app is getting the device's current location, usually to continue a user-initiated action related to device location.

*You can assign multiple foreground service types to a particular service.*

Fixes: dotnet#3712 API-29 added a new [`//service/@android:foregroundServiceType`][0] attribute to `AndroidManifest.xml`, but we forgot to bind this attribute as an `Android.App.ServiceAttribute.ForegroundServiceType` property so that the attribute could be *generated*. Add a `ServiceAttribute.ForegroundServiceType` property, which when set will emit the `//service/@android:foregroundServiceType` attribute within `AndroidManifest.xml`. [0]: https://developer.android.com/guide/topics/manifest/service-element#foregroundservicetype

Fixes: #3712 API-29 added a new [`//service/@android:foregroundServiceType`][0] attribute to `AndroidManifest.xml`, but we forgot to bind this attribute as an `Android.App.ServiceAttribute.ForegroundServiceType` property so that the attribute could be *generated*. Add a `ServiceAttribute.ForegroundServiceType` property, which when set will emit the `//service/@android:foregroundServiceType` attribute within `AndroidManifest.xml`. [0]: https://developer.android.com/guide/topics/manifest/service-element#foregroundservicetype

gugavaro requested review from dellis1972, jonathanpeppers and jonpryor as code owners October 17, 2019 13:20

dellis1972 approved these changes Oct 17, 2019

View reviewed changes

gugavaro added the do-not-merge PR should not be merged. label Oct 17, 2019

gugavaro changed the title ~~Fix: ServiceAttribute is missing foregroundServiceType~~ [Do Not Review Yet] Fix: ServiceAttribute is missing foregroundServiceType Oct 17, 2019

jonathanpeppers approved these changes Oct 17, 2019

View reviewed changes

gugavaro force-pushed the gugavaro_ServiceAttribute branch from 05661b4 to f24dd6e Compare October 17, 2019 13:58

gugavaro added Area: App Runtime Issues in `libmonodroid.so`. Area: Bindings Issues in Java Library Binding projects. Area: dotnet/android Build Issues building the dotnet/android repo *itself*. labels Oct 17, 2019

gugavaro force-pushed the gugavaro_ServiceAttribute branch 2 times, most recently from f8d3adf to 1b40e7e Compare October 17, 2019 23:04

gugavaro removed Area: App Runtime Issues in `libmonodroid.so`. Area: Bindings Issues in Java Library Binding projects. Area: dotnet/android Build Issues building the dotnet/android repo *itself*. labels Oct 17, 2019

gugavaro force-pushed the gugavaro_ServiceAttribute branch 2 times, most recently from 3afae18 to 1a9bdf8 Compare October 18, 2019 18:00

gugavaro changed the title ~~[Do Not Review Yet] Fix: ServiceAttribute is missing foregroundServiceType~~ Fix: ServiceAttribute is missing foregroundServiceType Oct 19, 2019

gugavaro removed the do-not-merge PR should not be merged. label Oct 19, 2019

gugavaro force-pushed the gugavaro_ServiceAttribute branch 3 times, most recently from f16f94c to 39bcbb8 Compare October 24, 2019 19:20

jonpryor reviewed Oct 24, 2019

View reviewed changes

Comment thread src/Xamarin.Android.Build.Tasks/Mono.Android/ServiceAttribute.Partial.cs Outdated

jonpryor reviewed Oct 24, 2019

View reviewed changes

Comment thread src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.props.in Outdated

gugavaro force-pushed the gugavaro_ServiceAttribute branch 4 times, most recently from b8bce10 to 5d6c4ff Compare October 28, 2019 17:46

gugavaro requested a review from jonpryor October 28, 2019 18:19

gugavaro force-pushed the gugavaro_ServiceAttribute branch 2 times, most recently from 3d386d4 to d7580ef Compare November 7, 2019 01:13

jonpryor reviewed Nov 7, 2019

View reviewed changes

Fix: ServiceAttribute is missing foregroundServiceType

1e24630

gugavaro force-pushed the gugavaro_ServiceAttribute branch from d7580ef to 1e24630 Compare November 7, 2019 20:14

gugavaro requested a review from jonpryor November 12, 2019 15:19

Alphabetize & fix indentation.

579fcd8

jonpryor merged commit 6e55c22 into master Nov 12, 2019

brendanzagaeski added this to the In Preview: 10.2 (d16-5) milestone Jan 22, 2020

gugavaro deleted the gugavaro_ServiceAttribute branch February 11, 2020 13:34

jpobst mentioned this pull request Jun 2, 2020

[XA.Build.Tasks] Update possible values for manifest foregroundServiceType. #4753

Merged

jpobst mentioned this pull request Aug 7, 2023

IntentFilterAttribute is missing PathSuffix and PathAdvancePattern #8235

Closed

github-actions Bot locked and limited conversation to collaborators Jan 24, 2024

Navigation Menu

Repository navigation

Fix: ServiceAttribute is missing foregroundServiceType - #3810

Fix: ServiceAttribute is missing foregroundServiceType#3810
jonpryor merged 2 commits into
masterfrom
gugavaro_ServiceAttribute

gugavaro commented Oct 17, 2019

Uh oh!

Uh oh!

Uh oh!

jonpryor commented Oct 24, 2019

Uh oh!

jonpryor Nov 7, 2019

Uh oh!

gugavaro Nov 7, 2019

Uh oh!

jonpryor Nov 12, 2019

Uh oh!

jahmai-ca commented Nov 13, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

gugavaro commented Oct 17, 2019

Uh oh!

Uh oh!

Uh oh!

jonpryor commented Oct 24, 2019

Uh oh!

jonpryor Nov 7, 2019

Choose a reason for hiding this comment

Uh oh!

gugavaro Nov 7, 2019

Choose a reason for hiding this comment

Uh oh!

jonpryor Nov 12, 2019

Choose a reason for hiding this comment

Uh oh!

jahmai-ca commented Nov 13, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants