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

[PAYARA-4160] - Error 500 when listing EJB Timer Count on DAS #4302

Merged
merged 3 commits into from Nov 7, 2019

Conversation

@cubastanley
Copy link
Contributor

cubastanley commented Nov 6, 2019

Description

This is a bug fix

Testing

Test suites executed

  • Quicklook
  • Java EE7 Samples
  • Java EE8 Samples
  • Payara Microprofile TCKs Runner
  • Cargo Tracker

Testing Environment

Ubuntu LTS 18.04, Java 1.8.0_222

Notes for Reviewers

This is a fix for an error 500 that occurs when selecting to list timers on instances within a deployment group on the DAS. This error is the result of a null array being returned when no EJB timers are present so I've simply filled the array with 0s.

Whilst working on this, I've discovered another potential issue which will require looking into, however it needs further research and will likely be resolved in a separate issue/PR

@cubastanley cubastanley changed the title [PAYARA-4160] - [PAYARA-4160] - Error 500 when listing EJB Timer Count on DAS Nov 7, 2019
@cubastanley cubastanley marked this pull request as ready for review Nov 7, 2019
@cubastanley cubastanley requested review from AlanRoth and MattGill98 Nov 7, 2019
@cubastanley

This comment has been minimized.

Copy link
Contributor Author

cubastanley commented Nov 7, 2019

Jenkins test please

@@ -146,6 +146,9 @@ public void execute(AdminCommandContext context) {

private String[] listTimers(String[] serverIds) {
String[] result = new String[serverIds.length];
for(int i = 0; i < result.length; i++) {

This comment has been minimized.

Copy link
@pzygielo

pzygielo Nov 7, 2019

Contributor

Why not set result[i]s if(!nonNull(ejbTimerService))?

@@ -146,6 +146,9 @@ public void execute(AdminCommandContext context) {

private String[] listTimers(String[] serverIds) {
String[] result = new String[serverIds.length];

This comment has been minimized.

Copy link
@pzygielo

pzygielo Nov 7, 2019

Contributor

Why allocate new String[...] just to be discarded if(nonNull(ejbTimerService))?

@@ -146,6 +146,9 @@ public void execute(AdminCommandContext context) {

private String[] listTimers(String[] serverIds) {
String[] result = new String[serverIds.length];
for(int i = 0; i < result.length; i++) {
result[i] = "0";

This comment has been minimized.

Copy link
@pzygielo

pzygielo Nov 7, 2019

Contributor

Is "0" special timer? Would empty "" do?

This comment has been minimized.

Copy link
@cubastanley

cubastanley Nov 7, 2019

Author Contributor

The "0" is purely for correct reflection on the DAS UI, I imagine returning an empty string would then present an empty string for the value of timer count (ie number of timers on an instance) 0 is much more accurate than nothing for an instance with no timers

for (int i = 0; i < result.length; i++) {
result[i] = "0";
}
Comment on lines 160 to 162

This comment has been minimized.

Copy link
@pdudits

pdudits Nov 7, 2019

Contributor

From "Obscure standard library methods" I present you Arrays.fill(Object[], Object value)

@cubastanley cubastanley requested a review from pdudits Nov 7, 2019
@cubastanley cubastanley self-assigned this Nov 7, 2019
@pdudits
pdudits approved these changes Nov 7, 2019
@pdudits

This comment has been minimized.

Copy link
Contributor

pdudits commented Nov 7, 2019

Jenkins test please

@cubastanley cubastanley merged commit 1ad633c into payara:master Nov 7, 2019
58 checks passed
58 checks passed
Payara Quick Build and Test Quick build and test passed!
Details
security/snyk - api/payara-api/pom.xml (payara-ci) No new issues
Details
security/snyk - api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admingui/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ant-tasks/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/appclient/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/batch/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/common/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/concurrent/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/connectors/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/core/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ejb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/extras/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/featuresets/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ha/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/installer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jdbc/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/load-balancer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/orb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/payara-appserver-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/persistence/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/registration/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/security/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/transaction/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/web/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/webservices/pom.xml (payara-ci) No new issues
Details
security/snyk - copyright/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/cluster/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/common/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/core/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/diagnostics/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/hk2/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/payara-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources-l10n/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/security/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/test-utils/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - pom.xml (payara-ci) No new issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.