Skip to content
This repository has been archived by the owner on Jul 25, 2018. It is now read-only.

feat(export): make lists and maps in spreadsheet export machine-readable #398

Merged
merged 1 commit into from
May 12, 2017

Conversation

alexbrdn
Copy link
Contributor

make lists and maps in spreadsheet export machine-readable by exporting as JSON

closes #353

Copy link
Member

@maxhbr maxhbr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good up to minor comments, which can be solved later

.collect(Collectors.toList());
return joinStrings(mapEntriesAsStrings);
Map<String, Object> originalMap = nullToEmptyMap(((Map<String, Object>) fieldValue));
Map<String, String> map = Maps.transformValues(originalMap, v -> v != null ? v.toString() : "");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the function you have implemented here as lambda is already in the system as nullToEmptyString

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops! That's embarrassing. I'll fix it.

try {
return mapper.writeValueAsString(value);
} catch (JsonProcessingException e) {
throw new IllegalArgumentException(String.format("Cannot serialize field value %s to JSON", value), e);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be better to use no RuntimeException here, maybe use instead SW360Exception.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please log the exception with log.error(msg,e)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right.

}
return fieldValue.toString();
}

private static String serializeToJson(Object value) {
ObjectMapper mapper = new ObjectMapper();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might improve performance to move the ObjectMapper to a static field

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a seldom used function. I wouldn't care about this performance gain at this point.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spreadsheet Export for Projects should stringify complex objects
3 participants