Wednesday, December 12, 2012

PowerMock no last call on a mock available

Seems like every other time I write unit tests with PowerMock I run into this error "IllegalStateException: no last call on a mock available" What it should read is "Hey! You forgot to include in the PrepareForTest attribute a class that is used statically and was mocked with the MockStatic function"
At least that's always the reason I see that error.

Feel free to let me know if there are other reasons it pops up.


Wednesday, December 5, 2012

Race Condition

I have this weird problem where two separate users upload a picture to my web service and user1 see's user2's error messages. Reviewing the code nothing sticks out to me, so I would like to ask what is wrong with this code and why was it creating this condition where errors for user2 would be visible to user1?

Here is some simplified code to try and demonstrate the situation.

    public class SomeService {
       
        private static SomeService service;
        private SomeService() {
        }
       
        public static SomeService getInstance() {
            if(service == null) {
               service = new SomeService();
            }
        }
   
        public ErrorStatus doSomething(ErrorStatus es) {
           
            es = new ErrorStatus(es);
            // stuff happens that causes an error
            es.addMessage(new ErrorMessage("some error happened"));
            return es;
        }
       
        public ErrorStatus doSomethingElse(ErrorStatus es) {
           
            es = new ErrorStatus(es);
            // stuff happens that causes an error
            es.addMessage(new ErrorMessage("some different error happened"));
            return es;
        }
    }
   
    public class ErrorMessage {
        String message;

        //simple constructor, getters and setters, nothing interesting
    }
   
    public class ErrorStatus {
        int id;
        String status;
        List<ErrorMessage> messages;

        public ErrorStatus() {
             id = 0;
             status = "";
             messages = new ArrayList<>();
        }

        public ErrorStatus(ErrorStatus other) {
            id = other.getId();
            status = other.getStatus();
            messages = other.getMessages();
        }
       
        public void addMessage(ErrorMessage message) {
       
            //data checks
            messages.add(message);
        }
       
        //getters and setters
    }   
   
    public class UploadServlet extends HttpServlet {
       
        public doGet(request, response) {
           
            ErrorStatus es = new ErrorStatus();
            SomeService service = SomeService.getInstance();
           
            es = service.doSomething(es);
           
            es = service.doSomethingElse(es);
           
            printErrors(response.getWriter(), es);
           
        }
               
        public void printErrors(PrintWriter pw, ErrorStatus es) {
           
            for(int i = 0; i < es.getMessages().size(); i++) {
               
                pw.write(es.getMessages().get(i).getMessage());
            }
        }
    }

The two places in the code I think something weird could be happening is the copy constructor or the fact that the service is a singleton. Maybe I'm not copying the list correctly, or maybe the service being a singleton changes how the stack and heap are used, I'm really not sure. From my understanding of how the stack, heap, singletons and servlets work one users data would never be affected by another users data.
Also they only part of this that ever had problems was the list, the primitive data was always correct, only the list of errors was ever showing up for the wrong user.

I should note, I have solved the problem I just don't understand why it was a problem. The solution was to stop using the copy constructor and to just let the ErrorStatus object get modified in the doSomething and doSomethingElse methods. So the revised code would have a void return type for doSomething and would just call es.addMessage, also the copy constructor was deleted from ErrorStatus.

Any help understanding why this caused a race condition would be appreciated.

I'll update this post once I find the answer to why this was a problem.

Update 1/8/13:
So in my spare time I tried to reproduce the error programmatically, I wrote a multithreaded test program which would run the code thousands of times, unfortunately I couldn't reproduce the race condition. As I have more spare time I might try a few more things but I'm beginning to think there was something more going on that I didn't include when I simplified the process to post here.