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

Introduce Resource Sharing Policy Management Component #416

Open
wants to merge 51 commits into
base: main
Choose a base branch
from

Conversation

BimsaraBodaragama
Copy link

@BimsaraBodaragama BimsaraBodaragama commented Nov 25, 2024

This PR adds a new component to manage resource-sharing policies between organizations in the WSO2 Identity Server. It enables defining, storing, and retrieving resource-sharing policies for users, applications, and identity providers.

Purpose

This feature addresses the need for a structured and scalable solution to manage resource-sharing policies across organizational hierarchies. It resolves the following issues:

Goals

  • Provide a unified interface for managing resource-sharing policies.
  • Enable consistent policy definitions across resource types.
  • Support dynamic and hierarchical sharing options, such as sharing with all current and future organizations or selected organizational hierarchies.

Approach

  • Created a new component for resource-sharing policy management.
  • Developed a core service with DAO and utility classes for CRUD operations.
  • Defined constants and enums for resource types and sharing policies.
  • Implemented robust exception handling mechanisms.

Release note

Introduced a centralized resource-sharing policy management service to unify and streamline sharing policies across all resource types.

Documentation

N/A (Documentation updates will be tracked separately.)

Test environment

  • JDK 11
  • macOS 15.1.1 (24B91)
  • H2 Database

Related Issue

Introduce a Centralized Resource Sharing Policy Management Service #21815

…licy record and shared resource attributes by resource sharing policy id
… attributes rather than deleting it from record id since that way of deletion is not currently needed.
… attributes rather than deleting it from record id since that way of deletion is not currently needed - Part 2.
@BimsaraBodaragama BimsaraBodaragama self-assigned this Nov 25, 2024
Copy link

codecov bot commented Nov 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.57%. Comparing base (021650f) to head (c30c9d5).

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #416      +/-   ##
============================================
+ Coverage     47.13%   48.57%   +1.44%     
- Complexity     1024     1080      +56     
============================================
  Files           112      115       +3     
  Lines          6710     6898     +188     
  Branches        802      810       +8     
============================================
+ Hits           3163     3351     +188     
  Misses         3259     3259              
  Partials        288      288              
Flag Coverage Δ
unit 32.18% <100.00%> (+3.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pom.xml Show resolved Hide resolved
* @throws ResourceSharingPolicyMgtException If an error occurs during the creation of the resource sharing policy.
*/
int addResourceSharingPolicy(ResourceSharingPolicy resourceSharingPolicy)
throws ResourceSharingPolicyMgtException, DataAccessException;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
throws ResourceSharingPolicyMgtException, DataAccessException;
throws ResourceSharingPolicyMgtException;

* @return {@code true} if the resource sharing policy was deleted successfully, {@code false} otherwise.
* @throws ResourceSharingPolicyMgtException If an error occurs while deleting the resource sharing policy.
*/
boolean deleteResourceSharingPolicyRecordById(int resourceSharingPolicyId, String sharingPolicyInitiatedOrgId)
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we make this a void method?
Otherwise invoker needs to handle false / exception cases specifically or we need to mention in which scenario we return false, in which scenarios it will throw an exception

Copy link
Contributor

Choose a reason for hiding this comment

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

Fix in all other delete operations

*/
boolean addResourceSharingPolicyWithAttributes(ResourceSharingPolicy resourceSharingPolicy,
List<SharedResourceAttribute> sharedResourceAttributes,
String sharingPolicyInitiatedOrgId)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need sharingPolicyInitiatedOrgId as a separate param? Shall we extract it from resourceSharingPolicy
for impl usages

*/
public class ResourceSharingPolicyHandlerServiceImpl implements ResourceSharingPolicyHandlerService {

private static final Log LOG = LogFactory.getLog(ResourceSharingPolicyHandlerService.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private static final Log LOG = LogFactory.getLog(ResourceSharingPolicyHandlerService.class);
private static final Log LOG = LogFactory.getLog(ResourceSharingPolicyHandlerServiceImpl.class);

* @throws ResourceSharingPolicyMgtException If an error occurs while retrieving the resource sharing
* policies or shared attributes.
*/
Optional<Map<String, Map<ResourceSharingPolicy, List<SharedResourceAttribute>>>>
Copy link
Contributor

Choose a reason for hiding this comment

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

If the service will anyway construct a Map, not need to make it Optional.
Check other service which return Map

Comment on lines +262 to +263
private boolean isInvalidId(String id) {
return id == null || id.isEmpty();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private boolean isInvalidId(String id) {
return id == null || id.isEmpty();
private boolean isInvalidId(String id) {
return id == null || id.isEmpty();

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.

4 participants