Wednesday, 10 August 2016

[CleanCode / Patterns] How classes like Period should be constructed ?

I've been thinking recently a lot about constructing objects that contain only two fields. The system I develop contains many classes like this which is rather problematic. I guess Period is a great example.
    public class Period {
        private final DateTime start;
        private final DateTime end;

	...
    }
How can we create instance of Period ?

1. No-arg constructor + setters

This is probably the worst way. It would look like that:
final Period period = new Period();
period.setStart(startDate);
period.setEnd(endDate);
Advantages:
  • None
Disadvantages:
  • you create empty object
  • it takes three lines of code
  • it's muttable so you never know if the object is consistent
  • you can make stupid mistakes like invoke same setter twice with different parameters and so on
Basically for me no-arg constructor and set of getters and setters is a data structure. It isn't encapsulation because you won't put any logic to setters. You can make all fields public - it's the same. Let's talk about object consistency for a while. Consider you have Address class:
    public static class Address {
        private final String street;
        private final String houseNumber;
        private final String postalCode;
    }
Some user (let's say Ben) registers in your system with the following address:
new Address("Long street", "16a", "50-500");
After few days Ben finds better flat on the same street say: Long Steet, 46d, 50-500 What now ? Should we update existing address or create new one ? Some people would say update but what if this house belongs to other post therefore should have other postal code ? I think that we should in such case always create new object. I have a validator which checks Address instance so I am sure that the address is correct. When I create and validate object I get consistent and correct object so immutable objects should be created whenever it's possible.

2. All-args constructor + getters

Advantages:
  • object is immutable
  • it takes one line to create instance
Disadvantages:
  • it's not very readable (espiecially for people who aren't developers)
  • when constructor's parameters are of the same type it's easy to swap them by mistake

3. Builder

I think it's best solution in this case. Most people use builder pattern for classes which have a lot of fields. In my opinion builder is a perfect pattern to create instance of class which has only two fields. Let's get back to Period class:
    @Getter
    public static class Period {
        private final DateTime startDate;
        private final DateTime endDate;

        public static Builder newPeriodThatStarts(final DateTime startDate) {
            return new Builder(startDate);
        }

        private Period(final DateTime startDate, final DateTime endDate) {
            this.startDate = startDate;
            this.endDate = endDate;
        }

        public static class Builder {
            private final DateTime startDate;
            
            private Builder(final DateTime startDate) {
                this.startDate = startDate;
            }
            
            public Period andEnds(final DateTime endDate) {
                return new Period(startDate, endDate);
            }
        }
    }
And this is how you instantiate Period instance:
    public static void main(String [] args) {
        Period period = newPeriodThatStarts(now()).andEnds(now().plusDays(1));
        Period anotherPeriod = newPeriodThatStarts(new DateTime(2016, 2, 15, 10, 0, 0)).andEnds(now());
    }
Advantages:
  • object is immutable
  • it takes one line to create instance
  • it's very readable even for someone who isn't software developer
Disadvantages:
  • you have to create a builder (or use lombok)
