Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP notification service extension service in dotnet #111

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
<LibraryProjectZip Include="Jars\core-release.aar" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="Xamarin.AndroidX.Core" Version="1.12.0.4" />
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need this for the setExtender method on IDisplayableNotification

<PackageReference Include="Xamarin.Kotlin.StdLib.Jdk8" Version="1.8.0.1" />
</ItemGroup>
</Project>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using AndroidX.Core.App;

namespace Com.OneSignal.Notifications.Android
{
public abstract class NotificationExtenderBase : Java.Lang.Object, NotificationCompat.IExtender
{
public abstract NotificationCompat.Builder Extend(NotificationCompat.Builder builder);
}
Comment on lines +5 to +8
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a plumbing class the, main purpose of this class is to inherit Java.Lang.Object to create a peer object and allow programmer to not have to think about peer objects.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
using Android.Content;

namespace Com.OneSignal.Android.Notifications
{
public abstract class NotificationServiceExtenderBase: Java.Lang.Object, Com.OneSignal.Android.Notifications.INotificationServiceExtension
{
public abstract void OnNotificationReceived(Com.OneSignal.Android.Notifications.INotificationReceivedEvent ev);
}
Comment on lines +5 to +8
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same idea than the other class.
Inherits from java.lang.object to create peer object, this is meant to be used as base class by the library-user.

}

Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@
<LibraryProjectZip Include="Jars\notifications-release.aar" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\OneSignalSDK.DotNet.Android.Core.Binding\OneSignalSDK.DotNet.Android.Core.Binding.csproj" />
</ItemGroup>
Comment on lines +58 to +59
Copy link
Author

@tmijieux tmijieux Jun 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dependency on the core binding project from the notifications binding to get types from the core library that are used in the notifications library. wWthout this, some api were missing, but I cant remember which one exactly at the time i write this comment.
I will re-review this later and check if this is absolutely required or not...

<ItemGroup>
<PackageReference Include="Xamarin.AndroidX.Core" Version="1.12.0.4" />
<PackageReference Include="Xamarin.Kotlin.StdLib.Jdk8" Version="1.8.0.1" />
</ItemGroup>
</Project>
</Project>
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
<metadata>
<attr path="/api/package[@name='com.onesignal.notifications']" name="managedName">Com.OneSignal.Android.Notifications</attr>
<attr path="/api/package[@name='com.onesignal.notifications']" name="managedName">Com.OneSignal.Android.Notifications</attr>

<attr path="/api/package[@name='com.onesignal.notifications.internal']/class[@name='Notification']/method[@name='getGroupedNotifications' and count(parameter)=0]" name="return">java.util.List&lt;com.onesignal.notifications.INotification&gt;</attr>
Copy link
Author

@tmijieux tmijieux Jun 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because of the new androidx api that were brought in, this api (getGroupedNotifications() ) was not generated before, is now generated.

In the java world , the class implements the interface by returning a list of concrete class (i.e List<com.onesignal.notifications.internal.Notification> while the interface expects a return type of List<com.onesignal.notifications.INotification> ( the internal Notification implements the INotification)

and this kind of polymorphism on the generic parameter of List is not accepted in c# , (i guess this is allowed in java ? ).

this was triggering a compilation error, so i tell the binding generation to replace return type by List in the class( i did not tested it yet ? i dont know if it works yet)

</metadata>
10 changes: 7 additions & 3 deletions OneSignalSDK.DotNet.Android/OneSignalSDK.DotNet.Android.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@
</ItemGroup>
<ItemGroup>
<PackageReference Include="Xamarin.Kotlin.StdLib.Jdk8" Version="1.8.0.1" />
<PackageReference Include="Xamarin.KotlinX.Coroutines.Core" Version="1.6.4.2" />
<PackageReference Include="Xamarin.KotlinX.Coroutines.Android" Version="1.6.4.2" />
<PackageReference Include="Xamarin.KotlinX.Coroutines.Core" Version="1.8.0.1" />
<PackageReference Include="Xamarin.KotlinX.Coroutines.Android" Version="1.8.0.1" />
<PackageReference Include="Xamarin.AndroidX.CardView" Version="1.0.0.16" />
<PackageReference Include="Xamarin.AndroidX.Legacy.Support.V4" Version="1.0.0.14" />
<PackageReference Include="Xamarin.AndroidX.Browser" Version="1.4.0.2" />
Expand All @@ -66,5 +66,9 @@
<PackageReference Include="Xamarin.Google.Dagger" Version="2.41.0.2" />
<PackageReference Include="Xamarin.GooglePlayServices.Base" Version="118.1.0" />
<PackageReference Include="Xamarin.AndroidX.Work.Work.Runtime.Ktx" Version="2.7.1.5" />
<PackageReference Include="Xamarin.AndroidX.Collection" Version="1.4.0.1" />
<PackageReference Include="Xamarin.AndroidX.Collection.Ktx" Version="1.4.0.1" />
<PackageReference Include="Xamarin.AndroidX.Activity" Version="1.7.2" />
<PackageReference Include="Xamarin.AndroidX.Activity.Ktx" Version="1.7.2" />
Comment on lines +69 to +72
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all this diffs are because of many conflicts i had on this binding library dependencies.
This part is not working yet and is subject to change a lot.

</ItemGroup>
</Project>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using System;
using System.Diagnostics;
using Android.App;
using Android.Content;

namespace Com.OneSignal.Android
{
[Serializable]
[AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = false)]
public sealed class NotificationExtensionAttribute : Attribute, Java.Interop.IJniNameProviderAttribute
{
public string Name { get; set; }
public NotificationExtensionAttribute()
{
}
}
Comment on lines +8 to +16
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the dotnet android team have made these compiling steps where they generate a class in java world from the name specified in any attribute that inherits from IJniNameProviderAttribute,

read the following files for details:

https://github.com/dotnet/java-interop/blob/ccafbe6a102a85efe84ceb210b3b8b93e49edbcb/src/Java.Interop.Tools.TypeNameMappings/Java.Interop.Tools.TypeNameMappings/JavaNativeTypeManager.cs#L314-L320

}
2 changes: 1 addition & 1 deletion OneSignalSDK.DotNet.nuspec
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="utf-8"?>
<package xmlns="http://schemas.microsoft.com/packaging/2012/06/nuspec.xsd">
<metadata>
<version>5.1.3</version>
<version>5.1.3.1-alpha.1</version>
<id>OneSignalSDK.DotNet</id>
Comment on lines +4 to 5
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i just bumped here to test with a locally built nuget.
this is not necessarily meant to stay

<title>OneSignal SDK for .Net6+ and Xamarin</title>
<authors>OneSignal</authors>
Expand Down
3 changes: 2 additions & 1 deletion OneSignalSDK.DotNet/OneSignalSDK.DotNet.csproj
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>net7.0;net7.0-android;net7.0-ios</TargetFrameworks>
<TargetFrameworks>netstandard2.0;net7.0;net7.0-android;net7.0-ios</TargetFrameworks>
<ImplicitUsings>enable</ImplicitUsings>
Comment on lines +4 to 5
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the nuget spec file seemed to expect netstandard dlls in this project. i did this because i wanted to test with a locally built nuget.

what to do in details here is up to the maintainer i guess.

<Nullable>enable</Nullable>
<LangVersion>10.0</LangVersion>
</PropertyGroup>
<ItemGroup>
<ProjectReference Include="..\OneSignalSDK.DotNet.Core\OneSignalSDK.DotNet.Core.csproj" />
Expand Down