[Accepted] SDL 0234 Revisions - Proxy Library RPC Generation

[theresalech]

Hello SDL community,

The review of "Revise SDL-0234 - Proxy Library RPC Generation proposal - Keyword Avoidance" begins now and runs through April 7, 2020.

This will be a review of proposed revisions to a previously accepted but not yet implemented proposal, SDL 0234.

The pull request outlining the revisions under review is available here:

#986

Reviews are an important part of the SDL evolution process. All reviews should be sent to the associated Github issue at:

#990

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of SDL. When writing your review, here are some questions you might want to answer in your review:

• Is the problem being addressed significant enough to warrant a change to SDL?
• Does this proposal fit well with the feel and direction of SDL?
• If you have used competitors with a similar feature, how do you feel that this proposal compares to those?
• How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
  Please state explicitly whether you believe that the proposal should be accepted into SDL.

More information about the SDL evolution process is available at

https://github.com/smartdevicelink/sdl_evolution/blob/master/process.md

Thank you,
Theresa Lech

Program Manager - Livio
theresa@livio.io

[vladmu]

Hello everyone,

From the motivation of the proposed revision #986 (comment), it is clear that mostly the author said about the old RPCs problems first. And the proposed keywords mechanism really is just one of the possible rules of the generator customization. At the same time, we remember the previous revision discussion where the committee decided to do not include customizations into the generator, and the next point was added into the proposal:

14. The generator script should only be used for newly created RPCs. Existing RPC classes should not be overwritten by generated code because the existing code may be slightly different than the generated one.

Also taking a look the list of proposed keywords we could see that in most cases, the keywords are platform-specific.

For the same reasons, we can't agree that including the new point 15 into the current proposal is a good idea. This enables customizations in generators and adds additional efforts to development and complexity to maintain the generator in the future.

At the same time, we agree that the list of keywords could be required as the guideline for creating new RPCs not to use those keywords in naming.
In our opinion, it would good to create CONTRIBUTING.md in https://github.com/smartdevicelink/rpc_spec repository and include those keywords as rules for RPCs contributors.

[joeygrover]

Thank you for your feedback! The main goal of this alteration is to provide parity between the client libraries which is very important. Since these rules are standard between all of the the client libraries this is actually not a "customization" like the previously removed feature. These are rules for generating the API for all the libraries. The keywords are curated from all libraries and therefore are not library independent. Again, this is because the only way to ensure parity between library APIs is to factor in all of the keywords used between all of the libraries.

Since the JavaScript Suite library is about to be released we want to make sure that the APIs that are in place can be the same without creating a major change or having to create the customization rules again. The goal is to allow all libraries to simply use the generator outright and without this rule it will never be a possibility without adding customizations per library. Therefore adding this feature from the start that provides parity between libraries there is still a chance all libraries could use the generator in the future. I think having rules in the future for the actual repo and spec will be helpful (especially if we can introduce some continuous integration to check PRs), but it does not address the current RPCs and therefore does not solve the problem this addition will solve.

[vladmu]

@joeygrover thank you for the explanation, but we still don't see how the keywords on the generator level could provide parity between the client libraries? Moreover, this could lead to unexpected results from the RPC's contributor and client's developer perspective, when they expect the defined name in XML and in all generated RPC classes of all client libraries, but instead gets the names with unexpected prefixes or suffixes.

Let's consider the CancelInteraction as a new function that the RPC contributor created and used the wrong functionID param name. According to the proposed solution, it will be transformed into FunctionIDParam on the generation stage, but the XML stays the same and has the wrong functionID value in the specification. Then the application developer reads the RPC specification and tries to find that name in the required library and, if lucky, also sees confused FunctionIDParam name.

That's the imparity between specification and realization. So if the RPC contributor will change the wrong name functionID on the start to the correct targetFunctionID for example, all became ok from all perspectives.

Avoiding using those keywords in names on the RPC contribution stage could fix that imparity and provide parity not only between client libraries but also between libraries and the RPC specification itself.

[joeygrover]

Please see the �PR proposal it clearly states the actual value of what is sent stays inline with the RPC spec:

private static final KEY_FUNCTION_ID_PARAM = "functionID"; //This actual string value must remain the same as the rpc spec

The parity exists then for all APIs in the client libraries to remain the same, while the RPC spec values are unchanged.

Again this has to happen for more than just new PRCs, and therefore must be included.

[joeljfischer]

Let's consider the CancelInteraction as a new function that the RPC contributor created and used the wrong functionID param name. According to the proposed solution, it will be transformed into FunctionIDParam on the generation stage, but the XML stays the same and has the wrong functionID value in the specification. Then the application developer reads the RPC specification and tries to find that name in the required library and, if lucky, also sees confused FunctionIDParam name.

This is impossible to avoid at this point because it conflicts with one or more library's keywords. In other words, one or more libraries are incapable of creating a functionID parameter. The question now is, do we want the app libraries to all have the same parameter name, or should some fit the specification and other modify to be something else? I cannot stress enough that this situation is completely unavoidable. The only question is our reaction to it.

The PM is stressing that it is important that all the app libraries have the same parameter names in the various libraries, that's the only this this proposal does.

[vladmu]

@joeljfischer it is possible to avoid this, simply don't allow RPC contributor to use defined keywords in the new RPC names and all libraries will have parity between each other and the specification. How to do that, adding guidelines, Git hooks that could check the newly added or changed RPCs, some tools like linter by keywords, etc.? We understand the purposes and don't propose using different names for different libraries.

However, according to point 14: The generator script should only be used for newly created RPCs. Therefore from that perspective, point 15 has a conflict.

Because if only newly created RPCs covered by the generator, then we don't need keywords checking and transformation in the generator code. This case could be managed early on the XML level. Otherwise, for early created RPCs, point 14 says just don't use the generator.

The only question is, should the generator support the old RPCs, then we need to go back and consider customizations. In that case, the requirement all the app libraries have the same parameter names is the direct function of the generator, so point 14 should be removed or rephrased alongside the adding of new point 15.

If the purpose still is the newly created RPCs, then no changes in the code required. Only the RPC contribution process requires changes because currently, the generator provides names from the RPC spec AS-IS and they will be the same over libraries.

[joeljfischer]

We understand that, but in order to leave ourselves the future option of changing the iOS and Java Suite RPCs to use the generator scripts for old RPCs as well, we need this piece. Otherwise the JavaScript Suite would have to change it's currently generated RPCs unnecessarily.

In addition, even if there aren't "hard" conflicts with the current JavaScript Suite APIs, there are "soft" conflicts that create ambiguous APIs. For example, RPCs like CancelInteraction will have the following methods with no clear indication of what they do:

setFunctionName (name)
getFunctionName ()
setFunctionID (id)
getFunctionID ()

By adding the generic keyword avoidance mechanism, we leave open the possibility of using the RPC_Spec as is in the future, and we solve the current JavaScript Suite ambiguity between method names.

This doesn't prevent us from using Git hooks to fail PRs that add keyworded parameter names, and that's still a good idea, but I think we should have a backward-compatible change as well as a forward-compatible one.

[vladmu]

@joeljfischer agree, from that point of view it makes sense.

[theresalech]

A quorum was not present during the 2020-04-07 Steering Committee Meeting, so this proposal will remain in review until our meeting on 2020-04-14.

[theresalech]

The Steering Committee fully agreed to these revisions.

[theresalech]

Comments have been left on implementation issues to note the accepted revisions:

• iOS
• Java Suite
• JavaScript Suite
• RPC Spec