Add TPS TPSCertService to v2 APIs #4827
Conversation
fmarco76
force-pushed
the
TPSCertService_v2
branch
3 times, most recently
from
cb48bdc
to
f00e48f
Compare
base/common/pom.xml
Outdated
| <dependency> | ||
| <groupId>org.apache.tomcat</groupId> | ||
| <artifactId>tomcat-servlet-api</artifactId> | ||
| <version>9.0.62</version> | ||
| </dependency> | ||
|
|
There was a problem hiding this comment.
I think the common library (which is used by the client) should not depend on Servlet API. I'm not sure which code requires Servlet API, but it probably should be moved into the server library.
There was a problem hiding this comment.
OK, I think the problem is with the error codes I have introduced in a previous PR. I'll remove this change and handle it in a separate PR.
| if (filter != null && filter.length() < PKIServlet.MIN_FILTER_LENGTH) { | ||
| throw new BadRequestException("Filter is too short. Must be at least " + PKIServlet.MIN_FILTER_LENGTH + " characters."); | ||
| } | ||
|
|
||
| if(tokenID == null) { | ||
| return findAllCerts(authorizedProfiles, filter, start, size); | ||
| } | ||
|
|
||
| Map<String, String> attributes = new HashMap<>(); | ||
| if (tokenID != null) { | ||
| attributes.put("tokenID", tokenID); | ||
| } |
There was a problem hiding this comment.
IIRC the problem is that the Certs page in TPS Web UI will pull all cert records in order to count the total number of entries so the UI can display the paging links such as Page 1, Page 2, ... Page N. If there are a lot of certs, this page will take a long time to open since by default there's no filter to narrow down the search, so that's why the minimum filter length was added, or if the token ID is available it can also be used to narrow down the search.
I think ideally we should change the UI to no longer require knowing the total entries, so the paging link we need to provide is just Next Page as long as there are more certs to display, then we don't have to require a minimum filter anymore.
There was a problem hiding this comment.
OK, I have update the code to match current implementation and added a TODO to fix this behaviour when the UI will be fixed
fmarco76
force-pushed
the
TPSCertService_v2
branch
from
f00e48f
to
2c17a5b
Compare
fmarco76
force-pushed
the
TPSCertService_v2
branch
from
2c17a5b
to
4af29a0
Compare
|
|
@edewata Thanks! |
@edewata The current implementation requires to specify the filter (see here). In the wiki page for the CLI this is not needed. If I try the same command with the current implementation I get
Additionally, the check on the filter is different to all the other cases.
For the v2 I have modified the filter to make it not mandatory and works like in the wiki page. Is this OK?