Introduce Generic Organization Resource Resolver #415

dhaura · 2024-11-21T08:30:55Z

Purpose

This PR implements a generic resource resolver designed for B2B organization hierarchies. It enables the resolution of any given resource through the ancestor hierarchy of an organization.

…esource resolver.

…-resolver

codecov · 2024-11-21T08:49:52Z

Codecov Report

Attention: Patch coverage is 78.16092% with 19 lines in your changes missing coverage. Please review.

Project coverage is 47.29%. Comparing base (a557994) to head (d2107a1).
Report is 22 commits behind head on main.

Files with missing lines	Patch %	Lines
...ervice/strategy/FirstFoundAggregationStrategy.java	63.63%	4 Missing and 4 partials ⚠️
.../service/strategy/MergeAllAggregationStrategy.java	73.33%	4 Missing and 4 partials ⚠️
...service/util/OrgResourceHierarchyTraverseUtil.java	85.71%	0 Missing and 2 partials ⚠️
...averse/service/OrgResourceResolverServiceImpl.java	95.23%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #415      +/-   ##
============================================
+ Coverage     46.45%   47.29%   +0.83%     
- Complexity     1034     1065      +31     
============================================
  Files           113      117       +4     
  Lines          6806     6956     +150     
  Branches        821      849      +28     
============================================
+ Hits           3162     3290     +128     
- Misses         3353     3363      +10     
- Partials        291      303      +12

Flag	Coverage Δ
unit	`31.94% <78.16%> (+2.77%)`	⬆️

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.

.../identity/organization/resource/hierarchy/traverse/service/strategy/AggregationStrategy.java

...on/identity/organization/resource/hierarchy/traverse/service/OrgResourceResolverService.java

components/org.wso2.carbon.identity.organization.resource.hierarchy.traverse.service/pom.xml

omindu · 2024-11-21T09:59:54Z

...tity/organization/resource/hierarchy/traverse/service/exception/NotImplementedException.java

