Refactor bindings and subscriptions

[DerAndereAndi]

• Refactor BindingManager and SubscriptionManager interface APIs
• Add support for read requests to NodeManagementBindingData and NodeManagementSubscriptionData
• Exclude sending notifications for updates on NodeManagementBindingData and NodeManagementSubscriptionData (see code comment in spine/feature_local.go method SetData on why this is done)
• Store bindings and subscriptions in NodeManagement data structures
• Store bindings and subscriptions for local client and server features
• Only store a requested binding or subscription if the remote accepted it
• Adding an already existing binding or subcription again will not return an error
• On add requests, only check for proper roles, if the serverFeatureType is provided on request
• Add support for deleting all bindings or subscriptions with the same role relation by omitting feature address or also entity address
• Add support for delete requests from a server feature to a client feature
• Fix a crash when incoming subscription requests where missing the optional serverFeatureType
• Improve test cases to consider bindings and subscriptions of multiple remote devices, which should only get their corresponding bindings and subscriptions when read

Fixes #56

[coveralls]

coverage: 93.893% (+0.1%) from 93.773%
when pulling 130494a on feature/nodemanagment-improvements
into cd41007 on dev.

[sthelen-enqs]

I've gone through about half the files and looks good so far. I've added a few suggestions for some comments.

Is there a reason why deleting subscriptions that don't exist is now a success and not an error? I couldn't find anything explicit on this in the spec. If there's a reason we're changing this we might want to document the reason in a comment somewhere.

I'll check the other half of the PR either later today or tomorrow when I have some more time.

[sthelen-enqs]

I've gone through the rest of the files. Except for a potential logic bug in RemoveForEntity functions the changes look good to me.

I added the comment to RemoveSubscriptionsForRemoteEntity, but the changes would need to be applied to all 4

[sthelen-enqs]

I just noticed this, but afaik we're trying to check if clientAddress == remoteEntity || serverAddress == remoteEntity, but are currently also allowing e.g. ClientAddress.Device == remoteDeviceAddress && ServerAddress.Entity == remoteEntityAddress

If we think of the resulting truth table [client device match, server device match, client entity match, server entity match], we're interested in [1, *, 1, *], and [*, 1, *, 1] but are currently also accepting [1, 0, 0, 1], and [0, 1, 1, 0]

I think we can use the following logic instead (even if it's not as pretty), unless the current logic is intended:

	// check if subscription matches ClientAddress or ServerAddress
	if (reflect.DeepEqual(subscription.ClientAddress.Device, remoteDeviceAddress) &&
		reflect.DeepEqual(subscription.ClientAddress.Entity, remoteEntityAddress)) ||
		(reflect.DeepEqual(subscription.ServerAddress.Device, remoteDeviceAddress) &&
			reflect.DeepEqual(subscription.ServerAddress.Entity, remoteEntityAddress)) {
		// process
	}

The same change would also need to be made in the binding_manager, and in Remove*ForLocalEntity

[DerAndereAndi]

Thanks for the great review and identifying this issue! This should be fixed with the latest commit.

[sthelen-enqs]

With the last few changes, I think this looks good now

[DerAndereAndi]

Thank you very much for the review @sthelen-enqs !