jMock

Allow ClassImposteriser to be extended

Details

  • Type: Improvement Improvement
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: 2.5.1
  • Fix Version/s: None
  • Component/s: JMock 2.x.x Library
  • Labels:
    None
  • Number of attachments :
    1

Description

I had a need to imposterise concrete classes (from 3rd-party jars) with final toString() method.

For background, please see discussion at:
http://www.nabble.com/Re%3A-mocking-concrete-classes-with-final-toString-method-p18509146.html
In that thread, Nat had advised one of the options as being "write your own Imposteriser".

So, for my needs I ended up writing ClassImposteriserAllowingFinalToString. It would be great if ClassImposteriser could be made easier to extend. As it is, it has a lot of private methods and properties and I pretty much had to rewrite my own imposteriser from scratch (essentially duplicating the private pieces of ClassImposteriser). If ClassImposteriser was extension friendly, I would probably need to do minimal work by writing my own imposterise() and canImposterise() methods. This way I would also be safeguarded against future bug-fixes that may happen in ClassImposteriser.

Cheers and thanks in advance.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Corporate Gadfly added a comment -

Patch against trunk allowing ClassImposteriser to be extended.

Show
Corporate Gadfly added a comment - Patch against trunk allowing ClassImposteriser to be extended.
Hide
Permalink
Nat Pryce added a comment -

That patch just makes all members protected. Making a class extensible needs a bit more design than that if it is going to be easy to maintain in the long run and ensure backward compatibility as we maintain it.

What exactly do you need to change in the ClassImposteriser? Can that be done by passing policies to the ClassImposteriser instead of by inheritance?

Show
Nat Pryce added a comment - That patch just makes all members protected. Making a class extensible needs a bit more design than that if it is going to be easy to maintain in the long run and ensure backward compatibility as we maintain it. What exactly do you need to change in the ClassImposteriser? Can that be done by passing policies to the ClassImposteriser instead of by inheritance?
Hide
Permalink
Corporate Gadfly added a comment -

Hi Nat,

I needed to override the methods canImposterise() and imposterise().

Here is my canImposterise() method:

Mine
public boolean canImposterise(Class<?> type) {
    return !type.isPrimitive() &&
           !Modifier.isFinal(type.getModifiers()) &&
           type.isInterface();
}

as compared to the original:

Original
public boolean canImposterise(Class<?> type) {
    return !type.isPrimitive() && 
           !Modifier.isFinal(type.getModifiers()) && 
           (type.isInterface() || !toStringMethodIsFinal(type));
}

Notice the extra call to toStringMethodIsFinal. So overriding this one is easy.

When I tried to override the imposterise() method, it had calls to private methods. Here's mine:

Mine
public <T> T imposterise(final Invokable mockObject, Class<T> mockedType, Class<?>... ancilliaryTypes) {
    try {
        setConstructorsAccessible(mockedType, true);
        Class<?> proxyClass = createProxyClass(mockedType, ancilliaryTypes);
        return mockedType.cast(createProxy(proxyClass, mockObject));
    }
    finally {
        setConstructorsAccessible(mockedType, false);
    }
}

compared to original:

Original
public <T> T imposterise(final Invokable mockObject, Class<T> mockedType, Class<?>... ancilliaryTypes) {
    if (!mockedType.isInterface() && toStringMethodIsFinal(mockedType)) {
        throw new IllegalArgumentException(mockedType.getName() + " has a final toString method");
    }

    try {
        setConstructorsAccessible(mockedType, true);
        Class<?> proxyClass = createProxyClass(mockedType, ancilliaryTypes);
        return mockedType.cast(createProxy(proxyClass, mockObject));
    }
	finally {
		setConstructorsAccessible(mockedType, false);
	}
}

Overriding this one was a little bit more difficult as it has calls to 3 private methods (createProxyClass, createProxy, setConstructorsAccessible).

Hope the situation is a little bit clearer.

Show
Corporate Gadfly added a comment - Hi Nat, I needed to override the methods canImposterise() and imposterise(). Here is my canImposterise() method:
Mine
public boolean canImposterise(Class<?> type) {
    return !type.isPrimitive() &&
           !Modifier.isFinal(type.getModifiers()) &&
           type.isInterface();
}
as compared to the original:
Original
public boolean canImposterise(Class<?> type) {
    return !type.isPrimitive() && 
           !Modifier.isFinal(type.getModifiers()) && 
           (type.isInterface() || !toStringMethodIsFinal(type));
}
Notice the extra call to toStringMethodIsFinal. So overriding this one is easy. When I tried to override the imposterise() method, it had calls to private methods. Here's mine:
Mine
public <T> T imposterise(final Invokable mockObject, Class<T> mockedType, Class<?>... ancilliaryTypes) {
    try {
        setConstructorsAccessible(mockedType, true);
        Class<?> proxyClass = createProxyClass(mockedType, ancilliaryTypes);
        return mockedType.cast(createProxy(proxyClass, mockObject));
    }
    finally {
        setConstructorsAccessible(mockedType, false);
    }
}
compared to original:
Original
public <T> T imposterise(final Invokable mockObject, Class<T> mockedType, Class<?>... ancilliaryTypes) {
    if (!mockedType.isInterface() && toStringMethodIsFinal(mockedType)) {
        throw new IllegalArgumentException(mockedType.getName() + " has a final toString method");
    }

    try {
        setConstructorsAccessible(mockedType, true);
        Class<?> proxyClass = createProxyClass(mockedType, ancilliaryTypes);
        return mockedType.cast(createProxy(proxyClass, mockObject));
    }
	finally {
		setConstructorsAccessible(mockedType, false);
	}
}
Overriding this one was a little bit more difficult as it has calls to 3 private methods (createProxyClass, createProxy, setConstructorsAccessible). Hope the situation is a little bit clearer.
Hide
Permalink
Nat Pryce added a comment -

So the only thing you need to change is the check for final toString methods, is that right?

Show
Nat Pryce added a comment - So the only thing you need to change is the check for final toString methods, is that right?
Hide
Permalink
Corporate Gadfly added a comment -

That's correct.

Show
Corporate Gadfly added a comment - That's correct.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Codehaus. Try JIRA - bug tracking software for your team.