Paths

Table of Contentst

Differential D12748

Expose function to get notifs inbound keys to C++
ClosedPublic
Actions

Authored by marcin on Jul 15 2024, 4:48 AM.

Tags

None

Referenced Files

	Unknown Object (File)
	Tue, Oct 22, 1:16 PM

	Unknown Object (File)
	Tue, Oct 22, 9:08 AM

	Unknown Object (File)
	Tue, Oct 22, 8:33 AM

	Unknown Object (File)
	Tue, Oct 22, 8:33 AM

	Unknown Object (File)
	Tue, Oct 22, 8:33 AM

	Unknown Object (File)
	Tue, Oct 22, 8:33 AM

	Unknown Object (File)
	Sun, Oct 20, 9:36 PM

	Unknown Object (File)
	Sep 18 2024, 7:07 PM

View All 72 Files

Subscribers

Details

Reviewers

kamil
bartek

Commits

rCOMM50a22635cf7d: Expose function to get notifs inbound keys to C++

Summary

This differential creates simple C++ class that calls platform-specific method to query for inbound keys. It is necessary have common race condition handling implementation in C++.

Test Plan

Call this function from CommCoreModule (e.x. place that gets called when updating draft) for some device ID taken from staging identity. Examine logs

Diff Detail

Repository

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

marcin created this revision.Jul 15 2024, 4:48 AM

Herald added a subscriber: ashoat. · View Herald TranscriptJul 15 2024, 4:48 AM

marcin added a parent revision: D12728: Implement Android HTTP call to identity to get notifs inbound keys.Jul 15 2024, 4:49 AM

marcin added inline comments.Jul 15 2024, 4:58 AM

native/android/app/src/main/java/app/comm/android/fbjni/NotificationsInboundKeysProvider.java
15 ↗	(On Diff #42288)	Why return JSON with `curve25519` only? We only need `curve25519` to create inbound session. In next diff I will modify `Session.cpp` to use `create_inbound_from` instead of `create_inbound`. In `Session.cpp` there is a convention that we pass keys as JSON's and then add length of `curve25519` or `ed25519` string to advance pointer to the position where to key value begins. I could return the whole JSON but I am afraid that JSON after stringification might not preserve the order and put `ed25519` before `curve25519` which could lead to serious issues.
native/ios/Comm/NotificationsInboundKeysProvider.mm
22 ↗	(On Diff #42288)	Explanation here is the same as for Android implementation.

Harbormaster completed remote builds in B30326: Diff 42288.Jul 15 2024, 5:08 AM

marcin requested review of this revision.Jul 15 2024, 5:08 AM

marcin added a child revision: D12750: Implement inbound notif session creation from the NSE with race condition handling.Jul 15 2024, 5:19 AM

marcin removed a reviewer: tomek.Jul 15 2024, 8:07 AM

kamil accepted this revision.Jul 19 2024, 5:31 AM

kamil added inline comments.

native/android/app/src/main/java/app/comm/android/fbjni/NotificationsInboundKeysProvider.java
15 ↗	(On Diff #42288)	we should implement ENG-6429 one day
15 ↗	(On Diff #42288)	could you add a code comment explaining this?

This revision is now accepted and ready to land.Jul 19 2024, 5:31 AM

Address review

Harbormaster failed remote builds in B30640: Diff 42735!Jul 24 2024, 6:29 AM

Harbormaster completed remote builds in B30640: Diff 42735.Jul 24 2024, 9:10 AM

Rebase before landing

Harbormaster completed remote builds in B30903: Diff 43083.Aug 5 2024, 6:07 AM

Closed by commit rCOMM50a22635cf7d: Expose function to get notifs inbound keys to C++ (authored by marcin, committed by Marcin Wąsowicz <marcinwasowicz@MacBook-Pro-Marcin.local>). · Explain WhyAug 5 2024, 6:40 AM

This revision was automatically updated to reflect the committed changes.

Marcin Wąsowicz <marcinwasowicz@MacBook-Pro-Marcin.local> added a commit: rCOMM50a22635cf7d: Expose function to get notifs inbound keys to C++.

Revision Contents
Changeset List

Path

Size

native/

android/

app/

1 line

src/

cpp/

NotificationsInboundKeysProvider.cpp

32 lines

main/

java/

app/

comm/

android/

commservices/

CommAndroidServicesClient.java

7 lines

fbjni/

NotificationsInboundKeysProvider.java

24 lines

notifications/

CommNotificationsHandler.java

7 lines

cpp/

CommonCpp/

Notifications/

BackgroundDataStorage/

NotificationsInboundKeysProvider.h

11 lines

ios/

Comm.xcodeproj/

project.pbxproj

8 lines

Comm/

NotificationsInboundKeysProvider.mm

31 lines

Diff 43083

native/android/app/CMakeLists.txt

Loading...

native/android/app/src/cpp/NotificationsInboundKeysProvider.cpp

Loading...

native/android/app/src/main/java/app/comm/android/commservices/CommAndroidServicesClient.java

Loading...

native/android/app/src/main/java/app/comm/android/fbjni/NotificationsInboundKeysProvider.java

Loading...

native/android/app/src/main/java/app/comm/android/notifications/CommNotificationsHandler.java

Loading...

native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsInboundKeysProvider.h

Loading...

native/ios/Comm.xcodeproj/project.pbxproj

Loading...

native/ios/Comm/NotificationsInboundKeysProvider.mm

Loading...