MODOKAPFAC-3: implementation of GET tenant interfaces endpoint #5
Conversation
…anager and tenant entitlement manager
…tening - in package names and in config properties names.
…nce mod-okapi-facade will be behind Sidecar, and we rely on Sidecar to provide us with a proper token.
There was a problem hiding this comment.
Looks good, except few things:
- Naming of clients\configurations: from my viewpoint, it should be like
MgrTenantEntitlementsClient|MgrTenantEntitlementsClientConfigurationorTenantEntitlementsClientConfiguration, becauseApplicationManagerandTenantManagerare legacy names for these components, and for old modules they could be named like that, but it's because of lack of time to make names proper.
| @Value("${application.mte.querylimit:500}") private int entitlementsQueryLimit = 500; | ||
| @Value("${application.ma.querylimit:500}") private int applicationsQueryLimit = 500; |
There was a problem hiding this comment.
Can be moved to configuration class
There was a problem hiding this comment.
Not sure if having 2 config properties warrants a whole separate configuration class. Will degrade readability.
| return interfaceDescriptors.map(!isFull ? mapper::mapSimple : mapper::map); | ||
| } | ||
|
|
||
| private Predicate<? super org.folio.common.domain.model.InterfaceDescriptor> getFilter(String interfaceType) { |
There was a problem hiding this comment.
It should be in the import section
import org.folio.common.domain.model.InterfaceDescriptor;
...
private Predicate<? super InterfaceDescriptor> ...
There was a problem hiding this comment.
No, it can't be in import section because there is already a class named InterfaceDescriptor there. We have two classes with same name - one for OKAPI compatible API, and one for Eureka-compatible API.
| } | ||
|
|
||
| private Stream<InterfaceDescriptor> map( | ||
| Stream<org.folio.common.domain.model.InterfaceDescriptor> interfaceDescriptors, boolean isFull) { |
There was a problem hiding this comment.
| Stream<org.folio.common.domain.model.InterfaceDescriptor> interfaceDescriptors, boolean isFull) { | |
| Stream<InterfaceDescriptor> interfaceDescriptors, boolean isFull) { |
There was a problem hiding this comment.
No, it can't be in import section because there is already a class named InterfaceDescriptor there. We have two classes with same name - one for OKAPI compatible API, and one for Eureka-compatible API.
src/main/java/org/folio/okapi/facade/service/tenant/TenantInterfacesService.java
Outdated
Show resolved
Hide resolved
src/main/java/org/folio/okapi/facade/service/tenant/TenantInterfacesService.java
Outdated
Show resolved
Hide resolved
…chains, static imports for Collectors
… manager applications (e.g. MgrTenantsClient instead of TenantsManager).
mykolam-epm
changed the title
|
Purpose
Implementation of
GET /_/proxy/<tenant name>/interfacesendpointPre-Merge Checklist:
Before merging this PR, please go through the following list and take appropriate actions.
If there are breaking changes, please STOP and consider the following:
Ideally, all the PRs involved in breaking changes would be merged on the same day to avoid breaking the folio-testing
environment. Communication is paramount if that is to be achieved, especially as the number of inter-module and
inter-team dependencies increase.
While it's helpful for reviewers to help identify potential problems, ensuring that it's safe to merge is ultimately the
responsibility of the PR assignee.