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

[WFCORE-591] NPE in revertReloadRequired #559

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
@@ -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 <a href="http://jmesnil.net/">Jeff Mesnil</a> (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;
}
}
}
}