You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by GitBox <gi...@apache.org> on 2021/03/30 14:37:08 UTC

[GitHub] [knox] pzampino opened a new pull request #426: KNOX-2562 - TokenStateService getTokenMetadata method should throw Un…

pzampino opened a new pull request #426:
URL: https://github.com/apache/knox/pull/426


   …knownTokenException
   
   ## What changes were proposed in this pull request?
   
   To align with other methods in the TokenStateService interface, the getTokenMetadata(String tokenId) method has modified to throw UnknownTokenException when there is no metadata for the specified token identifier.
   
   ## How was this patch tested?
   
   Existing tests have been modified/executed, and I've added org.apache.knox.gateway.services.token.impl.DefaultTokenStateServiceTest#testGetMetadata_InvalidToken(), which also gets invoked for the Alias- and Journal-based TokenStateService implementations.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] pzampino commented on a change in pull request #426: KNOX-2562 - TokenStateService getTokenMetadata method should throw Un…

Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #426:
URL: https://github.com/apache/knox/pull/426#discussion_r604354398



##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/AliasBasedTokenStateService.java
##########
@@ -446,13 +446,15 @@ protected void addMetadataInMemory(String tokenId, TokenMetadata metadata) {
   }
 
   @Override
-  public TokenMetadata getTokenMetadata(String tokenId) {
+  public TokenMetadata getTokenMetadata(String tokenId) throws UnknownTokenException {
     TokenMetadata tokenMetadata = super.getTokenMetadata(tokenId);

Review comment:
       Do you think we need to log the fact that it's missing from the in-memory record? I think not, but I'm interested in your opinion.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] pzampino commented on a change in pull request #426: KNOX-2562 - TokenStateService getTokenMetadata method should throw Un…

Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #426:
URL: https://github.com/apache/knox/pull/426#discussion_r604354398



##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/AliasBasedTokenStateService.java
##########
@@ -446,13 +446,15 @@ protected void addMetadataInMemory(String tokenId, TokenMetadata metadata) {
   }
 
   @Override
-  public TokenMetadata getTokenMetadata(String tokenId) {
+  public TokenMetadata getTokenMetadata(String tokenId) throws UnknownTokenException {
     TokenMetadata tokenMetadata = super.getTokenMetadata(tokenId);

Review comment:
       Do you think we need to log the fact that it's missing from the in-memory record? I think not, but I'm interested in your opinion.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] pzampino merged pull request #426: KNOX-2562 - TokenStateService getTokenMetadata method should throw Un…

Posted by GitBox <gi...@apache.org>.
pzampino merged pull request #426:
URL: https://github.com/apache/knox/pull/426


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [knox] smolnar82 commented on a change in pull request #426: KNOX-2562 - TokenStateService getTokenMetadata method should throw Un…

Posted by GitBox <gi...@apache.org>.
smolnar82 commented on a change in pull request #426:
URL: https://github.com/apache/knox/pull/426#discussion_r604318035



##########
File path: gateway-server/src/main/java/org/apache/knox/gateway/services/token/impl/AliasBasedTokenStateService.java
##########
@@ -446,13 +446,15 @@ protected void addMetadataInMemory(String tokenId, TokenMetadata metadata) {
   }
 
   @Override
-  public TokenMetadata getTokenMetadata(String tokenId) {
+  public TokenMetadata getTokenMetadata(String tokenId) throws UnknownTokenException {
     TokenMetadata tokenMetadata = super.getTokenMetadata(tokenId);

Review comment:
       UnknownTokenException should be handled. In HA case it may happen that the in-memory collection will not have the given token metadata -> super will throw UTE. We should catch it and continue with the alias service action.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org