From 36645e8d7c8715e5e7c19418daf909ffa6acb0ef Mon Sep 17 00:00:00 2001 From: Jeff Mesnil Date: Thu, 12 Mar 2015 16:02:21 +0100 Subject: [PATCH] [WFCORE-591] NPE in revertReloadRequired Prevent revertReloadRequired (resp. revertRestartReloadRequired) to throw a NPE if reloadRequired (resp. restartRequired) as not been called beforehands. JIRA: https://issues.jboss.org/browse/WFCORE-591 --- .../controller/AbstractOperationContext.java | 8 ++ .../test/RevertReloadRequiredTestCase.java | 104 ++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 controller/src/test/java/org/jboss/as/controller/test/RevertReloadRequiredTestCase.java diff --git a/controller/src/main/java/org/jboss/as/controller/AbstractOperationContext.java b/controller/src/main/java/org/jboss/as/controller/AbstractOperationContext.java index 23d31a2e385..23eced6e1b4 100644 --- a/controller/src/main/java/org/jboss/as/controller/AbstractOperationContext.java +++ b/controller/src/main/java/org/jboss/as/controller/AbstractOperationContext.java @@ -1030,6 +1030,10 @@ public final void restartRequired() { @Override public final void revertReloadRequired() { if (processState.isReloadSupported()) { + //skip if reloadRequired() was not called + if (this.activeStep.restartStamp == null) { + return; + } processState.revertReloadRequired(this.activeStep.restartStamp); if (activeStep.response.get(RESPONSE_HEADERS).hasDefined(OPERATION_REQUIRES_RELOAD)) { activeStep.response.get(RESPONSE_HEADERS).remove(OPERATION_REQUIRES_RELOAD); @@ -1044,6 +1048,10 @@ public final void revertReloadRequired() { @Override public final void revertRestartRequired() { + //skip if restartRequired() was not called + if (this.activeStep.restartStamp == null) { + return; + } processState.revertRestartRequired(this.activeStep.restartStamp); if (activeStep.response.get(RESPONSE_HEADERS).hasDefined(OPERATION_REQUIRES_RESTART)) { activeStep.response.get(RESPONSE_HEADERS).remove(OPERATION_REQUIRES_RESTART); diff --git a/controller/src/test/java/org/jboss/as/controller/test/RevertReloadRequiredTestCase.java b/controller/src/test/java/org/jboss/as/controller/test/RevertReloadRequiredTestCase.java new file mode 100644 index 00000000000..cfbe13e12a6 --- /dev/null +++ b/controller/src/test/java/org/jboss/as/controller/test/RevertReloadRequiredTestCase.java @@ -0,0 +1,104 @@ +/* + * JBoss, Home of Professional Open Source. + * Copyright 2015, Red Hat Middleware LLC, and individual contributors + * as indicated by the @author tags. See the copyright.txt file in the + * distribution for a full listing of individual contributors. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package org.jboss.as.controller.test; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.junit.Assert.assertThat; + +import org.jboss.as.controller.AbstractAddStepHandler; +import org.jboss.as.controller.AbstractRemoveStepHandler; +import org.jboss.as.controller.ManagementModel; +import org.jboss.as.controller.OperationContext; +import org.jboss.as.controller.OperationFailedException; +import org.jboss.as.controller.PathAddress; +import org.jboss.as.controller.PathElement; +import org.jboss.as.controller.SimpleResourceDefinition; +import org.jboss.as.controller.descriptions.NonResolvingResourceDescriptionResolver; +import org.jboss.as.controller.operations.common.Util; +import org.jboss.as.controller.operations.global.GlobalNotifications; +import org.jboss.as.controller.operations.global.GlobalOperationHandlers; +import org.jboss.as.controller.registry.ManagementResourceRegistration; +import org.jboss.as.controller.registry.Resource; +import org.jboss.dmr.ModelNode; +import org.junit.Test; + +/** + * Test to verify that calling revertReloadRequired does not generate a NPE if + * reloadRequired was not called first. + * + * @author Jeff Mesnil (c) 2015 Red Hat inc. + */ +public class RevertReloadRequiredTestCase extends AbstractControllerTestBase { + + private static ModelNode createdResource = null; + private static Throwable throwable = null; + + @Test + public void testRevertReloadRequiredWithoutReloadRequired() throws Exception { + assertThat(createdResource, is(nullValue())); + + //Just make sure it works as expected for an existent resource + ModelNode op = Util.createAddOperation(PathAddress.pathAddress("test", "exists")); + executeCheckForFailure(op); + + assertThat(createdResource, is(nullValue())); + // call to context.revertReloadRequired() must not throw a NPE + assertThat(throwable, is(nullValue())); + } + + @Override + protected void initModel(ManagementModel managementModel) { + ManagementResourceRegistration registration = managementModel.getRootResourceRegistration(); + GlobalOperationHandlers.registerGlobalOperations(registration, processType); + GlobalNotifications.registerGlobalNotifications(registration, processType); + registration.registerSubModel(new TestResource()); + } + + private static class TestResource extends SimpleResourceDefinition { + + public TestResource() { + super(PathElement.pathElement("test"), + new NonResolvingResourceDescriptionResolver(), + new TestResourceAddHandler(), + new AbstractRemoveStepHandler() { + }); + } + } + + private static class TestResourceAddHandler extends AbstractAddStepHandler { + + @Override + protected void performRuntime(OperationContext context, ModelNode operation, Resource resource) throws OperationFailedException { + context.setRollbackOnly(); + } + + @Override + protected void rollbackRuntime(OperationContext context, ModelNode operation, Resource resource) { + try { + context.revertReloadRequired(); + } catch (Throwable t) { + throwable = t; + } + } + } +}