notifo-io / sdk-xamarin

6 stars 1 forks source link

Missing config for iOS Notification Service Extension #9

Open CR4567 opened 1 year ago

CR4567 commented 1 year ago

On iOS there is an issue with the notification service extension when trying to send a request to the notifo backend. It seems that the configure of the main app is not used for the service extension. Whenever the service extension is trying to send a request it receives an:

Error: System.InvalidOperationException: Neiter, API Key, nor Client ID and secret is defined.
  at Notifo.SDK.NotifoClientBuilder.Build () [0x00027] in <4749b32e680245aba38c4d8f0f7d70fa>:0 
  at Notifo.SDK.NotifoMobilePush.NotifoClientProvider.get_Client () [0x00021] in /Users/reichelt/Documents/workspace/xamarin/easierlife_app/sdk-xamarin/sdk/Notifo.SDK/NotifoMobilePush/NotifoClientProvider.cs:60 
  at Notifo.SDK.NotifoMobilePush.NotifoMobilePushImplementation.get_Notifications () [0x00000] in /Users/reichelt/Documents/workspace/xamarin/easierlife_app/sdk-xamarin/sdk/Notifo.SDK/NotifoMobilePush/NotifoMobilePushImplementation.shared.cs:56 
  at Notifo.SDK.NotifoMobilePush.TrackSeenCommand.TrackByIdentifierAsync (Notifo.SDK.TrackNotificationDto trackRequest, System.Threading.CancellationToken ct) [0x00020] in /Users/reichelt/Documents/workspace/xamarin/easierlife_app/sdk-xamarin/sdk/Notifo.SDK/NotifoMobilePush/TrackSeenCommand.cs:69 
  at Notifo.SDK.NotifoMobilePush.TrackSeenCommand.ExecuteAsync

when I set a fixed value for the url and apikey for testing purposes directly in front of the calls, it works fine.

Any suggestions how this could be possible? Does it make sense to use the preferences to store these config options in the main app and restore them in the xamarin sdk?

SebastianStehle commented 1 year ago

I think these are 2 different processes. There is no sync or so in place.

CR4567 commented 1 year ago

yes they are and they have there own data containers, but they also have a shared space. The xamarin preferences class abstracts the native stores and should give access to these shared space.

/// <summary>A class to interact with the preferences/settings of the native platform.</summary>
/// <remarks>
///     <para>Each platform uses the platform provided native APIs for storing application/user preferences:</para>
///     <list type="bullet">
///         <item>
///             <term>iOS: NSUserDefaults</term>
///         </item>
///         <item>
///             <term>Android: SharedPreferences</term>
///         </item>
///         <item>
///             <term>UWP: ApplicationDataContainer</term>
///         </item>
///     </list>
/// </remarks>

1_5uA0PvKYitVU1Bab7es_GA@2x

But 'm not sure if this will work. The only other option would be to disable the authentication for these specific calls or switch back to the tokenConfirmUrl of the notification data. But for querying unseen notifications this would be still an issue.

SebastianStehle commented 1 year ago

Yes, we have to use the confirm URL again I think.

I would either...

  1. Introduce a new TrackingCommand for URLs
  2. Update the current tracking command to handle URLs as fallback.

I think the command would update the state anyway, so it would not cause issues for unseen state. But we might have to change our logic here and we cannot keep a local version anymore.

CR4567 commented 1 year ago

With the recent adoptions in branch fallback-tracking, sharing the credentials seems to work fine. But there seems to be an issue with the http client. When configuring notifo SDK by the main app everything works fine but the notification service extension is a seperated process which doesn't get these information. So there is always a missing apiKey and apiUrl because the setApiUrl and setApiKey are never called. I can hack it by changing the NotifoClientProvider a little bit

      public INotifoClient Client
      {
          get
          {
              clientBuilder.SetClient(CreateHttpClient());
              clientBuilder.SetApiKey(ApiKey);
              clientBuilder.SetApiUrl(ApiUrl);
              clientInstance = clientBuilder.Build();

              return clientInstance;
          }
      }

So each time the builder is updated. Not perfect but in this way the service extension is working correctly. An idea how we could handle this in a better way? Something like a propertyChanged would be nice but I don't think that this would work in this place.

SebastianStehle commented 1 year ago

I think it should be solved in the notifo SDK, so that there is a way to resolve the api key and url from another data source or function.

Then we could just get rid of the provider.

SebastianStehle commented 1 year ago

I have extended the SDK, The options are now injected from an INotifoOptions interface, which means we can provide our own implementation, that loads everything from the settings.