Skip to content

Rsa core redesign#98

Merged
amichair merged 5 commits intoapache:masterfrom
amichair:rsa-core-redesign
May 8, 2026
Merged

Rsa core redesign#98
amichair merged 5 commits intoapache:masterfrom
amichair:rsa-core-redesign

Conversation

@amichair
Copy link
Copy Markdown
Contributor

@amichair amichair commented May 1, 2026

Redesign of the RSA core module's import and export registration implementations as well as the service factory and resource management - to untagle the old convoluted design and make it more simple and intuitive, while fixing multiple incompatibilities with the spec that are now more straightforward to implement.

The details are in the JIRA tickets.

Comment thread rsa/src/main/java/org/apache/aries/rsa/core/ExportRegistrationImpl.java Outdated
@amichair
Copy link
Copy Markdown
Contributor Author

amichair commented May 6, 2026

Regarding direct access to the Shared class fields:

  • This is a private class, so encapsulation via getters/setters is moot - it's all internal implementation details anyway, so doesn't conflict with OOP design principals. This is similar to a class accessing its own private fields directly and not via accessors, which is how the majority of classes are written in OOP (I made up that statistic, but stand by it :-) )

  • This is very common even in the Java platform itself - nearly all of the collections we know and love access their inner class fields directly, along with many utilities: HashMap, TreeMap, LinkedList, Hashtable, regex.Pattern, ThreadPoolExecutor and many more (these are just the first ones that came to mind). If it's good enough for Java it's good enough for us ;-)

  • Adding getters+setters to all the fields would add 30-40 sloc to the class without a clear benefit - all else being equal, I would prefer less code to do the same functionality.

Is any of this convincing?

@alien11689
Copy link
Copy Markdown
Contributor

I know that the class is private, I am not blocking it. Maybe I am too focused on business applications or I use lombok so the number of lines is not growing

@amichair amichair force-pushed the rsa-core-redesign branch from 5c7f5e3 to 946d559 Compare May 7, 2026 06:29
@amichair
Copy link
Copy Markdown
Contributor Author

amichair commented May 7, 2026

I rebased on current master and renamed the methods. Other than the encapsulation thing, are there any other issues? Are the rest of the design/impl changes good?

Copy link
Copy Markdown
Contributor

@alien11689 alien11689 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beside the encapsulation rules it's ok so approved

@alien11689
Copy link
Copy Markdown
Contributor

Feel free to merge but please squash the commits or add jira id to the latest commit message

@amichair amichair force-pushed the rsa-core-redesign branch from 946d559 to 9c0479a Compare May 7, 2026 20:32
@amichair
Copy link
Copy Markdown
Contributor Author

amichair commented May 7, 2026

If you meant the first commit then I added a JIRA for it. Not sure what exactly to say about it in the ticket, it's just routine maintenance refactoring within the class, but also not directly related to the changes in the other commits so doesn't feel right to squash them together.

@alien11689
Copy link
Copy Markdown
Contributor

Let's merge then

@amichair amichair merged commit d13e90d into apache:master May 8, 2026
4 checks passed
@amichair amichair deleted the rsa-core-redesign branch May 8, 2026 13:28
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

Successfully merging this pull request may close these issues.

2 participants