Use ISapFunctionMetadata in InstallGenericServerFunctionHandler

[campersau]

Since we now have the APIs for getting metadata, the InstallGenericServerFunctionHandler could expose the callback for handling metadata. Which gives the user more control by allowing the use of different connections for retrieving metadata and connection pooling, which I have highlighted in the readme example.

This also exposes the metadata APIs on the SapPooledConnection.

Should we deprecate the old API? Old API is removed.

[campersau]

Looks like VS can't handle this correctly see dotnet/roslyn#23391

[huysentruitw]

I'd like to keep classes closed. In my book, internal properties like this create unwanted coupling. Would there be an other way to solve what you're trying to do?

[campersau]

Hm, not without exposing the _functionDescHandle in the public interface or some other kind of internal coupling.

[huysentruitw]

If the changes in this PR are solely to remove some duplication here, I would not merge it.

But I understand that it is to avoid opening connections each time? I wonder, do we have to get the metadata from the connection each time the server requests it, or could we get the metadata once and return that everytime the server asks for it. What would be the downside of this? (I never used server callbacks before, so please bear with me 😅)

[campersau]

This PR exposes the getFunctionMetadata to the user, so they have more control over retrieving the metadata. So they can:

• Handle exceptions on their own.
• Reuse SapConnections.
• Keep control over the SapConnectionParameters.
• Use different connections depending on functionName and SapAttributes. Which might be needed if there are multiple SapServer instances connected to different SAP systems. Since InstallGenericServerFunctionHandler is static after all.

[huysentruitw]

I missed the fact that InstallGenericServerFunctionHandler was static 😅 (I have no experience with SapServer myself)

So the idea is that InstallGenericServerFunctionHandler is only called once in an application? I don't quite understand how you could have different SapServers with different connection parameters, but would then only call InstallGenericServerFunctionHandler once and also have to pass some connection parameters. Shouldn't the SapServer class register a server listener itself and encapsulate it and have a much more user friendly API for handling different function callbacks?

[campersau]

Yes, InstallGenericServerFunctionHandler is only called once in an application.

Another way is to register server functions for specific sysIds:

DECL_EXP RFC_RC SAP_API RfcInstallServerFunction(SAP_UC const *sysId, RFC_FUNCTION_DESC_HANDLE funcDescHandle, RFC_SERVER_FUNCTION serverFunction, RFC_ERROR_INFO* errorInfo);

But with that you also can't tie it to a single SapServer either, because you could have multiple SapServers for the same sysId but with different PROGRAM_IDs... (Which is the use case I use at work.)

---

I don't quite understand how you could have different SapServers with different connection parameters, but would then only call InstallGenericServerFunctionHandler once and also have to pass some connection parameters.

That is exactly what this PR is solving by adding:

public static void InstallGenericServerFunctionHandler(Func<string, SapAttributes, ISapFunctionMetadata> getFunctionMetadata, Action<ISapServerConnection, ISapServerFunction> action)

I just have kept the previous methods for backward compatibility, as the ISapFunctionMetadata wasn't available when I first added them which is why I did the hack with the SapConnectionParameters 😞 :

public static void InstallGenericServerFunctionHandler(string connectionString, Action<ISapServerConnection, ISapServerFunction> action)

public static void InstallGenericServerFunctionHandler(SapConnectionParameters parameters, Action<ISapServerConnection, ISapServerFunction> action)

Should we deprecate the old API?

---

Shouldn't the SapServer class register a server listener itself and encapsulate it and have a much more user friendly API for handling different function callbacks?

Unfortunately there isn't an API to register a server function for a specific SAP server instance...
Maybe we could do it ourselves by using SYSID and PROGRAM_ID as an identifier but I am not sure if that works.

[huysentruitw]

From what I understand from the sapnwrfc.h file (which is my only documentation at this point):

RfcCreateServer simplifies the process of managing server connections ourselves by calling RfcRegisterServer internally. But this function returns RFC_SERVER_HANDLE instead of a server connection, which seems logic as that implementation handles reconnects itself when needed.

Now, in the callback (RFC_SERVER_FUNCTION) we get the RFC_CONNECTION_HANDLE):

typedef RFC_RC (SAP_API* RFC_SERVER_FUNCTION)(RFC_CONNECTION_HANDLE rfcHandle, RFC_FUNCTION_HANDLE funcHandle, RFC_ERROR_INFO* errorInfo);

Am I correct that, if we would register the server ourselves using RfcRegisterServer, which returns an RFC_CONNECTION_HANDLE, we would be able to match the server in the callback by that connection handle?

If that is the case, would it be possible to boil down a given RFC_CONNECTION_HANDLE to the RFC_SERVER_HANDLE ?

[campersau]

I am not sure that you can combine RfcCreateServer with RfcRegisterServer like that, because you would either need to pass the RFC_CONNECTION_HANDLE from RfcRegisterServer to the RFC_SERVER_HANDLE which I don't think you can or implement the connection / dispatch logic of RfcCreateServer yourself and don't use RfcCreateServer at all. Or maybe I couldn't follow your thoughts?

---

But I may have found another way this could work:

1. Add a RfcAddServerSessionChangedListener and keep track of the sessionIDs

DECL_EXP RFC_RC SAP_API RfcAddServerSessionChangedListener(RFC_SERVER_HANDLE serverHandle, RFC_SERVER_SESSION_CHANGE_LISTENER sessionChangeListener, RFC_ERROR_INFO* errorInfo);

typedef struct _RFC_SESSION_CHANGE {
	SAP_UC sessionID[30 + 1];	///< Session ID of the user session in question
	RFC_SESSION_EVENT event;	///< What has been done with that session
} RFC_SESSION_CHANGE;

2. Call RfcGetServerContext inside the function handler to get the sessionID.

DECL_EXP RFC_RC SAP_API RfcGetServerContext(RFC_CONNECTION_HANDLE rfcHandle, RFC_SERVER_CONTEXT* context, RFC_ERROR_INFO* errorInfo);

typedef struct _RFC_SERVER_CONTEXT{
	RFC_CALL_TYPE type;						///< Specifies the type of function call. Depending on the value of this field, some of the other fields of this struct may be filled.
	RFC_TID tid;							///< If type is RFC_TRANSACTIONAL or RFC_QUEUED, this field is filled with the 24 digit TID of the tRFC/qRFC unit.
	RFC_UNIT_IDENTIFIER* unitIdentifier;	///< If type is RFC_BACKGROUND_UNIT, this pointer is set to the unit identifier of the LUW. Note: the pointer is valid only during the execution context of your server function.
	RFC_UNIT_ATTRIBUTES* unitAttributes;	///< If type is RFC_BACKGROUND_UNIT, this pointer is set to the unit attributes of the LUW. Note: the pointer is valid only during the execution context of your server function.
	unsigned isStateful;					///< Specifies whether the current server connection is processing stateful RFC requests (assigned permanently to one fixed ABAP user session).
	SAP_UC sessionID[33];					///< Contains a unique zero-terminated session ID, identifying the ABAP or external user session. Can be used in stateful servers to store session context in a hashmap.
}RFC_SERVER_CONTEXT;

3. Find the SapServer which has the same sessID and invoke an eventhandler / function there.

It looks like it requires stateful server though.

---

https://support.sap.com/en/product/connectors/nwrfcsdk.html there is a SAP NetWeaver RFC SDK ProgrammingGuide at the bottom which has some examples.

[campersau]

It looks like we can bind to SapServer instances with RfcAddServerSessionChangedListener and RfcGetServerContext see 3901a81

A generic metadata handler is still needed though.

[huysentruitw]

Excuse me for the large delay. I'm no longer working on this project for which I wrote this library, and I no longer have access to an SAP server. Can you give me an update about this PR, do you think it is till in good shape to be merged?

[campersau]

@huysentruitw I have rebased it based on #70 should be good to go now.

@tom-j-irvine since you are using the SapServer it would be great if you could try out this PR.

[tom-j-irvine]

Yes, I will give it a try. Hopefully I can report back in a day or two. Thanks a lot for working on this - this project has been very helpful.

[campersau]

@tom-j-irvine Thanks! The setup is a little different now see https://github.com/campersau/SapNwRfc-1/blob/sapserverfunctionmetadata/README.md#rfc-server-generic-handler
As it should now be possible to use multiple SapServers with multiple SAP systems. The readme currently just demonstrates the simplest case though.

[tom-j-irvine]

Everything seems to work well in my project. I stopped (and fully disposed) the server multiple times, restarted and never had the GC issue creep back in. If nothing else, the API seems a little more intuitive now too.

One minor enhancement I can think of would be to add a ServerState property to the ISapServer that could be updated by the StateChange event. That would allow us check the state and not have to track it externally based on the event. By no means is this necessary, but would be a nice-to-have item.

[campersau]

@tom-j-irvine Thanks for testing!

Regarding the feedback to the ServerState property: There is actually a method called RfcGetServerAttributes which also returns the server state and some additional info. I would prefer to expose this method on ISapServer instead.

/** \struct _RFC_SERVER_ATTRIBUTES
* \ingroup autoserver
*
* Information about an RFC Server returned by RfcGetServerAttributes(). 
*/
typedef struct _RFC_SERVER_ATTRIBUTES{
	SAP_UC* serverName;			///< This server's name as given when creating the server.
	RFC_PROTOCOL_TYPE type;		///< This RFC server's type. Will be one of RFC_MULTI_COUNT_REGISTERED_SERVER or RFC_TCP_SOCKET_SERVER.
	unsigned registrationCount;	///< The current number of active registrations (in case of a Registered Server) or the maximum number of parallel connections the server will accept (in case of a TCP Socket Server).
	RFC_SERVER_STATE state;		///< This server's state.
	unsigned currentBusyCount;	///< The number of requests currently being processed.
	unsigned peakBusyCount;		///< The maximum number of requests the server has been processing in parallel since it has been created.
} RFC_SERVER_ATTRIBUTES;

DECL_EXP RFC_RC SAP_API RfcGetServerAttributes(RFC_SERVER_HANDLE serverHandle, RFC_SERVER_ATTRIBUTES* serverAttributes, RFC_ERROR_INFO* errorInfo);

[tom-j-irvine]

Yeah, asking the server itself sounds a lot better than trying to keep track of it in code.

[campersau]

I have implemented RfcGetServerAttributes here c60998d

[tom-j-irvine]

Works great. Thank you!