Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OIDMap and Extensions should uses Class objects, not strings #653

Open
pki-bot opened this issue Oct 2, 2020 · 3 comments
Open

OIDMap and Extensions should uses Class objects, not strings #653

pki-bot opened this issue Oct 2, 2020 · 3 comments
Milestone

Comments

@pki-bot
Copy link

pki-bot commented Oct 2, 2020

This issue was migrated from Pagure Issue #81. Originally filed by admiyo on 2011-12-21 19:44:51:


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>

@pki-bot
Copy link
Author

pki-bot commented Oct 2, 2020

Comment from admiyo at 2011-12-23 06:00:09

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.

@pki-bot pki-bot added this to the UNTRIAGED milestone Oct 2, 2020
@pki-bot
Copy link
Author

pki-bot commented Oct 2, 2020

Comment from edewata (@edewata) at 2016-11-30 05:20:58

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

@pki-bot
Copy link
Author

pki-bot commented Oct 2, 2020

Comment from admiyo at 2017-02-27 13:59:03

Metadata Update from @admiyo:

  • Issue assigned to edewata
  • Issue set to the milestone: UNTRIAGED

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant