Skip to content

fix: Template getObjects doesn't throw NPE when objects is null #3606

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Dec 10, 2021
Merged

fix: Template getObjects doesn't throw NPE when objects is null #3606

merged 2 commits into from
Dec 10, 2021
+106 −2

Conversation

manusa
Copy link
Member

@manusa manusa commented Nov 22, 2021 •
edited
Loading

Description

fix: Template getObjects doesn't throw NPE when objects is null

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift

Sorry, something went wrong.

@centos-ci
Copy link

Can one of the admins verify this patch?

@manusa manusa mentioned this pull request Nov 22, 2021
Merged
12 tasks
@manusa manusa requested a review from rohanKanojia November 22, 2021 08:58
@manusa manusa self-assigned this Nov 22, 2021
@manusa manusa force-pushed the fix/template-getObjects-NPE branch 3 times, most recently from c7829fb to 9dff1bd Compare November 23, 2021 11:39
@shawkins
Copy link
Contributor

@rohanKanojia @manusa this may be a silly question, but why is sorting required?

@manusa
Copy link
Member Author

manusa commented Nov 23, 2021

No idea, I guess that at some point the order on which the items was applied was important.

@manusa
Copy link
Member Author

manusa commented Nov 23, 2021

Traced it back to fabric8io/kubernetes-model@78ab3a6#diff-4a305396a3d2548c17df3329d5251eda2dca65c060f858588d99068cc7d205e9

@shawkins
Copy link
Contributor

No idea, I guess that at some point the order on which the items was applied was important.

If a reference can't be found, I'd say this behavior seems undesirable. Based upon HasMetadataComparator (which seems somewhat arbitrary) it look like a pretty print option that was added to KubernetesList and Template. Ideally it should optional or the user's responsibility.

@manusa
Copy link
Member Author

manusa commented Nov 23, 2021

At this point this would be a breaking change, and I'm sure some projects rely on this (JKube at some extent does).
I would be more comfortable creating an issue from this and then target v6

shawkins
shawkins approved these changes Nov 30, 2021
View reviewed changes
Copy link
Contributor

@shawkins shawkins left a comment

Choose a reason for hiding this comment

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

LGTM with the consideration that sorting may be optional or removed in the future.

@manusa manusa force-pushed the fix/template-getObjects-NPE branch from 9dff1bd to 885be1f Compare December 2, 2021 11:37
@manusa manusa mentioned this pull request Dec 2, 2021
Closed
@manusa manusa added this to the 5.11.0 milestone Dec 2, 2021
@manusa manusa requested a review from oscerd December 2, 2021 11:56
@manusa
test: Template getObjects when null should not throw NPE
e80fd16 
This failing test highlights an existing BUG. When calling
Template#getObjects method with a null objects field, an NPE
is thrown.

Signed-off-by: Marc Nuri <marc@marcnuri.com>
@manusa
fix: Template getObjects doesn't throw NPE when objects is null
f5d254b 
Signed-off-by: Marc Nuri <marc@marcnuri.com>
@manusa manusa force-pushed the fix/template-getObjects-NPE branch from 885be1f to f5d254b Compare December 10, 2021 09:25
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@manusa manusa merged commit 7c41f11 into fabric8io:master Dec 10, 2021
@manusa manusa deleted the fix/template-getObjects-NPE branch December 10, 2021 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shawkins shawkins

@rohanKanojia rohanKanojia

@oscerd oscerd

Assignees

@manusa manusa

Labels
None yet
Projects
None yet
Milestone
5.11.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@manusa @centos-ci @shawkins @rohanKanojia