Builder seems to be the best way for me. For Period you could also use all args constructor because the parameters have natural order - the first one is startDate and the second endDate (I can't really imagine the opposite) so you shouldn't swap the parameters by mistake. A lot of domain objects take two strings as parameters and in this case I would definitely use builder.

Tuesday, 19 July 2016

[Java 8 / Threads / Parallel stream] How to control pool size while using parallel stream ?

Recently I had to implement huge functionality which among other things is responsible for automatic buying of products available in some shop. The api allows to put many products into single request but due to some legal issues the items have to be bought one by one. So if someone wants to buy:
Product: Witcher 3, quantity: 15
Product: GTA 5, quantity: 5
I have to make 20 requests. It's a soap endpoint so it takes lots of time. Consider the following method:
    public OrderResult makeOrder(final List<ExternalSupplierOrderEntry> orderEntries, final String orderId, final String language) {
        final List<List<ExternalSupplierOrderEntry>> orderEntriesChunks = orderSplitter.split(orderEntries);
        final List<ExternalSupplierCode> boughtCodes = orderEntriesChunks.stream()
                .map(chunk -> Try.ofFailable(() -> buy(orderEntries, orderId, language))
                                 .whenFailure(t -> log().error("Something went wrong while making order", t))
                                 .orElse(markCodesAsFailed(chunk)))
                .flatMap(Collection::stream)
                .collect(toList());

        return new OrderResult(boughtCodes, orderId);
   }
It splits the order into chunks (one item per chunk) and buys it. buy() method calls soap endpoint which returns the code. When I try to buy 50 codes it takes one minute to complete the order. It's way too long so my first thought was: replace stream() with parallelStream(). And it actually works :)
    public OrderResult makeOrder(final List<ExternalSupplierOrderEntry> orderEntries, final String orderId, final String language) {
        final List<List<ExternalSupplierOrderEntry>> orderEntriesChunks = orderSplitter.split(orderEntries);
        final List<ExternalSupplierCode> boughtCodes = orderEntriesChunks.parallelStream()
                .map(chunk -> Try.ofFailable(() -> buy(orderEntries, orderId, language))
                                 .whenFailure(t -> log().error("Something went wrong while making order", t))
                                 .orElse(markCodesAsFailed(chunk)))
                .flatMap(Collection::stream)
                .collect(toList());

        return new OrderResult(boughtCodes, orderId);
   }
I'm trying to buy 50 codes so buy() method is being invoked 50 times. For stream() I get: 61.3s For parallelStream() I get: 10.21s 10 seconds is a huge improvement but it's still very long so my second thought was to increase the number of threads in pool. parallelStream() is ok but there's no overloaded parallelStream(int threads) method. Since Java 7 fork / join framework is available directly in the JDK. Parallel stream utilizes the framework in order to perform operations on stream's elements using multiple threads. When you look into ForkJoinPool class you'll see that default construvtor sets default number of threads (parallelism parameter) like that:
    public ForkJoinPool() {
        this(Math.min(MAX_CAP, Runtime.getRuntime().availableProcessors()),
             defaultForkJoinWorkerThreadFactory, null, false);
    }
I takes minimum(availableProcessors, 0x7fff) where 0x7fff = 32767 You'll typically get here min(8, 32767) = 8. Let's make some test.
    public static void main(String [] args) {
        final Set<Object> threadNames = IntStream.range(0, 10).parallel()
                .boxed()
                .peek(i -> Try.ofFailable(() -> { Thread.sleep(1000); return i; }))
                .map(i -> Thread.currentThread().getName())
                .collect(toSet());
        System.out.println(threadNames.size());
        System.out.println(threadNames);
    }
It prints:
4
[ForkJoinPool.commonPool-worker-1, ForkJoinPool.commonPool-worker-2, main, ForkJoinPool.commonPool-worker-3]
Note that peek operation which sleeps for a second has been added to make operations longer so all the threads are being used. Let's try to increase number of threads in the pool using ForkJoinPool.
    public static void main(String [] args) throws ExecutionException, InterruptedException {
        final ForkJoinPool forkJoinPool = new ForkJoinPool(20);
        final Set<String> threadNames = forkJoinPool.submit(() -> IntStream.range(0, 20).parallel()
                .boxed()
                .peek(i -> Try.ofFailable(() -> { Thread.sleep(1000); return true; }).toOptional())
                .map(i -> Thread.currentThread().getName())
                .collect(toSet())).get();

        System.out.println(threadNames.size());
        System.out.println(threadNames);
    }
This one prints:
20
[ForkJoinPool-1-worker-8, ForkJoinPool-1-worker-30, ForkJoinPool-1-worker-9, ForkJoinPool-1-worker-23, ForkJoinPool-1-worker-12, ForkJoinPool-1-worker-22, ForkJoinPool-1-worker-11, ForkJoinPool-1-worker-20, ForkJoinPool-1-worker-1, ForkJoinPool-1-worker-4, ForkJoinPool-1-worker-5, ForkJoinPool-1-worker-2, ForkJoinPool-1-worker-16, ForkJoinPool-1-worker-27, ForkJoinPool-1-worker-15, ForkJoinPool-1-worker-26, ForkJoinPool-1-worker-25, ForkJoinPool-1-worker-19, ForkJoinPool-1-worker-18, ForkJoinPool-1-worker-29]
As you can see number of threads involved in resolving stream's output has been increased to 20. It takes only one additional line because you have to create ForkJoinPool object. I'd really like to get rid of that line so I don't have to remember about ForkJoinPool so I've created this class:
/**
 * @author Grzegorz Taramina
 *         Created on: 18/07/16
 */
public class ForkJoinPoolInvoker {
    private final ForkJoinPool forkJoinPool;

    public static ForkJoinPoolInvoker usePoolWithSize(final int poolSize) {
        return new ForkJoinPoolInvoker(poolSize);
    }

    private ForkJoinPoolInvoker(final int poolSize) {
        this.forkJoinPool = new ForkJoinPool(poolSize);
    }

    public <T> T andInvoke(final Callable<T> task) {
        final ForkJoinTask<T> submit = forkJoinPool.submit(task);
        return Try.ofFailable(submit::get).orElseThrow(RuntimeException::new);
    }
}
Now the previous example would lool like that:
    public static void main(String [] args) throws ExecutionException, InterruptedException {
        final Set<Object> threadNames = usePoolWithSize(20).andInvoke(() -> IntStream.range(0, 20).parallel()
                .boxed()
                .peek(i -> Try.ofFailable(() -> { Thread.sleep(1000); return true; }).toOptional())
                .map(i -> Thread.currentThread().getName())
                .collect(toSet()));

        System.out.println(threadNames.size());
        System.out.println(threadNames);
    }
Let's get back to the main example that buys codes:
    public OrderResult makeOrder(final List<ExternalSupplierOrderEntry> orderEntries, final String language) {
        final List<List<ExternalSupplierOrderEntry>> orderEntriesChunks = orderSplitter.split(orderEntries);
        final List<ExternalSupplierCode> boughtCodes =  usePoolWithSize(nexwaySettings.getNumberOfBuyingThreads())
                                                        .andInvoke(() ->
             orderEntriesChunks.stream()
                .parallel()
                .map(chunk -> {
                    final String orderId = uuidProvider.randomUUID();
                    return Try.ofFailable(() -> buy(chunk, orderId, language))
                            .whenFailure(t -> log().error("Something went wrong while making order", t))
                            .orElse(markCodesAsFailed(chunk, orderId));
                })
                .flatMap(Collection::stream)
                .collect(toList())
        );

        return new OrderResult(boughtCodes);
    }
I've also checked how many threads would be sufficient to make order in a reasonable time:
Note that all the results presented in the chart have been averaged (for each number of threads the test has been performed 10 times). The chart shows that using 15 threads is sufficient because it takes slightly more than 4 seconds to make 50 requests. As you can see changing pool size is quite easy. I do realize that I could do that differently but in the end this solution looks good. All the calls to the API have timeout so I shouldn't experience all the typical problems connected to parallel stream that people talk about.

Thursday, 14 July 2016

[Spring / Async] How to invoke method in separate thread ?

[Spring / Async] How to invoke method in separate thread ? Some tasks need to be invoked in a separate thread - asynchronously. For instance when the operation needs a lot of time to finish. You just want to run it and return its id or some kind of status. In pure Java you would use Threads / Runnables and other stuff available in low-level cuncurrency API which JDK provides. Actually it isn't very convenient so Spring developers have taken care of that. I've prepared very simple Spring application which contains only two beans: RequestHandler and OperationRunner. The first one invokes the other. Here's the application runner:
    public class App {
        public static void main(final String [] args) throws InterruptedException {
            final AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(AppConfig.class);
            final RequestHandler handler = context.getBean(RequestHandler.class);
            handler.handle();
        }
    }
And a configuration:
@Configuration
@ComponentScan("gt.dev.spring.async")
public class AppConfig {
}
Implementation of components:
    @Component
    public class RequestHandler {
        private static final Logger LOG = Logger.getLogger(RequestHandler.class);

        private final OperationRunner operationRunner;

        @Autowired
        public RequestHandler(final OperationRunner operationRunner) {
            this.operationRunner = operationRunner;
        }

        public void handle() throws InterruptedException {
            LOG.info("Request processing started. Invoking operation...");
            operationRunner.run();
            LOG.info("Request processed");
        }
    }
And OperationRunner:
    @Component
    public class OperationRunner {
        private static final Logger LOG = Logger.getLogger(OperationRunner.class);

        public void run() throws InterruptedException {
            LOG.info("Operation started. Sleeping...");
            Thread.sleep(5000);
            LOG.info("Operation finished");
        }
    }
After running main method in App class I get the following logs:
maj 07, 2015 3:39:28 PM gt.dev.spring.async.RequestHandler handle
INFO: Request processing started. Invoking operation...
maj 07, 2015 3:39:28 PM gt.dev.spring.async.OperationRunner run
INFO: Operation started. Sleeping...
maj 07, 2015 3:39:33 PM gt.dev.spring.async.OperationRunner run
INFO: Operation finished
maj 07, 2015 3:39:33 PM gt.dev.spring.async.RequestHandler handle
INFO: Request processed
It works as expected:
  • RequestHandler logs information which indicates that process has been started
  • RequestHandler invokes OperationRunner
  • OperationRunner logs that operation started
  • OperationRunner performs long operation (sleeps for 5 seconds)
  • OperationRunner logs that operation finished
  • control gets back to RequestHandler
  • RequestHandler logs that request has been processed
Every client has to wait until the operation is completed. In some cases we may want to return some kind of id of operation to the client and process in the background asynchronously. Let's make the run() method async. To do that we need @Async annotation that indicates that particular method should be run in separate thread.
    @Component
    public class OperationRunner {
        private static final Logger LOG = Logger.getLogger(OperationRunner.class);

        @Async
        public void run() throws InterruptedException {
            LOG.info("Operation started. Sleeping...");
            Thread.sleep(5000);
            LOG.info("Operation finished");
        }
    }
Spring also has to know that async calls are enabled so we need one additional annotation in spring configuration - @EnableAsync. Lots of people forget about that. It's actually quite clever. You can put @Async on all the methods that need to be asynchronous and when needed disable all async calls by removing @EnableAsync.
    @Configuration
    @ComponentScan("gt.dev.spring.async")
    @EnableAsync
    public class AppConfig {
    }
After running the app I get the following log which proves that request has been processed before the async operation finished.
maj 07, 2015 3:54:13 PM gt.dev.spring.async.RequestHandler handle
INFO: Request processing started. Invoking operation...
maj 07, 2015 3:54:13 PM gt.dev.spring.async.RequestHandler handle
INFO: Request processed
maj 07, 2015 3:54:13 PM gt.dev.spring.async.OperationRunner run
INFO: Operation started. Sleeping...
maj 07, 2015 3:54:18 PM gt.dev.spring.async.OperationRunner run
INFO: Operation finished

Tuesday, 12 July 2016

[Java 8 / Functional programming] Functional util that invokes a command n times.

Recently I've been doing major refactoring of integration tests. I've found many tests which do stuff like that:
for (int i = 0; i < 256; i++) {
     addProduct(UUID.randomUUID().toString);
 }
It's pretty ugly, isn't it ? It would be nice to have a small tool that invokes given piece of code n times. In Java 8 we can use IntStream:
IntStream.range(0, 256).forEach(i -> addProduct(UUID.randomUUID().toString()));
Looks better but it's still not very readable. Again I've started with a test that specifies how the tool should work:
    @Test
    public void shouldInvokeCommandFiveTimes() throws Exception {
        // given
        final List<String> list = newArrayList();

        // when
        times(5).invoke(() -> list.add("item"));

        // then
        assertThat(list).containsExactly("item", "item", "item", "item", "item");
    }
I've come up with the following class:
/**
 * @author Grzegorz Taramina
 *         Created on: 12/07/16
 */
public class Times {
    private final int times;

    private Times(final int times) {
        this.times = times;
    }

    public static Times times(final int times) {
        Assert.isTrue(times >= 0, "times must be at least equal to zero");
        return new Times(times);
    }

    public void invoke(final Runnable runnable) {
        IntStream.range(0, times).forEach(i -> runnable.run());
    }
}
It's very simple but makes code concise and readable:
times(5).invoke(() -> addProduct(randomUUID().toString));
I might have exaggerated saying that this is functional tool. It's simply higher order function but very useful.

Monday, 11 July 2016

[Java8 / Functional programming] How to create object that will be created lazily ?

Sometimes you may want to create some objects lazily. Especially when it comes to really heavy objects that may or may not be used in runtime. In one of the companies I used to work we had to use Java 6. Some developers must have read some articles about laziness and started to create literally all the objects lazily like that:
    public static class OldFashionedHeavyObjectHolder {
        private HeavyObject heavyObject;

        public synchronized HeavyObject getHeavyObject() {
            if (heavyObject == null) {
                heavyObject = new HeavyObject();
            }

            return heavyObject;
        }
    }
After couple of months we had a lot of classes with tons of getters that check if an object is null and so on. I can notice at least four disadvantages of that approach:
  • the method has to be synchronized because more than one thread can invoke the method when heavyObject == null
  • even if heavyObject has already been created you have to check that
  • it's extremely ugly
  • it's hard to test it
Luckily I've changed the company and now I can use all those fancy streams, lambdas and everything that Java 8 comes with. Basically I wanted to create a tool which works like that:
/**
 * @author Grzegorz Taramina
 *         Created on: 23/06/16
 */
public class LazyInstanceTest {
    @Test
    public void shouldCreateLazyInstance() throws Exception {
        // given
        LazyInstance<String> instance = LazyInstance.of(() -> "i'm lazy");

        // when
        String result = instance.get();

        // then
        assertThat(result).isEqualTo("i'm lazy");
    }
}
String is obviously just a simplification. So some kind of factory that creates a holder of a heavy instance and takes care of creating it lazily. I've figured out the following class:
 *
 * @author Grzegorz Taramina
 *         Created on: 23/06/16
 */
public class LazyInstance<T> {
    private final Supplier<T> instanceSupplier;
    private Supplier<T> instance = this::create;

    public static <T> LazyInstance<T> of(final Supplier<T> instanceSupplier) {
        return new LazyInstance<>(instanceSupplier);
    }

    /**
     * Creates LazyInstance
     * @param instanceSupplier supplier that will be lazily used while creating instance
     */
    private LazyInstance(final Supplier<T> instanceSupplier) {
        this.instanceSupplier = instanceSupplier;
    }

    public T get() {
        return instance.get();
    }

    private synchronized T create() {
        class InstanceFactory implements Supplier<T> {
            private final T instance = instanceSupplier.get();

            public T get() {
                return instance;
            }
        }

        if (!InstanceFactory.class.isInstance(instance)) {
            instance = new InstanceFactory();
        }

        return instance.get();
    }
}
It works like that:
final LazyInstance<HeavyObject> heavy = new LazyInstance<>(HeavyObject::new);
HeavyObject heavyObject = heavy.get();
The main idea of this class is that the supplier is being invoked lazily. Synchronized create method returns value returned by InstanceFactory (in fact it's a Supplier). Instance factory in turn returns value that returns Supplier provided to the LazyInstance. So basically we're invoking supplier that invokes supplier that creates real instance. Another thing: instance = new InstanceFactory(); - this line's really important because it swaps suppliers which means that synchronized block and if-else statement are being invoked only once. After the object is created the instance field (in LazyInstance not the InstanceFactory) contains InstanceFactory instance which returns real instance. It may look a bit complicated but I think it does all the stuff quite elegantly. Just to prove that the instance is being created lazily:
    public static class HeavyObject {
        public HeavyObject() {
            System.out.println("heavy's being created...");
        }
    }

    public static void main(String [] args) {
        System.out.println("Started executing main method");
        final LazyInstance<HeavyObject> heavy = LazyInstance.of(HeavyObject::new);
        System.out.println("Created lazy instance");
        System.out.println("Calling heavy.get()");
        HeavyObject heavyObject = heavy.get();
        System.out.println("End of main");
    }
The output:
Started executing main method
Created lazy instance
Calling heavy.get()
heavy's being created...
End of main
I should also mention that it looks good in Java 8 because of lambdas but it can be also implemented in older versions using anonymous classes.

Wednesday, 10 February 2016

[Java8 / Spring / Test] How to test TransactionTemplate's execute() method ?

If your app uses Spring framework you may be familiar with either @Transactional or TransactionTemplate. Altough TransactionTemplate couples your app with Spring a lot of people use it. In most cases it's being injected into DAOs or some kind of AbstractDAO. DAO is an object which typically is tested by integration test which enables in-memory database. In most cases you won't need unit testing here but what if TransactionTemplate has been injected into some kind of service / transaction etc - in general a class which has to be unit tested ? There is one problem with TransactionTemplate - execute() method takes TransactionCallback as a parameter. This is how you would invoke it:
transactionTemplate.execute((s) -> propertyDao.persist(copyOf(toSave).withNewResourceId(accountId)));
If you mock TransactionTemplate then propertyDao.persist() will never be invoked. In my unit test PropertyDao is a mock so now I cannot use Mockito.verify() to check whether persist method has been invoked (it returns void).
private final PropertyDao propertyDao = mock(PropertyDao.class);
Let's see how execute() method has been implemented:
    @Override
    public <T> T execute(TransactionCallback<T> action) throws TransactionException {
        if (this.transactionManager instanceof CallbackPreferringPlatformTransactionManager) {
            return ((CallbackPreferringPlatformTransactionManager) this.transactionManager).execute(this, action);
        }
        else {
            TransactionStatus status = this.transactionManager.getTransaction(this);
            T result;
            try {
                result = action.doInTransaction(status);
            }
            catch (RuntimeException ex) {
                // Transactional code threw application exception -> rollback
                rollbackOnException(status, ex);
                throw ex;
            }
            catch (Error err) {
                // Transactional code threw error -> rollback
                rollbackOnException(status, err);
                throw err;
            }
            catch (Exception ex) {
                // Transactional code threw unexpected exception -> rollback
                rollbackOnException(status, ex);
                throw new UndeclaredThrowableException(ex, "TransactionCallback threw undeclared checked exception");
            }
            this.transactionManager.commit(status);
            return result;
        }
    }
The most important line:
result = action.doInTransaction(status);
It simply means that our function:
transactionTemplate.execute((s) -> propertyDao.persist(copyOf(toSave).withNewResourceId(accountId)));
is being invoked in the method so when you mocked the template it won't happen at all. How to deal with that ? My first thought was to use ArgumentCaptor to catch the parameter passed to execute method and invoke it but I think I found a better way.
class FunctionCallingTransactionTemplate extends TransactionTemplate {
        @Override public <T> T execute(TransactionCallback<T> action) throws TransactionException {
            final TransactionStatus irrelevantStatus = null;
            return action.doInTransaction(irrelevantStatus);
        }
    }
In the code above I extend TransactionTemplate so that in only invokes the action passed to execute() method without other stuff. I guess I'm gonna need this in many tests so we can create a simple trait:
public interface FunctionCallingTransactionTemplateTrait {
    default TransactionTemplate functionCallingTransactionTemplate() {
        return new FunctionCallingTransactionTemplate();
    }

    class FunctionCallingTransactionTemplate extends TransactionTemplate {
        @Override public <T> T execute(TransactionCallback<T> action) throws TransactionException {
            final TransactionStatus irrelevantStatus = null;
            return action.doInTransaction(irrelevantStatus);
        }
    }
}
Now in my test I have:
public class SaveAccountAttributesTransactionTest implements FunctionCallingTransactionTemplateTrait {
    private final ArgumentCaptor propertyCaptor = ArgumentCaptor.forClass(Property.class);
    private final PropertyDao propertyDao = mock(PropertyDao.class);
    private final TransactionTemplate transactionTemplate = functionCallingTransactionTemplate();

    private final SaveAccountAttributesTransaction transaction = new SaveAccountAttributesTransaction(propertyDao, transactionTemplate);
    ...
}
And some test:
    @Test
    public void shouldUpdateOneValueAndPersistOther() throws Exception {
        // given
        when(propertyDao.fetchResourceProperties("root", ACCOUNT)).thenReturn(Lists.newArrayList(
                propertyOf("firstProp", "2.21", "root"),
                propertyOf("secondProp", null, null),
                propertyOf("thirdProp", null, null),
                propertyOf("fourthProp", null, null)
        ));
        SaveAccountAttributesEvent event = new SaveAccountAttributesEvent("root", Lists.newArrayList(
                propertyOf("firstProp", "2.22", "root"),
                propertyOf("secondProp", null, null),
                propertyOf("thirdProp", "1.11", "root"),
                propertyOf("fourthProp","default", null)
        ));

        // when
        transaction.execute(event);

        // then
        verify(propertyDao).updatePropertyValue(anyString(), eq("2.22"));
        verify(propertyDao).persist(propertyCaptor.capture());
        assertThat(propertyCaptor.getAllValues()).extracting(Property::getResourceId, Property::getValue)
                .containsOnly(tuple("root", "1.11"));
    }
And it passess :) As you can see I verify behaviour of propertyDao which is being invoked by our extended TransactionTemplate. Hope it helps.

Monday, 8 February 2016

]Bash / Gawk] How to print how long do you have to stay at work today ?

