INT-3485 Remove final Modifier AbstractEndpoint.stop(Runnable callback)

[krisjacyna]

JIRA: https://jira.spring.io/browse/INT-3485

I have signed and agree to the terms of the SpringSource Individual Contributor License Agreement.

[garyrussell]

Thanks for your contribution!

However, I should have been a little clearer in the JIRA description; we'd prefer to use this technique...

public final void stop(Runnable callback) {
    this.lifecycleLock.lock();
    try {
        doStop(callback);
    }
    finally {
        this.lifecycleLock.unlock();
    }
}

protected void doStop(Runnable callback) {
    this.stop();
    callback.run();
}

allowing the subclass to override the doStop(...) method instead of stop() itself.

[krisjacyna]

Thanks Gary, makes much more sense.

Please see updated, and a simple test for arguments sake:

package org.springframework.integration.endpoint;

import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.mock;

import java.util.concurrent.atomic.AtomicInteger;

import org.junit.Before;
import org.junit.Test;
import org.springframework.beans.factory.BeanFactory;

/**
 * @author Kris Jacyna
 */
public class CustomPollingConsumerEndpointTests {

    private CustomEndpoint endpoint;

    @Before
    public void init() throws Exception {
        endpoint = new CustomEndpoint();
        endpoint.setBeanFactory(mock(BeanFactory.class));
        endpoint.afterPropertiesSet();
    }

    @Test
    public void customDoStop() {
        assertEquals(0, endpoint.getCount());
        endpoint.stop(new Runnable() {
            @Override
            public void run() {
                try {
                    Thread.sleep(1000L);
                } catch (final Exception e) {}
            }
        });
        assertEquals(1, endpoint.getCount());
    }

    private static class CustomEndpoint extends AbstractEndpoint {

        private final AtomicInteger count = new AtomicInteger(0);

        public int getCount() {
            return this.count.get();
        }

        @Override
        protected void doStop(final Runnable callback) {
            this.count.incrementAndGet();
            super.doStop(callback);
        }

        @Override
        protected void doStart() {
            // start
        }

        @Override
        protected void doStop() {
            // stop
        }
    }
}

[artembilan]

@KaPr , thank you for contribution!

Here you should really invoke that abstract doStop(), not stop() with one more lock acquiring.
Further:

• The test-case on the matter is required.
• Add your name to author list
• Update copyright
• Minor: do not use this. for methods
• no blank lines in the method JavaDocs

For the future: please, correctly format your first commit message:

• headline <= 50 symbols
• JIRA link over a blank line after header
• Some minor description

[artembilan]

I think MessageProducerSupportTests class is enough to place a new test for custom doStop().

[krisjacyna]

Thanks Artem for the feedback, I have updated as follows:

• Delegated to doStop() rather than stop()
• Added author tag
• Added test case
• Removed blank line from javadoc

I didn't remove this. from the method call as the rest of the class uses this style, and the copyright already covers 2014.

Thanks

[krisjacyna]

Is there anyway to get more info from the travis build?

[garyrussell]

No; they blow away the VM; if we get a reliable failure, we add --debug to the build. However, don't worry about that failure - it's probably a timing issue (happens from time to time). I'll re-kick the build on Travis.

Thanks again!!

[artembilan]

LGTM
Merging...

Looking forward for more contribution!

[artembilan]

Merged via: d2da161

[krisjacyna]

Thanks guys