go through objectlink-core-cpp

[jryannel]

General impression is that the design is a bit knotty.

• [x] split into 2 issues protocol part and the template part - change protocol minor version, change the version which we use in template to that one

Client/ Sink side General rule:

• Object Sink should not know about the Olink client
• the registry should inform the outside world about changes - nodes added or removed
• registry is purely about data
• the client node is about behavior
  • [x] general classes description improvements: the class purpose, method purpose
  • [x] split to to separate files, classes from cpp too, they can be closed in deatil folder/namespace if necesary, but would be easier for me to look for each class in its own file.
  • [x] clean up objects interfaces - move some functions to private, some functions not implemented, some functions unused.
  • [x] store ClientRegistry with&
  • [x] store WebSocket with smart pointer
  • [x] Shut down scenarios for Client side 1) disconnect scenarios on client side- OK 2) object sink death scenario - missing info to olinkclient to clean up in m_linked objects (C++ template) 3) OlinkClient destroy - missing info to objectsink that client node is no longer valid, also both OliinkClient and Client node make same cleanup in registry, use ObjectSink.olinkOnRelease (C++ template)
  • [x] 4) add unlink node scenario - -I can't see such scenario, the client says unlink (called by OlinkClient), the call does nothing else but sending message. The Server side does not reply anything-
  • [x] Remove storing m_linkedObjects in OlinkClient -> should be moved to registry . The OlinkClient does not need registry. (C++ template)
  • [x] ClientRegistry go through adding/removing record scenarios, check if it is symmetric

• [x] check if ObjectSink needs OlinkClient general rule if anything needs an "ugly" code, that should NOT be the object-link-core-cpp which is used by many parties

Server/source side

• [x] general classes description improvements.

• [x] add description for olinkHOst that RequestHandlerFactory is owned by POCO server, and similar that OlinkWebSocketHandler will be deleted by it as soon as the request will be handled.

• [x] split to to separate files, classes from cpp too, they can be closed in deatil folder/namespace if necesary, but would be easier for me to look for each class in its own file. An exception might be the two POCO iheriting classes, but factory for now is used in more than one file

• [x] clean up objects interfaces - move some functions to private, some functions not implemented, some functions unused.

• [x] change use of raw pointers and new/delete to unique_ptrs (OlinkRemote, Poco::Net::WebSocket, HTTPServer)

• [x] HTTPRequestHandlerFactory can be stored at least with Poco::SharedPTr - this is what is taken by HTTPServer. If server manages it, it is not safe for other classes to keep raw ptr, we won't get notified when the factory will be deleted (although not very likely)

• [x] std::set<OLinkRemote*> could probably be std::vector, and RequestHandlerFactory should not be a place to keep it - but don't know yet where to? OlinkHost? Registry? something new?

• [x] RemoteRegistry should be kept with &

• [x] Shut down scenarios for Server side 1) olinkHost close() - missing info to object source that remote node is no longer valid - that is ok (after fix) we don't care there is no client 2) object source death - we're cleaning the registry, we don't send any message through the internet that the object is no longer available, is that ok? TODO - add a task that we should introduce error handling for protocol with this scenario (also client side), also scenarios with version mismatch (both protocol and api) maybe add handshake 3) unlink message - remove the node from record in registry and setting node to null in sink - assuming we have more nodes we shouldn't - OK 4) not sure if possible, close frame received in socket - no other close action earlier - object source is not informed that remote node is no longer valid -OK

• [x] I am not sure if I get architecture of nodes for server side. The registry keeps <name, source, collection> but object source stores only one node. Assuming Source talks with all of the nodes it should write to all of them. Maybe the idea was that object source doesn't keep any, and takes each time all the "subscribers" from registry? It only need to know about one certain node when the client is calling a function that returns a result - only one needs to be notified about that. TODO each property or signal should be broadcasted to all the registered nodes for the source for the methods - reply only to caller node

• [x] Poco::Net::HTTPRequestHandler* createRequestHandler - handle fail case when we cannot upgrade

• return sth? what POCO expects, what happens if we return null

• log there might be a http response there sth like 426 Upgrade Required - HTTP | MDN

Protocol extracted to separate task https://github.com/apigear-io/objectlink-core-cpp/issues/15

• Protocol should have only static members.
• Method getMessageType
• Methods decodeXMessage and tand return proper data in structure.
• The switch-case would go to baseNode then, no need for the IProtocolListner

Remarks from #9 issue

• [x] REMARK OLINK uniqueness of the name, overwrite sink in registry. See the remote_interface.h for comments - don't allow TODO
• add documentation - in form of function description
• log
• return some kind of error?
• [x] REMARK OLINK - clean up setting up connection and disconnecting - they are not symmetrically placed in classes

[dorotaphanSiili]

olinkClientMermaid.txt OlinkServerMermaid.txt sequence_diagram_client.txt sequence_diagram_server.txt

[dorotaphanSiili]

Check thread safety and race condition eg. same service, two requests of change each in different thread (different client)