+/**
+ * The exception to throw when the code is not implemented.
+ */
+public class NotImplementedException extends RuntimeException {


Can't we use UnsupportedOperationException instead of this ?

NotImplementedException was used here since it is the usual practice when it comes to default methods in interfaces.

Examples:

https://github.com/wso2/carbon-kernel/blob/4.10.x/core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/NotImplementedException.java

https://github.com/wso2/carbon-kernel/blob/41af87519e8f65e0efcfd3ce9c33cfb51f839e0d/core/org.wso2.carbon.user.core/src/main/java/org/wso2/carbon/user/core/UniqueIDUserStoreManager.java#L169-L173

https://github.com/wso2/identity-organization-management-core/blob/main/components/org.wso2.carbon.identity.organization.management.service/src/main/java/org/wso2/carbon/identity/organization/management/service/exception/NotImplementedException.java

identity-organization-management/components/org.wso2.carbon.identity.organization.management.application/src/main/java/org/wso2/carbon/identity/organization/management/application/OrgApplicationManager.java

Lines 159 to 164 in 021650f

default Map<String, String> getAncestorAppIds(String childAppId, String childOrgId)

throws OrganizationManagementException {

throw new NotImplementedException(

"getAncestorAppIds method is not implemented in " + this.getClass().getName());

}

UnsupportedOperationException has been mainly used in following places.

deprecated methods [1]

to specify that the functionality is not supported at all [2]

in branching logic (if/else) where we don't support the method for different branches.

[1] - https://github.com/wso2/carbon-kernel/blob/41af87519e8f65e0efcfd3ce9c33cfb51f839e0d/core/org.wso2.carbon.registry.core/src/main/java/org/wso2/carbon/registry/core/config/StaticConfiguration.java#L124-L133
[2] - https://github.com/wso2/carbon-kernel/blob/41af87519e8f65e0efcfd3ce9c33cfb51f839e0d/core/org.wso2.carbon.registry.core/src/main/java/org/wso2/carbon/registry/core/jdbc/utils/RegistryDataSource.java#L120-L123
[3] - https://github.com/wso2/carbon-identity-framework/blob/091a62a931a4a65f270cd3253136ba288be2c7a9/components/application-mgt/org.wso2.carbon.identity.application.mgt/src/main/java/org/wso2/carbon/identity/application/mgt/ApplicationManagementServiceImpl.java#L426

omindu · 2024-11-21T10:43:22Z

.../identity/organization/resource/hierarchy/traverse/service/strategy/AggregationStrategy.java

+     * @return Aggregated resource.
+     * @throws OrgResourceHierarchyTraverseException If an error occurs while aggregating the resources.
+     */
+    default T aggregate(List<String> organizationHierarchy, Function<String, Optional<T>> resourceRetriever)


Why do we have these as default ?

They are specified as default to simplify the process of creating a new aggregation strategy, particularly when the strategy only requires implementing a single method.

Ex: If there's a resource type which is available at the org-level but not at the app-level, there is no need to implement the application specific aggregate method.

...dentity/organization/resource/hierarchy/traverse/service/OrgResourceResolverServiceImpl.java

…ate method.

omindu · 2024-11-21T15:06:05Z

...dentity/organization/resource/hierarchy/traverse/service/OrgResourceResolverServiceImpl.java

+/**
+ * Implementation of the OrgResourceResolverService.
+ */
+public class OrgResourceResolverServiceImpl implements OrgResourceResolverService {


Can we introduce OrganizationManager as a constructor ? This will make writing unit tests easy.

Our usual practice with OSGI services is to store all the required services/ managers by the component in the data holder of the component.

…viceTest.

Copilot reviewed 6 out of 20 changed files in this pull request and generated no suggestions.

Files not reviewed (14)

components/org.wso2.carbon.identity.organization.resource.hierarchy.traverse.service/pom.xml: Language not supported
components/org.wso2.carbon.identity.organization.resource.hierarchy.traverse.service/src/test/resources/testng.xml: Language not supported
features/org.wso2.carbon.identity.organization.management.server.feature/pom.xml: Language not supported
pom.xml: Language not supported
components/org.wso2.carbon.identity.organization.resource.hierarchy.traverse.service/src/main/java/org/wso2/carbon/identity/organization/resource/hierarchy/traverse/service/strategy/AggregationStrategy.java: Evaluated as low risk
components/org.wso2.carbon.identity.organization.resource.hierarchy.traverse.service/src/main/java/org/wso2/carbon/identity/organization/resource/hierarchy/traverse/service/internal/OrgResourceHierarchyTraverseServiceDataHolder.java: Evaluated as low risk
components/org.wso2.carbon.identity.organization.resource.hierarchy.traverse.service/src/main/java/org/wso2/carbon/identity/organization/resource/hierarchy/traverse/service/exception/OrgResourceHierarchyTraverseException.java: Evaluated as low risk
components/org.wso2.carbon.identity.organization.resource.hierarchy.traverse.service/src/main/java/org/wso2/carbon/identity/organization/resource/hierarchy/traverse/service/exception/NotImplementedException.java: Evaluated as low risk
components/org.wso2.carbon.identity.organization.resource.hierarchy.traverse.service/src/main/java/org/wso2/carbon/identity/organization/resource/hierarchy/traverse/service/exception/OrgResourceHierarchyTraverseClientException.java: Evaluated as low risk
components/org.wso2.carbon.identity.organization.resource.hierarchy.traverse.service/src/main/java/org/wso2/carbon/identity/organization/resource/hierarchy/traverse/service/OrgResourceResolverServiceImpl.java: Evaluated as low risk
components/org.wso2.carbon.identity.organization.resource.hierarchy.traverse.service/src/main/java/org/wso2/carbon/identity/organization/resource/hierarchy/traverse/service/internal/OrgResourceHierarchyTraverseServiceComponent.java: Evaluated as low risk
components/org.wso2.carbon.identity.organization.resource.hierarchy.traverse.service/src/main/java/org/wso2/carbon/identity/organization/resource/hierarchy/traverse/service/util/OrgResourceHierarchyTraverseUtil.java: Evaluated as low risk
components/org.wso2.carbon.identity.organization.resource.hierarchy.traverse.service/src/main/java/org/wso2/carbon/identity/organization/resource/hierarchy/traverse/service/strategy/MergeAllAggregationStrategy.java: Evaluated as low risk
components/org.wso2.carbon.identity.organization.resource.hierarchy.traverse.service/src/test/java/org/wso2/carbon/identity/organization/resource/hierarchy/traverse/service/mock/resource/impl/model/MockResource.java: Evaluated as low risk

...dentity/organization/resource/hierarchy/traverse/service/OrgResourceResolverServiceImpl.java

...tity/organization/resource/hierarchy/traverse/service/exception/NotImplementedException.java

...source/hierarchy/traverse/service/exception/OrgResourceHierarchyTraverseClientException.java

...ion/resource/hierarchy/traverse/service/exception/OrgResourceHierarchyTraverseException.java

...source/hierarchy/traverse/service/internal/OrgResourceHierarchyTraverseServiceComponent.java

...ource/hierarchy/traverse/service/internal/OrgResourceHierarchyTraverseServiceDataHolder.java

...organization/resource/hierarchy/traverse/service/strategy/FirstFoundAggregationStrategy.java

...ource/hierarchy/traverse/service/internal/OrgResourceHierarchyTraverseServiceDataHolder.java

jenkins-is-staging · 2024-11-29T13:28:50Z

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/12085608306

...dentity/organization/resource/hierarchy/traverse/service/OrgResourceResolverServiceTest.java

jenkins-is-staging · 2024-11-29T15:21:07Z

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/12085608306
Status: success

jenkins-is-staging

Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/12085608306

…to DP-add-b2b-resource-resolver

jenkins-is-staging · 2024-12-04T05:41:43Z

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/12153757493

jenkins-is-staging · 2024-12-04T07:35:27Z

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/12153757493
Status: success

jenkins-is-staging

Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/12153757493

dhaura added 2 commits November 21, 2024 13:59

Introduce organization resource hierarchy traverse service with the r…

a18f28c

…esource resolver.

Fix traverse service pom version.

6cbaa3d

dhaura mentioned this pull request Nov 21, 2024

Introduce B2B Organization Resource Resolver wso2/carbon-identity-framework#6139

Closed

Merge remote-tracking branch 'upstream/main' into DP-add-b2b-resource…

941a2ae

…-resolver

omindu reviewed Nov 21, 2024

View reviewed changes

.../identity/organization/resource/hierarchy/traverse/service/strategy/AggregationStrategy.java Show resolved Hide resolved

omindu reviewed Nov 21, 2024

View reviewed changes

...on/identity/organization/resource/hierarchy/traverse/service/OrgResourceResolverService.java Show resolved Hide resolved

darshanasbg reviewed Nov 21, 2024

View reviewed changes

components/org.wso2.carbon.identity.organization.resource.hierarchy.traverse.service/pom.xml Outdated Show resolved Hide resolved

omindu reviewed Nov 21, 2024

View reviewed changes

...dentity/organization/resource/hierarchy/traverse/service/OrgResourceResolverServiceImpl.java Outdated Show resolved Hide resolved

Improve bundle packaging and extract ancestor org retrieval to a priv…

28341aa

…ate method.

omindu reviewed Nov 21, 2024

View reviewed changes

dhaura added 8 commits November 22, 2024 12:15

Add unit tests and improve javadoc comments.

e9667be

Add unit test comments.

fc14805

Improve OrgResourceResolverService comments.

1577064

Fix checkstyle issues and rename test class to OrgResourceResolverSer…

1d653e0

…viceTest.

Apply test name change in testng.xml.

22415d3

Add missing fullstops in comments.

b71a3cc

Improve assertResolvedResponse method.

4c93167

Improve NotImplementedException comments.

7f071c1

dhaura mentioned this pull request Nov 26, 2024

Introduce Resource Sharing Policy Management Component #416

Open

Handle invalid organization ids as error scenarios.

c145e9f

darshanasbg requested a review from Copilot November 29, 2024 04:45

Copilot AI reviewed Nov 29, 2024

View reviewed changes

AnuradhaSK reviewed Nov 29, 2024

View reviewed changes

...dentity/organization/resource/hierarchy/traverse/service/OrgResourceResolverServiceImpl.java Outdated Show resolved Hide resolved

AnuradhaSK reviewed Nov 29, 2024

View reviewed changes

...tity/organization/resource/hierarchy/traverse/service/exception/NotImplementedException.java Outdated Show resolved Hide resolved

AnuradhaSK reviewed Nov 29, 2024

View reviewed changes

...source/hierarchy/traverse/service/exception/OrgResourceHierarchyTraverseClientException.java Outdated Show resolved Hide resolved

AnuradhaSK reviewed Nov 29, 2024

View reviewed changes

...ion/resource/hierarchy/traverse/service/exception/OrgResourceHierarchyTraverseException.java Outdated Show resolved Hide resolved

AnuradhaSK reviewed Nov 29, 2024

View reviewed changes

...ion/resource/hierarchy/traverse/service/exception/OrgResourceHierarchyTraverseException.java Show resolved Hide resolved

AnuradhaSK reviewed Nov 29, 2024

View reviewed changes

...source/hierarchy/traverse/service/internal/OrgResourceHierarchyTraverseServiceComponent.java Outdated Show resolved Hide resolved

AnuradhaSK reviewed Nov 29, 2024

View reviewed changes

...source/hierarchy/traverse/service/internal/OrgResourceHierarchyTraverseServiceComponent.java Outdated Show resolved Hide resolved

AnuradhaSK reviewed Nov 29, 2024

View reviewed changes

...ource/hierarchy/traverse/service/internal/OrgResourceHierarchyTraverseServiceDataHolder.java Outdated Show resolved Hide resolved

AnuradhaSK reviewed Nov 29, 2024

View reviewed changes

...ource/hierarchy/traverse/service/internal/OrgResourceHierarchyTraverseServiceDataHolder.java Outdated Show resolved Hide resolved

AnuradhaSK reviewed Nov 29, 2024

View reviewed changes

...ource/hierarchy/traverse/service/internal/OrgResourceHierarchyTraverseServiceDataHolder.java Outdated Show resolved Hide resolved

AnuradhaSK reviewed Nov 29, 2024

View reviewed changes

...organization/resource/hierarchy/traverse/service/strategy/FirstFoundAggregationStrategy.java Outdated Show resolved Hide resolved

dhaura added 2 commits November 29, 2024 17:41

Remove redundant org id list empty check.

b4aa7f9

Update pom version in traverse service.

3349bbd

omindu reviewed Nov 29, 2024

View reviewed changes

...ource/hierarchy/traverse/service/internal/OrgResourceHierarchyTraverseServiceDataHolder.java Outdated Show resolved Hide resolved

dhaura added 2 commits November 29, 2024 18:08

Improve javadoc comments and variable naming.

938dc30

Fix data holder method comment.

beca634

AnuradhaSK reviewed Nov 29, 2024

View reviewed changes

...dentity/organization/resource/hierarchy/traverse/service/OrgResourceResolverServiceTest.java Outdated Show resolved Hide resolved

AnuradhaSK reviewed Nov 29, 2024

View reviewed changes

...dentity/organization/resource/hierarchy/traverse/service/OrgResourceResolverServiceTest.java Outdated Show resolved Hide resolved

jenkins-is-staging approved these changes Nov 29, 2024

View reviewed changes

dhaura and others added 4 commits December 4, 2024 10:48

Fix server error unit tests.

e742138

Merge branch 'main' into DP-add-b2b-resource-resolver

1a8702a

Bump resource traverse version.

0507e26

Merge remote-tracking branch 'origin/DP-add-b2b-resource-resolver' in…

d2107a1

…to DP-add-b2b-resource-resolver

AnuradhaSK approved these changes Dec 4, 2024

View reviewed changes

jenkins-is-staging approved these changes Dec 4, 2024

View reviewed changes

AnuradhaSK merged commit 5551fc9 into wso2-extensions:main Dec 4, 2024
4 checks passed

This was referenced Dec 4, 2024

Bump Org Mgt Version wso2/product-is#21911

Merged

Introduce B2B Notification Template Inheritance wso2-extensions/identity-event-handler-notification#291

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce Generic Organization Resource Resolver #415

Introduce Generic Organization Resource Resolver #415

dhaura commented Nov 21, 2024 •

edited

Loading

codecov bot commented Nov 21, 2024 •

edited

Loading

omindu Nov 21, 2024

dhaura Nov 22, 2024 •

edited

Loading

dhaura Nov 22, 2024

omindu Nov 21, 2024

dhaura Nov 21, 2024

omindu Nov 21, 2024

dhaura Nov 29, 2024

jenkins-is-staging commented Nov 29, 2024

jenkins-is-staging commented Nov 29, 2024

jenkins-is-staging left a comment

jenkins-is-staging commented Dec 4, 2024

jenkins-is-staging commented Dec 4, 2024

jenkins-is-staging left a comment

	default Map<String, String> getAncestorAppIds(String childAppId, String childOrgId)
	throws OrganizationManagementException {

	throw new NotImplementedException(
	"getAncestorAppIds method is not implemented in " + this.getClass().getName());
	}

Introduce Generic Organization Resource Resolver #415

Introduce Generic Organization Resource Resolver #415

Conversation

dhaura commented Nov 21, 2024 • edited Loading

Purpose

Public Issues

Related Issues

Related PRs

codecov bot commented Nov 21, 2024 • edited Loading

Codecov Report

omindu Nov 21, 2024

Choose a reason for hiding this comment

dhaura Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

dhaura Nov 22, 2024

Choose a reason for hiding this comment

omindu Nov 21, 2024

Choose a reason for hiding this comment

dhaura Nov 21, 2024

Choose a reason for hiding this comment

omindu Nov 21, 2024

Choose a reason for hiding this comment

dhaura Nov 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jenkins-is-staging commented Nov 29, 2024

jenkins-is-staging commented Nov 29, 2024

jenkins-is-staging left a comment

Choose a reason for hiding this comment

jenkins-is-staging commented Dec 4, 2024

jenkins-is-staging commented Dec 4, 2024

jenkins-is-staging left a comment

Choose a reason for hiding this comment

dhaura commented Nov 21, 2024 •

edited

Loading

codecov bot commented Nov 21, 2024 •

edited

Loading

dhaura Nov 22, 2024 •

edited

Loading