Skip to content

Avoid NPE in DefaultApplications.java#1126

Merged
dmikusa merged 3 commits into
cloudfoundry:mainfrom
aboutcontent:main
Jan 4, 2022
Merged

Avoid NPE in DefaultApplications.java#1126
dmikusa merged 3 commits into
cloudfoundry:mainfrom
aboutcontent:main

Conversation

@marc-schaefers

Copy link
Copy Markdown
Contributor

avoid NPE, if environmentJsons is null from application entity

avoid NPE, if environmentJsons is null from application entity
@linux-foundation-easycla

linux-foundation-easycla Bot commented Nov 18, 2021

Copy link
Copy Markdown

CLA Signed

The committers are authorized under a signed CLA.

@marc-schaefers marc-schaefers changed the title Update DefaultApplications.java Avoid NPE in DefaultApplications.java Nov 18, 2021
@dmikusa

dmikusa commented Nov 18, 2021

Copy link
Copy Markdown

@marc-schaefers - Thanks for the submission! Are you seeing this NPE in actual usage of the library? Do you have a stack trace you can share? Just looking for a little more context on this.

Also, you need to click through the links above and appease the EasyCLA bot before we can accept this PR.

@dmikusa dmikusa added the bug label Nov 18, 2021
@marc-schaefers

Copy link
Copy Markdown
Contributor Author

Hi, i use the PushApplicationRequest to push a static build pack with the cloud foundry java libs.
I want to push it on a customer cloud foundry environment, so i haven't configure it or anything.

On one app name i get a nullpointer exception:
ERROR 18.11.2021 09:22:23.296 (de.aboutcontent.cloudfoundry.deploy.DeployExecutable): error on push java.lang.NullPointerException at org.cloudfoundry.operations.applications.DefaultApplications.lambda$getApplicationId$67(DefaultApplications.java:793) Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException: Assembly trace from producer [reactor.core.publisher.MonoIgnoreElements] : reactor.core.publisher.Mono.checkpoint(Mono.java:1902) org.cloudfoundry.operations.applications.DefaultApplications.pushManifest(DefaultApplications.java:432) Error has been observed at the following site(s): |_ Mono.checkpoint ⇢ at org.cloudfoundry.operations.applications.DefaultApplications.pushManifest(DefaultApplications.java:432) |_ Mono.checkpoint ⇢ at org.cloudfoundry.operations.applications.DefaultApplications.push(DefaultApplications.java:404) Stack trace: at org.cloudfoundry.operations.applications.DefaultApplications.lambda$getApplicationId$67(DefaultApplications.java:793)ations.java:793)

So i have changed this line to insert an empty map, if the jsons env null and now it works like expected.

@dmikusa

dmikusa commented Nov 21, 2021

Copy link
Copy Markdown

Thanks for the additional info. I think I can see how it might get into that case. I'm kind of surprised that the server is returning null and not an empty map, but we should handle it either way.

Two points of feedback on the PR:

  1. Can you add a unit test? Probably here. Just to reproduce the error and then verify it's fixed.
  2. Could you use Optional instead of the ternary operation & != null check? Not a major difference, but it matches the style of the rest of the code better.

Thanks

@marc-schaefers

Copy link
Copy Markdown
Contributor Author

Hi @dmikusa-pivotal , the source code is updated now and i added a new push test case.

@dmikusa dmikusa merged commit 6e94919 into cloudfoundry:main Jan 4, 2022
@dmikusa

dmikusa commented Jan 4, 2022

Copy link
Copy Markdown

Sorry for the delay, Log4j2 issues had us scrambling before the holidays. Thanks for the PR!

dmikusa pushed a commit that referenced this pull request Feb 17, 2022
Avoid NPE, if `environmentJsons` is null from the application entity & add test for push with null entity env.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants