Getting a Custom Field value safely

17 December 2015

Written by Jon Bevan

How hard can it be?

So, in our code we retrieve a custom field and we have an issue, and we just get the custom field value right?

final CustomField customField = customFieldManager.getCustomFieldObjectByName("My Multi-select");
final Object value = issue.getCustomFieldValue(customField);

Great, now we're done!

Object?! That's no use...

Hmmm, Object isn't going to be very helpful. Let's assume it's a List<String> because that makes sense for a multiselect right?

final CustomField customField = customFieldManager.getCustomFieldObjectByName("My Multi-select");

@SuppressWarnings("unchecked")
final List<String> value = (List<String>) issue.getCustomFieldValue(customField);
log.debug(value.get(0));

What could possibly go wrong?

NullPointerException

Ah yes, that. Wait... does Java even have pointers?!

So it turns out we forgot to assign a value to our custom field in JIRA. Oops.

Let's fix that "edge case" up quickly.

final CustomField customField = customFieldManager.getCustomFieldObjectByName("My Multi-select");

@SuppressWarnings("unchecked")
final List<String> value = (List<String>) issue.getCustomFieldValue(customField);
if (value != null) {
    log.debug("No value assigned to custom field 'My Multi-select' on issue {}", issue.getKey());
} else {
    log.debug(value.get(0));
}

Alright, sorted. Nice.

ClassCastException

java.lang.ClassCastException: com.atlassian.jira.issue.customfields.option.LazyLoadedOption cannot be
cast to java.lang.String

D'oh! Well, at least we know what's really returned by the Issue#getCustomFieldValue for a multi-select now!

OK, so we can just cast to the right kind of List now right?

NB: Annoyingly, Java won't throw a ClassCastException here if the returned List is of a different type the type its being cast to.

final CustomField customField = customFieldManager.getCustomFieldObjectByName("My Multi-select");

@SuppressWarnings("unchecked")
final List<LazyLoadedOption> value = (List<LazyLoadedOption>) issue.getCustomFieldValue(customField);
if (value != null) {
    log.debug("No value assigned to custom field 'My Multi-select' on issue {}", issue.getKey());
} else {
    final LazyLoadedOption opt = value.get(0);
    log.debug(opt.getValue());
}

Great, let's tidy this up into a useful method!

Refactoring

/**
 * Useful method for retrieving multi-select custom field value as List<String> 
 */
@Nonnull
public List<String> getMultiSelectFieldValue(@Nonnull String fieldName, @Nonnull Issue issue) {
    Validate.notNull(fieldName);
    Validate.notNull(issue);
    final CustomField customField = customFieldManager.getCustomFieldObjectByName(fieldName);

    // Let's use the Option interface here as well instead of a specific implementation
    @SuppressWarnings("unchecked")
    final List<Option> value = (List<Option>) issue.getCustomFieldValue(customField);
    if (value == null) {
        log.debug("No value assigned to custom field 'My Multi-select' on issue {}", issue.getKey());
        return Lists.newArrayList();
    }
    // Oooo, Java 8! Shiny!
    return value.stream()
            .map(Option::getValue)
            .collect(Collectors.toList());
}

Now we're clearly finished right?

Do you trust your JIRA Admin?

Don't.

Remember, rule 101 about external input to computer programs?

You can't trust anyone!

How is our shiny new method going to cope if the custom field we've told it to use isn't a multi-select?

java.lang.ClassCastException: java.lang.String cannot be cast to java.util.List

Our good friend ClassCastException again!

Let's get defensive

Let's check we actually have what we're expecting!

/**
 * Useful method for retrieving multi-select custom field value as List<String> 
 */
@Nonnull
public List<String> getMultiSelectFieldValue(@Nonnull String fieldName, @Nonnull Issue issue) {
    Validate.notNull(fieldName);
    Validate.notNull(issue);
    final CustomField customField = customFieldManager.getCustomFieldObjectByName(fieldName);

    // Let's use the Option interface here as well instead of a specific implementation
    @SuppressWarnings("unchecked")
    final List<Option> value = (List<Option>) issue.getCustomFieldValue(customField);
    // Handle NullPointerException
    if (value == null) {
        log.debug(
            "No value assigned to custom field '{}' on issue {}. Returning empty list.",
            customField, issue.getKey()
        );
        return Lists.newArrayList();
    }
    // Handle non-list return values
    if (!(value instanceof List)) {
        log.debug(
            "Value of custom field '{}' on issue {} was not a List. Returning empty list.",
            customField, issue.getKey()
        );
        return Lists.newArrayList();
    }
    // If it's empty, lets just return a new empty string list and forget about the origin type
    if (value.isEmpty()) {
        return Lists.newArrayList();
    }
    // Handle potential ClassCastException for lists of any other kind, like Label
    if (!(value.get(0) instanceof Option)) {
        log.debug(
            "Value of custom field '{}' on issue {} was not a List<Option>. Returning empty list.",
            customField, issue.getKey()
        );
        return Lists.newArrayList();
    }
    // Oooo, Java 8! Shiny!
    return value.stream()
            .map(Option::getValue)
            .collect(Collectors.toList());
}

Overkill?

That depends on how you want failures to be treated within your code.

The code above essentially fails silently, trying to recover from all possible scenarios by returning an empty data set. This may work perfectly in some scenarios, but be a disaster in others.

As the developer you need to work with the customer to determine how each of these edge cases should be handled. Most of the additional code in the example above handles exceptional circumstances, so perhaps throwing an Exception would be the correct thing to do - it just often feels like a nasty user experience if the UI breaks and displays a huge stack trace.

At least with the code above the behaviour degrades gracefully and if a user isn't seeing what they expect they can flag that up with someone who can probably ask someone else to look at the logs and find out what's going wrong.

At the very least as developers we should be asking the questions that led us to this result, rather than only coding the happy path.

blog comments powered by Disqus