Today I'm going to show you a short script wirtten when I was running integration tests. It simply shows minutes you still have to be at work.
#!/bin/bash
START_HOUR=$1
START_MINUTE=$2

if [ -z "$START_HOUR" ] || [ -z "$START_MINUTE" ]; then
    START_HOUR="9"
    START_MINUTE="45"
fi

date | gawk '{print $4}' | gawk -F":" '{print 8 * 60 - (($1 * 60 + $2) - ('"$START_HOUR"' * 60 + '"$START_MINUTE"'))}'
It's actually very simple.
START_HOUR=$1
START_MINUTE=$2
Here I create two variables which contain hour and minute I came to the office.
[ -z "$START_HOUR" ]
This condition checks whether a variable contains some value. In case I don't pass the date when I came it sets my default which is 9:45. Then very simple oneliner:
date | gawk '{print $4}' | gawk -F":" '{print 8 * 60 - (($1 * 60 + $2) - ('"$START_HOUR"' * 60 + '"$START_MINUTE"'))}'
date
prints current date -> Mon 8 Feb 15:12:09 CET 2016
date | gawk '{print $4}'
prints the fourth field (split by whitespace) -> 15:12:55
gawk -F":"
-F allows you to specify how do you want the input string to be split - in this case it's ':' so $1 now contains current hour and $2 current minute.
'"$START_HOUR"'
this is how you can access shell variables in gawk And then some simple math (note that I assume that working day == 8h):
'{print 8 * 60 - (($1 * 60 + $2) - ('"$START_HOUR"' * 60 + '"$START_MINUTE"'))}'
8 * 60 = working day (minutes)
($1 * 60 + $2) - ('"$START_HOUR"' * 60 + '"$START_MINUTE"'))
current minute of day minus minute I came to the office
The result of the script is: 148 which means I can go home after 148 minutes :)
You can obviously pass what time you came to work:
./howLong 10 0 prints 162

Wednesday, 27 May 2015

[Java / Guava] How to generate map while having either key or value ?

Map is a data structure which every software developer uses on a daily basis. Maps are generally created to provide a quick access to some object associated to a key. On the other hand we often create maps just have a more conveniant access to a collection of objects. Consider the following class as a main domain object:
public class Book {
 private final String title;
 private final String isbn;

 public Book(String title, String isbn) {
  this.title = title;
  this.isbn = isbn;
 }

 public String getTitle() {
  return title;
 }

 public String getIsbn() {
  return isbn;
 }
}
In case we want to retrieve a book of given isbn number from a list we would do something like this:
public class BookShelf {
    private static final List<Book> books = ImmutableList.of(
            new Book("Metro 2033", "54213452435"),
            new Book("The Witcher", "123123123")
    );

    public Book getBookByIsbn(String isbn) {
        for (Book book : books) {
            if (book.getIsbn().equals(isbn)) {
                return book;
            }
        }

        return null;
    }
}
Here's a simple unit test which checks whether the book has been found:
@Test
    public void shouldReturnMetro() throws Exception {
        // given
        String isbn = "54213452435";

        // when
        final Book book = new BookShelf().getBookByIsbn(isbn);

        // then
        assertThat(book.getTitle()).isEqualTo("Metro 2033");
    }
As you can see we have to iterate through the list and check whether book's isbn is equal to the one passed to the method. Such construction increases method complexity (if you use sonar or some similar tool you probably try to keep as lowest as it's possible). Other thing you have to take care of is a value returned in case the book of given isbn does't exist. I think I chose the worst possible solution which is returning null :) Other options may be:
  • -throwing exception
  • -null object pattern
Let's see how it would look if the books were stored in a map.
public class BookShelf {
    private static final Map<String, Book> books = ImmutableMap.of(
            "54213452435", new Book("Metro 2033", "54213452435"),
            "123123123", new Book("The Witcher", "123123123")
    );

    public Book getBookByIsbn(String isbn) {
        return books.get(isbn);
    }
}
I think it looks much better now. The unit test still passess and null value is returned by default when value for a given key doesn't exist. In real life such map won't exist Typically you fetch books from database or some restful service so it will be a list of books not the map. Let's see how to generate a map from the list in traditional imperative way:
public class BookShelf {
    private final Map<String, Book> books;

    public BookShelf(List<Book> books) {
        Map<String, Book> booksMap = Maps.newHashMap();
        for (Book book : books) {
            booksMap.put(book.getIsbn(), book);
        }

        this.books = booksMap;
    }

    public Book getBookByIsbn(String isbn) {
        return books.get(isbn);
    }
}
And the unit test:
@Test
    public void shouldReturnMetro() throws Exception {
        // given
        List<Book> books = ImmutableList.of(
                new Book("Metro 2033", "54213452435"),
                new Book("The Witcher", "123123123")
        );

        // when
        final Book book = new BookShelf(books).getBookByIsbn("54213452435");

        // then
        assertThat(book.getTitle()).isEqualTo("Metro 2033");
    }
Works fine... but I really don't like the way it's been done. Looks just ugly. Fortunately Guava can help here :) There's a uniqueIndex() method in Maps class which takes Iterable and Function as arguments.
public static <K, V> ImmutableMap<K, V> uniqueIndex(
      Iterable<V> values, Function<? super V, K> keyFunction) {
    return uniqueIndex(values.iterator(), keyFunction);
  }
The fcuntion will be applied to every book in Iterable and will generate a key so uniqueIndex() returns ImmutableMap in which value returned from function is a key and a book a value.
public class BookShelf {
    private final Map<String, Book> books;

    public BookShelf(List<Book> books) {
        this.books = Maps.uniqueIndex(books, new Function<Book, String>() {
            public String apply(Book book) {
                return book.getIsbn();
            }
        });
    }

    public Book getBookByIsbn(String isbn) {
        return books.get(isbn);
    }
}
Much better now. The Function may obviously be injected. In such case changing the function becomes extremely easy. Consider the following example in Spring:
@Component
public class BookShelf {
    private final Map<String, Book> books;

    @Autowired
    public BookShelf(ImmutableList<Book> books, Function<Book, String> bookIndexFunction) {
        this.books = Maps.uniqueIndex(books, bookIndexFunction);
    }

    public Book getBookByIsbn(String isbn) {
        return books.get(isbn);
    }
}
And the configuration:
@Configuration
@ComponentScan("gt.dev.sample")
public class BookshelfConfig {
    @Bean
    public Function<Book, String> bookIndexFunction() {
        return new Function<Book, String>() {
            public String apply(Book book) {
                return book.getIsbn();
            }
        };
    }

    @Bean
    public ImmutableList<Book> books() {
        return ImmutableList.of(
                new Book("Metro 2033", "54213452435"),
                new Book("The Witcher", "123123123")
        );
    }
}
It's just an example so I've injected the books as well. Let's run the application:
public class App {
    public static void main( String[] args ) {
        final BookShelf bookShelf = new AnnotationConfigApplicationContext(BookshelfConfig.class).getBean(BookShelf.class);
        final Book bookByIsbn = bookShelf.getBookByIsbn("54213452435");
        System.out.println(bookByIsbn);
    }
}
And the output is:
maj 26, 2015 1:42:46 PM org.springframework.context.annotation.AnnotationConfigApplicationContext prepareRefresh
INFO: Refreshing org.springframework.context.annotation.AnnotationConfigApplicationContext@67eaf25d: startup date [Tue May 26 13:42:46 CEST 2015]; root of context hierarchy
Book{title=Metro 2033, isbn=54213452435}
Now let's say that I want to generate map while having only a list of keys. uniqueIndex() method cannot be used because it does the opposite so Guava Maps class contains following methods:
  • asMap()
  • toMap()
The only difference between those two methods is that the toMap() method returns an instance of ImmutableMap while asMap() returns a view of the original map. Now I've added the following bean into the configuration:
@Bean
    public ImmutableList<String> theWitcherIsbns() {
        return ImmutableList.of(
                "325252525", 
                "432653462", 
                "23463667", 
                "324632636");
    }
The list contains ISBN numbers of different editions of The Witcher book. I want to create a map in which the key is ISBN and the value an object of Book class. In traditional imperative way you would write another foreach loop which creates an object of Book class and puts it into the map which has to be created before the loop as well. Using Guava it can be written like that:
@Component
public class BooksGenerator {
    private final ImmutableMap<String, Book> theWitcherBooks;

    @Autowired
    public BooksGenerator(ImmutableList<String> theWitcherIsbns) {
        this.theWitcherBooks = Maps.toMap(theWitcherIsbns, new Function<String, Book>() {
            public Book apply(String isbn) {
                return new Book("The Witcher", isbn);
            }
        });
    }

