Skip to content

Fix null type on simple attribute when value is null in DefaultAttributeFactory#461

Merged
BimsaraBodaragama merged 2 commits into
wso2:masterfrom
BimsaraBodaragama:fix-7428-master
Jun 18, 2026
Merged

Fix null type on simple attribute when value is null in DefaultAttributeFactory#461
BimsaraBodaragama merged 2 commits into
wso2:masterfrom
BimsaraBodaragama:fix-7428-master

Conversation

@BimsaraBodaragama

@BimsaraBodaragama BimsaraBodaragama commented Jun 18, 2026

Copy link
Copy Markdown
Member

Root cause and fix

  • DefaultAttributeFactory.createSimpleAttribute() returns without calling setType() when the attribute value is null. The type is a schema property and must always be set regardless of value.
  • AbstractValidator.validateReturnedAttributes() calls subAttribute.getType().equals(...) without a null guard, causing NPE when a role's audience sub-attribute has a null value.
  • Fix: add simpleAttribute.setType(attributeSchema.getType()); before the final return simpleAttribute on the null-value path. One-line change; the value-present path is unchanged.

Tracking

wso2/product-is#28010

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

In DefaultAttributeFactory.createSimpleAttribute, a single line was added to call simpleAttribute.setType(attributeSchema.getType()) on the early-return path that handles a null attribute value. Previously, setType was only invoked after the non-null value had been validated and assigned, leaving the type unset for SimpleAttribute instances constructed with a null value.

The test suite is extended with a regression test for issue #28010, a data-provider-driven test validating schema-type assignment for multiple data types with null values, a negative test confirming type mismatches are still rejected, and a positive test verifying valid non-null values are handled correctly.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The PR description provides root cause analysis, fix details, and tracking information, but lacks structure required by the template. Add missing template sections: Purpose (with issue link formatting), Goals, Approach, Release note, Documentation, and other required checklist items to align with repository standards.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: a null type fix for simple attributes when values are null in DefaultAttributeFactory.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Jun 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 31.96%. Comparing base (3090b94) to head (295ccbb).

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #461      +/-   ##
============================================
+ Coverage     31.93%   31.96%   +0.02%     
- Complexity     1112     1114       +2     
============================================
  Files           137      137              
  Lines         13523    13526       +3     
  Branches       2589     2592       +3     
============================================
+ Hits           4319     4324       +5     
  Misses         8666     8666              
+ Partials        538      536       -2     
Flag Coverage Δ
unit 31.09% <100.00%> (+0.03%) ⬆️

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

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jenkins-is-staging

Copy link
Copy Markdown

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

@jenkins-is-staging

Copy link
Copy Markdown

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

@jenkins-is-staging jenkins-is-staging left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

@BimsaraBodaragama BimsaraBodaragama merged commit dfc9ac0 into wso2:master Jun 18, 2026
5 checks passed
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.

3 participants