Details

Written by Ingo

Created: 04 December 2014

Last Updated: 04 December 2014

Yesterday I encountered a strange bug - programatically generated archives of a Lucene index in the tar format were corrupted during creation. The process finished normally, but occasionally the resulting archive would be broken. Turns out that someone had (with probably good reason once upon a time) created our own implementation of a tar packaging module in Java, based upon the Apache Ant task.

The problem with the original source ode is: the Ant tar task is limited to an individual file size of 8 GByte, though the resulting archive may be far larger. This was fixed in the Apache Compression library v1.4, some time ago, but you would have to use one of the Gnu Tar formats which support unlimited file size.

The workaround for the current problem seems to be: use the default Lucene indexRamBufferSize setting of 16 MByte, so the segment files won't grow to 25 GByte and stay below the 8 GB limit. But a change of the compression module in the near future (preferably using standard open source components instead of homebrew versions) is already planned.

Details

Written by Ingo

Created: 02 November 2014

Last Updated: 02 November 2014

•     Added more tests to ProductionServiceSpec and cleaned up the code.
•     Use Google Guava Ints/Longs.tryParse() to make code more elegant.
•     Use new OptionalResult class instead of new RuntimeException to report problems.   
      (The ProductionController still uses the old problem reporting method via Exception, but is now ready for further refactoring)
•     Add OptionalResult<T> class
     
      This helps reducing the places where problems are communicated via RuntimeException
      and also reduces cases where nullable objects are returned and not properly handled.
     
      The old pattern in Little Goblin is:
      1. if(problem){throw new RuntimeException(errorMessage))
      2. catch RuntimeException e, render error message from e.getMessage (this breaks with unexpected exceptions lacking a proper messageId...)
     
      The new pattern should be:
      1. for each problem: add error to OptionalResult
      2. return OptionalResult and handle valid/invalid case from there
     
      The old pattern mixes serious exceptions with common user errors (for example, using a too-short password),
      it often uses expensive Exception objects (with complete StackTraces) and can only ever communicate one problem.
      You can work around the expensive objects problem by using static exceptions, but it's still ugly.
•     Remove a hasMany connection, which makes it easier to unit test.
•     Use Java 8
•     Changed formatting; add custom toString() to several classes to to improve debugging and test output.
•     Upgrade to Grails 2.4.4 - Closes #97
•     Add Google's Guava library.
•     This lib provides many useful constructs, for example Longs.tryParse(str) which can parse Strings and returns null instead of throwing NumberFormatException in case of invalid input.
•     Started work on ProductionServiceSpec.
•     Refactored ConstraintUnitSpec so it's no longer a parent class for unit tests.
      The idea of a class which could provide all kinds of mocked items for tests was good, but failed to take implementation details into account. Different unit tests would need specialized Item or Product instances and this complexity would care over to the providing class. Also unit tests with TestFor annotation would not necessarily want those classes to be Mock'ed. Current test writing strategy will be to make the test classes as independent as possible. A little bit more redundant, a lot less confusing/complex.
•     Update Tomcat plugin to version 8.
•     Made Creature class abstract as there is no use case for instantiating Creature objects - LG always uses sub classes. 
      Also I think this might make ORM easier.
•     In Item class changed field owner from Creature to PlayerCharacter.   
      Reason: I was having a hard time using creating a unit test that would work with the extending class PlayerCharacter with this field.
      Then I changed Creature to be an abstract class as there won't be any Creature objects instantiated directly anyway.
      Now it looks like the ProductionServiceSpec will run without problems.
      Only drawback: Mobs which extends Creature cannot own Items now. But then was already a need to write new code for Mobs using Items.
•     Fix #93; computeMaxProduction to return correct amount.

Details

Written by Ingo

Created: 10 October 2014

Last Updated: 10 October 2014

Today I had a very puzzling Exception case in a Spock unit test for LittleGoblin:

void "happyPathTest"() {

    given:
    def accountResult = userAccountService.createUserAccount('a username', 'a password', 
'This email address is being protected from spambots. You need JavaScript enabled to view it.')

    expect:
    accountResult.isOkay()

}

was giving an NPE in the line with 'def accountResult'.

After making sure that the userAccountService was not null, I checked the return value of createUserAccount - which looks like this:

package de.dewarim.goblin

import grails.transaction.Transactional

@Transactional
class UserAccountService {

    AccountCreationResult createUserAccount(String username, String password, String email){
        try {
            UserAccount newAccount = new UserAccount(username: username, email: email, userRealName: username)
            newAccount.passwd = password
            newAccount.save()
            Role role = Role.findByName('ROLE_USER')
            UserRole userRole = new UserRole(newAccount, role)
            userRole.save()
            return new AccountCreationResult(userAccount: newAccount)
        }
        catch (Exception e){
            log.debug("Failed to create account: "+e.getMessage())
            return new AccountCreationResult(errorMessage: e.getMessage())
        }
    }
}

So, due to the try...catch block, the method should _always_ return an object, never null.

After searching for quite some time I came upon the JIRA entry:

Unit test of service doesn't work with @Transactional annotation

Turns out, you need to add @Mock([...]) for all domain objects used in the service method along with mocking the transactionManager for the service in the tests setup().

userAccountService.transactionManager = Mock(PlatformTransactionManager) {
    getTransaction(_) >> Mock(TransactionStatus)
}

Details

Written by Ingo

Created: 28 September 2014

Last Updated: 01 October 2014

In the last weeks, I have begun to getting reacquainted with Little Goblin and started to fix the outstanding bugs and issues.

I have moved the code base to Grails 2.4.3, which took some work.

My goal is to get the plugin stable again and then continue work on project Schedim (a game based upon the LittleGoblin framework).

Changes for 0.4.3:

• Fixed JavaScript bug in shop page: jQuery was not being loaded.
• Changed all pages containing inventory to use a better template/layout. (internal improvement, less GSP code)
• Currently buggy: equip items, also see: https://github.com/dewarim/LittleGoblin/issues for outstanding issues.

Thanks to Rainer for pointing out the bugs :)