    public Book getWitcherBookByIsbn(String isbn) {
        return theWitcherBooks.get(isbn);
    }
}
After starting the application:
public class App {
    public static void main( String[] args ) {
        final BooksGenerator booksGenerator = new AnnotationConfigApplicationContext(BookshelfConfig.class).getBean(BooksGenerator.class);
        final Book book = booksGenerator.getWitcherBookByIsbn("23463667");
        System.out.println(book);
    }
}
I get a proper book:
maj 26, 2015 4:53:27 PM org.springframework.context.annotation.AnnotationConfigApplicationContext prepareRefresh
INFO: Refreshing org.springframework.context.annotation.AnnotationConfigApplicationContext@6f609af9: startup date [Tue May 26 16:53:27 CEST 2015]; root of context hierarchy
Book{title=The Witcher, isbn=23463667}
asMap() method works the same but as I've mentioned before it returns a view of original map. I strongly recommend using toMap() as immutable collections are more safe (threads) and it's harder to complicate the code using them.

Sunday, 17 May 2015

[Bash / ssh] How to invoke command remotely without password / private key prompts ?

Sometimes you may want to invoke command on remote machine via ssh. You can obviously pass a command in double quotes:
ssh root@somehost.com "echo \$HOME"
This example prints to the console a value of env variable HOME (note that the dollar sign has to be escaped otherwise HOME variable will be resolved on your local machine). Let's say I want to fetch a value of some env variable in my bash script which will be started by Jenkins. There are actually two problems:
  • ssh will prompt for password,
  • if you haven't already accepted host's key there will be another prompt.
If you don't have proper entry in ~/.ssh/known_hosts you will see:
gt ~ ssh root@somehost.com "echo \$HOME"
                                                        
The authenticity of host 'somehost.com (10.92.30.38)' can't be established.
RSA key fingerprint is b0:c6:ad:6b:06:73:a3:de:31:8c:f8:4d:07:4e:2c:e6.
Are you sure you want to continue connecting (yes/no)? 

so you need to type "yes" in order to invoke the command.
In case you've already accepted the key you will see only:
gt ~ ssh root@somehost.com "echo \$HOME"                                                                                                           
root@somehost.com's password: 
Ssh doesn't have any flag for password (security) so you cannot do something like:
ssh root@somehost.com -p mySecretPassword
Solution for that is sshpass. I'm sure it's available in your linux distribution's repository. On Fedora install it using:
sudo yum install sshpass
So now you can pass the password easily:
gt ~ sshpass -p mySecretPassword ssh root@somehost.com "echo \$HOME"                                    
/root
In case you need to accept host's key you can use ssh -oStrictHostKeyChecking=no. Example:
gt ~ ssh root@somehost.com "echo \$HOME"                                                                          
The authenticity of host 'somehost.com (10.92.30.38)' can't be established.
RSA key fingerprint is b0:c6:ad:6b:06:73:a3:de:31:8c:f8:4d:07:4e:2c:e6.
Are you sure you want to continue connecting (yes/no)? ^C
zsh: interrupt  ssh root@somehost.com "echo \$HOME"
gt ~ sshpass -p mySecretPassword ssh -oStrictHostKeyChecking=no root@somehost.com "echo \$HOME"                             
Warning: Permanently added 'somehost.com,10.92.30.38' (RSA) to the list of known hosts.
/root
There is sctually another way of importing keys - ssh-keyscan command which output has to be appended to ~/.ssh/known_hosts file.
ssh-keyscan -H somehost.com >> ~/.ssh/known_hosts
Example:
gt ~ ssh root@somehost.com          
The authenticity of host 'somehost.com (10.92.30.39)' can't be established.
RSA key fingerprint is 3d:7d:a0:82:d7:3b:60:bc:58:ce:14:d2:bf:1e:d5:89.
Are you sure you want to continue connecting (yes/no)? ^C
zsh: interrupt  ssh root@somehost.com
gt ~ ssh-keyscan -H somehost.com >> ~/.ssh/known_hosts  
# somehost.com SSH-2.0-OpenSSH_5.3
# somehost.com SSH-2.0-OpenSSH_5.3
no hostkey alg
gt ~ ssh root@somehost.com 
Warning: Permanently added the RSA host key for IP address '10.92.30.39' to the list of known hosts.
root@somehost.com's password: 
Last login: Thu May 14 15:24:06 2015 from 10.154.8.71
[root@somehost ~]#
Value returned by the command invoked on remote host can obviously be assigned to some variable in bash script:
gt ~ cat script.sh
#!/bin/bash
REMOTE_HOME=$(sshpass -p arthur ssh -oStrictHostKeyChecking=no root@somehost.com "echo \$HOME")
echo "remote home = ${REMOTE_HOME}"
gt ~ ./script.sh
remote home = /root
As you can see both problems can be solved quite easily but you should realize that this kind of hacks (sshpass) shouldn't be used in production environment. Actually I use this kind of scripts which pass password in plain text only in test environments which aren't directly connected to the internet. Generally such machines are used only for snapshots' testing and don't store any crucial data. You should definitely read this part of sshpass man page:
SECURITY CONSIDERATIONS

First and foremost, users of sshpass should realize that ssh's insistance on only getting the password interactively is not without reason. 
It is close to impossible to securely store the password, and users of sshpass should consider whether ssh's public key authentication provides the same end-user experience, while involving less hassle and being more secure.

The -p option should be considered the least secure of all of sshpass's options. 
All system users can see the password in the command line with a simple "ps" command. Sshpass makes a minimal attempt to hide the password, but such attempts are doomed to create race conditions without actually solving the problem. 
Users of sshpass are encouraged to use one of the other password passing techniques, which are all more secure.

In particular, people writing programs that are meant to communicate the password programatically are encouraged 
to use an anonymous pipe and pass the pipe's reading end to sshpass using the -d option. 

Monday, 27 April 2015

[Spring] How to create global exception handler ?

