fix: APN notification topic not composed based on push type

[cbaker6]

New Pull Request Checklist

• I am not disclosing a vulnerability.
• I am creating this PR in reference to an issue.

Issue Description

An APN notification topic currently stays as the topic initialized by the Parse-Server (_Installation.appIdentifier) and isn't updated based on pushType. Based on the Apple Documentation, a specific pushType may require a topic to change.

In addition, the notification header determined by parse-server-push-adapter currently has the highest priority. This makes it harder for the developer to specify particular header information in Cloud Code to prioritize over parse-server-push-adapter header information (parse-server running in memory). This is also useful when Apple updates/adds new notification types and allows devs to add updates to their respective Cloud Code dynamically and adapt the latest features.

Closes: #348

Approach

• Update notification topic based on pushType
• Prioritize notification information passed as data over default header information without breaking the current API (see explanation above)
• Clean up code/errors

TODOs before merging

• Add tests

[parse-github-assistant]

Thanks for opening this pull request!

• 🎉 We are excited about your hands-on contribution!

[cbaker6]

@mtrezza this PR is ready for review. After you approve the workflow to run, I'll address any test-passing issues. The tests currently pass on my local machine, but it's possible there can be failures since the CI uses older versions of Node.

[cbaker6]

Don’t merge yet, should have an additional commit up within the next hour

[cbaker6]

@mtrezza I'm finished with my updates, ready for your review

[mtrezza]

Is there a breaking change in this, since it now updates the topic based on push type?

[cbaker6]

Is there a breaking change in this since it now updates the topic based on push type?

I don't believe the changes are breaking, but let me know what you think based on my explanation below:

The current topic comes from the first device (_Installation) appIdentifier property in the list of devices passed to the Parse send method here:

parse-server-push-adapter/src/APNS.js

Lines 86 to 105 in 3cb34c4

	// Start by clustering the devices per appIdentifier
	allDevices.forEach(device => {
	const appIdentifier = device.appIdentifier;
	devicesPerAppIdentifier[appIdentifier] = devicesPerAppIdentifier[appIdentifier] || [];
	devicesPerAppIdentifier[appIdentifier].push(device);
	});
	for (const key in devicesPerAppIdentifier) {
	const devices = devicesPerAppIdentifier[key];
	const appIdentifier = devices[0].appIdentifier;
	const providers = this._chooseProviders(appIdentifier);
	// No Providers found
	if (!providers || providers.length === 0) {
	const errorPromises = devices.map(device => APNS._createErrorPromise(device.deviceToken, 'No Provider found'));
	allPromises = allPromises.concat(errorPromises);
	continue;
	}
	const headers = { expirationTime: expirationTime, topic: appIdentifier, collapseId: collapseId, pushType: pushType, priority: priority }

This method is called from the PushAdapter here:

parse-server-push-adapter/src/ParsePushAdapter.js

Lines 68 to 87 in 3cb34c4

	send(data, installations) {
	const deviceMap = classifyInstallations(installations, this.validPushTypes);
	const sendPromises = [];
	for (const pushType in deviceMap) {
	const sender = this.senderMap[pushType];
	const devices = deviceMap[pushType];
	if(Array.isArray(devices) && devices.length > 0) {
	if (!sender) {
	log.verbose(LOG_PREFIX, `Can not find sender for push type ${pushType}, ${data}`)
	const results = devices.map((device) => {
	return Promise.resolve({
	device,
	transmitted: false,
	response: {'error': `Can not find sender for push type ${pushType}, ${data}`}
	})
	});
	sendPromises.push(Promise.all(results));
	} else {
	sendPromises.push(sender.send(data, devices));

My thought is, "technically", this value comes directly from the first _Installation and remains constant for a particular app. Before the current PR, there wasn't a way to replace a different value for topic via this repo or node-apn (not even through rawPayload as this is a header value and rawPayload has no effect on header values). Of course, a "savvy" Parse developer may have decided to modify the "first" appIdentifier of a list of _Installation's before calling the send method, but this would be a hack IMO, so I don't particularly count breaking the hack as a "breaking change".

I guess that other developers who have LiveActivity or other pushTypes that require changes to the topic working are either not using the ParsePushAdapter (using another package or custom code to send them) or APN isn't enforcing the topic changes per pushType, but may start enforcing it in the future since it's documented.

Definitely let me know if you agree/disagree, as this is just my opinion.

[cbaker6]

In addition, the code changes pass all the old tests without modification. Which can be used as a lightweight indicator of "non-breaking," but that depends on the test suite's robustness in capturing the library's current expectations.

[mtrezza]

I think we can merge as non-breaking, with a proper description in the changelog. I actually looks like a bug fix, so how about:

fix: APN notification topic not composed based on push type

[cbaker6]

LGTM!

[parseplatformorg]

🎉 This change has been released in version 6.9.1