Groups API #1565
Groups API #1565
Conversation
Add an API to retrieve the group logo
| Path imagePath = null; | ||
| FileTime lastModifiedTime = null; | ||
| if (StringUtils.isNotBlank(logoUUID) && !logoUUID.startsWith("http://") && !logoUUID.startsWith("https//")) { | ||
| imagePath = Resources.findImagePath(logoUUID, |
There was a problem hiding this comment.
Would be better to use a Utility function. Pretty sure there is more than one time this in the code.
Maybe use this https://github.com/geonetwork/core-geonetwork/blob/develop/services/src/main/java/org/fao/geonet/api/records/attachments/AttachmentsApi.java#L298 and we should move that to a Utility class.
There was a problem hiding this comment.
There is already a class to find the mime type in GN, MimeTypeFinder, but it degrades a lot the performance of the detection and it causes ConcurrentModificationException when multiple threads are querying the mime type of the same file.
|
LGTM @juanluisrp, only minor comments. We should discuss in Bolsena some global stuff like ServiceContext.get() null check, i18n for exception (eg. https://github.com/geonetwork/core-geonetwork/blob/develop/services/src/main/java/org/fao/geonet/api/user/AccountCreationApi.java#L92). |
| throw new RuntimeException("ServiceContext not available"); | ||
| } | ||
|
|
||
| GroupRepository groupRepository = context.getBean(GroupRepository.class); |
There was a problem hiding this comment.
@fxprunayre I was discussing with @juanluisrp about using @Autowired instead. Can that cause any issue with multinode feature?
The code in GeoNetwork seem mixed with both methods to get beans. If not a problem for multinode I would prefer we switch to @Autowired.
There was a problem hiding this comment.
Yep, it will be an issue with multinode. It looks like we might be able to define custom scope of Bean and be able to use Autowired even in multinode mode but I'm not really sure how and no support here to make progress on it.
So for the time being, the rule remains:
- use Autowired if a bean is shared by all nodes
- use getBean if not (which is the case most of the time unfortunately)
There was a problem hiding this comment.
For the i18n of the Exceptions, current implementation of LanguageUtils.parseAcceptLanguage doesn't work well because the three letters ISO code stored in the languages bean don't match the ones returned by Locale.getISO3Language. For example, the languagelist contains "fre" for French, but getISO3Language returns "fra".
There was a problem hiding this comment.
doesn't work well because the three letters ISO cod
Yep this is something to discuss and see how we should handle that.
* Retrieve the exception message from a properties file. * Add a log message instead of printing it to the standard output. * Remove an extra space in the exception error message.
|
A couple of changes I've made related with i18n. The messages resource bundle is injected in the controller and I let it to format the message instead of using |
|
This PR needs an update due to conflicts. |
|
@fxprunayre conflicts resolved |
Add an API to retrieve the group logo