Sometimes you may want to handle exceptions globally. Let's say you have a service which exposes several rest endpoints which are conntected to the same database. Your data access objects may throw DatabaseConnectionException (or something like that) when db is not available. In such case the response should contain proper HTTP status code and error message. Sample rest endpoint:
@RestController
public class TemperatureEndpoint {
 @Inject
 private WeatherService temperatureService;

 @RequestMapping(
  value = "/temperature/{city}",
  method = RequestMethod.GET,
  produces = "application/json"
 )
 @ResponseBody
 public Temperature currentTemperature(@PathVariable("city") String city) {
  return temperatureService.getCurrentTemperatureIn(city);
 }
}
And another one:
@RestController
public class HumidityEndpoint {
 @Inject
 private HumidityService humidityService;

 @RequestMapping(
  value = "/humidity/{city}",
  method = RequestMethod.GET,
  produces = "application/json"
 )
 @ResponseBody
 public Humidity currentHumidity(@PathVariable("city") String city) {
  return humidityService.getCurrentHumidityIn(city);
 }
}
Both HumidityService and TemperatureService use the same database so you may use some unified runtime exception like DatabaseConnectionException. Handling the exception in both services isn't a good idea because the result will in most cases be the same. By the way each try-catch block makes you to write additional unit test so global handler seems to be a natural choice. Spring allows you to create a global exception handler (will be applied to all the rest controllers) using @ControllerAdvice annotation. Typically class annotated with ControllerAdvice contains methods which map exceptions to HTTP status codes and messages. In well-designed applications it can be the only place where exception handling occurs. Let's create some mappings. Each mapper should also generate proper json response so the client can handle it. Let's create a class annotated with @ControllerAdvice (note that the class has to be in scope of component scan).
@ControllerAdvice
public class WeatherServiceExceptionHandler {
        private static final Logger LOG = LoggerFactory.getLogger(WeatherServiceExceptionHandler.class);
}
I've added a logger because we may want to log something. Let's get back to DatabaseCommunicationException. In case the exception is thrown I want my service to log proper information and return exception's message in json so the client can handle it properly.
@ResponseStatus(value = HttpStatus.INTERNAL_SERVER_ERROR)
@ExceptionHandler(DatabaseCommunicationException.class)
public void handleDatabaseConnectionException(DatabaseCommunicationException e) { 
 LOG.error("DatabaseCommunicationException occurred", e);
}
As you can see I've created a method which takes as a parameter DatabaseCommunicationException so I can log its content. @ExceptionHandler annotation tells Spring that the method handles DatabaseCommunicationException exception. @ResponseStatus allows you to specify which HTTP status will be assigned to the response when this particular handler is being fired. In this case WeatherService cannot work without database connection so I chose 500 which is internal server error. You can obviously use whichever code you want to. For now the handler only logs a message and returns http code but it should also return a message to the client. In all RestControllers I use jackson to map POJO cleassess to json. It can be used in ControllerAdvice as well. Jackson dependency:
<dependency>
 <groupId>com.fasterxml.jackson.core</groupId>
 <artifactId>jackson-databind</artifactId>
</dependency>
In this example json response will only contain a message but you can add here anything else. My response is just a sample immutable POJO. It looks as follows:
public class ErrorMessage {
 private final String message;

 public ErrorMessage(String message) {
  this.message = message;
 }

 public String getMessage() {
  return message;
 }
}
Now the handler should return ErrorMessage and indicate that response should be converted into json format. Complete method looks like this:
@ResponseStatus(value = HttpStatus.INTERNAL_SERVER_ERROR)
@ExceptionHandler(DatabaseCommunicationException.class)
@ResponseBody
public ErrorMessage handleDatabaseConnectionException(DatabaseCommunicationException e) {
 LOG.error("DatabaseCommunicationException occurred", e);
 return new ErrorMessage(e.getMessage());
}
Let's add another one. This time I want to tell the user that the service doesn't support requested city. Fo instance the user wants to get current temperature in Goszowice (which is very small village in southern Poland) but the database simply doesn't contain any data about this place. In such case TemperatureService throws UnsupportedCityException. I guess HttpStatus.NOT_FOUND (404) fits best.
@ResponseStatus(value = HttpStatus.NOT_FOUND)
@ExceptionHandler(UnsupportedCityException.class)
@ResponseBody
public ErrorMessage handleUnsupportedCityException(UnsupportedCityException e) {
 LOG.error("UnsupportedCityException occurred", e);
 return new ErrorMessage(e.getMessage());
}
As you can see bodies of both handlers are almost the same so it can be extracted to new method. It's just an example so let it be as it is. Complete code:
@ControllerAdvice
public class WeatherServiceExceptionHandler {
 private static final Logger LOG = LoggerFactory.getLogger(WeatherServiceExceptionHandler.class);

 @ResponseStatus(value = HttpStatus.INTERNAL_SERVER_ERROR)
 @ExceptionHandler(DatabaseCommunicationException.class)
 @ResponseBody
 public ErrorMessage handleDatabaseConnectionException(DatabaseCommunicationException e) {
  LOG.error("DatabaseCommunicationException occurred", e);
  return new ErrorMessage(e.getMessage());
 }

 @ResponseStatus(value = HttpStatus.NOT_FOUND)
 @ExceptionHandler(UnsupportedCityException.class)
 @ResponseBody
 public ErrorMessage handleUnsupportedCityException(UnsupportedCityException e) {
  LOG.error("UnsupportedCityException occurred", e);
  return new ErrorMessage(e.getMessage());
 }
}
You can obviously provide a better way of constructing error message because you may not want to expose your internal messages. Something like this looks a bit better:
private static final ImmutableMap<Class, String> EXCEPTION_TO_MESSAGE_MAP = ImmutableMap.<Class, String>of(
   DatabaseCommunicationException.class, "Service is not available. Please try again later.",
   UnsupportedCityException.class, "Service does not track temperature in requested city."
);
That would be all :) Cheers