#81 OIDMap and Extensions should uses Class objects, not strings
Closed: migrated 3 years ago by dmoluguw. Opened 12 years ago by admiyo.

OIDMap is a class that is supposed to map OIDs to the Java classes that implement them. Speicfically, it is used for extensions. THe classes are specified by strings. If these classes are refactored, the strings will no longer point to real classes, and the mechanism will be broken. Instead of Map<String, String>, the map should be Map<OID, Class>


While trying to clean up the type safety issues around the Extension API, I came across an issue, and made a patch on how I think it should be fixed. This is not a rewrite solution, but merely an incremental. Ade has some legitimate reservations on the patch, and so it has been pocket veto-ed for now.

the approach I suggested is in keeping with how I see Java being maintained, but I realize that it is not super obvious, so I would like to lay out the reasons for the approach.

Much of the Extension API builds from the OIDMap. Regardless of the many functions this class performs, the most important thing it does and rthe reason it exists is to provide a mapping from OIDs to the Classes that implement them. This is used predominantly for the Extension API. It loads the list from a properties files, and so the OID and the classname are supplied as strings.

This is probably the first thing that should be cleaned up. The classes should remain as strings for a very short period of time, probably only during load. As soon as possible, the code should validate that they are indeed loadable classes. Thus, during load, each should be proceesed with Class.forName. THe map can then hold on to the Class instance instead of the String. This will have the benefit of letting the end user know that the Classes are all valid, or throw and exception at Application start up time if one is invalid.

When a class is registered with OIDMap, it should also be evaluated immediately. There is a Class that registeres an invalid string with OIDMap. However, internal, registration should be done with classes, not with Strings any way.

The OIDKMap is a 3 way map. It maps OIDs to classes, OIDs to names, and names to classes. I am not sure what the origianl need for the Name string was, other than a convinient way to reference the Extension. They are not part of the RFC, so the Strings in the Names are not fixed. They are used to instantiate Extensions. THe Names are, for tha majority of the Extensions, static instance variables. This makes them awkward to use, as there is no way to get an instance variable from java.lang.Class other than the reflection API, which is not type safe.

This is a code refactoring which doesn't affect the user.

Metadata Update from @admiyo:
- Issue assigned to edewata
- Issue set to the milestone: UNTRIAGED

7 years ago

Dogtag PKI is moving from Pagure issues to GitHub issues. This means that existing or new
issues will be reported and tracked through Dogtag PKI's GitHub Issue tracker.

This issue has been cloned to GitHub and is available here:
https://github.com/dogtagpki/pki/issues/653

If you want to receive further updates on the issue, please navigate to the
GitHub issue and click on Subscribe button.

Thank you for understanding, and we apologize for any inconvenience.

Metadata Update from @dmoluguw:
- Issue close_status updated to: migrated
- Issue status updated to: Closed (was: Open)

3 years ago

Login to comment on this ticket.

Metadata