-
Notifications
You must be signed in to change notification settings - Fork 527
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
support: invalid cache through rpc #1357
Conversation
Change-Id: I3fe18cd8c893a84a56fb66df651b78c5e0fe5f4b
Change-Id: I2f8edb2c70b1ad185f66da527af0e34557c6c889
Change-Id: I4e4abf3b71a6182e8f16af1cd17f60c2690c0767
Change-Id: I8723f401ee430519bc488554c31da16e4fdffdb4
Change-Id: Iefde073a4f07b5bad4cc08f065a3ee0f034114d8
Change-Id: I9c6c5f5e069d2e6f4aad9ec7adbb3dba26de2460
Change-Id: Id45b11844ed5c0a7e6cfb672057c42b455119ba3
46f1cc6
to
578a16c
Compare
Codecov Report
@@ Coverage Diff @@
## master #1357 +/- ##
============================================
- Coverage 69.19% 62.08% -7.12%
- Complexity 5401 5835 +434
============================================
Files 328 386 +58
Lines 26334 32169 +5835
Branches 3750 4492 +742
============================================
+ Hits 18222 19972 +1750
- Misses 6333 10168 +3835
- Partials 1779 2029 +250
Continue to review full report at Codecov.
|
this.tokens.put(USER_ADMIN, config.get(ServerOptions.ADMIN_TOKEN)); | ||
this.tokens.putAll(config.getMap(ServerOptions.USER_TOKENS)); | ||
this.tokens.put(USER_ADMIN, config.get(ServerOptions.AUTH_ADMIN_TOKEN)); | ||
this.tokens.putAll(config.getMap(ServerOptions.AUTH_USER_TOKENS)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need check AUTH_USER_TOKENS doesn't contains user named 'admin'?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can change their order to ensure the admin stays in the map
String rpcUrl = conf.get(ServerOptions.RPC_REMOTE_URL); | ||
String selfUrl = conf.get(ServerOptions.RPC_SERVER_HOST) + ":" + | ||
conf.get(ServerOptions.RPC_SERVER_PORT); | ||
rpcUrl = execludeSelfUrl(rpcUrl, selfUrl); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exclude
} | ||
|
||
if (responses.size() > 0) { | ||
// Just choose one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
} | ||
|
||
private static String methodName(SofaRequest request) { | ||
String method = request.getInterfaceName() + "." + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just return is enough
|
||
public void invalid(HugeType type, Id id); | ||
|
||
public void invalid2(HugeType type, Object[] ids); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any friendship name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sofa-rpc don't allowed to use the same method name in a service, may be names like invalidBatch() is more friendship, here just to keep the name short.
Change-Id: I23878e7991774dbb3dcc7723137bac314502cc68
Change-Id: Ia671437ae51f88f8eebb3fcdac20321af65d10a9
return config.refer(); | ||
private static String excludeSelfUrl(String rpcUrl, String selfUrl) { | ||
String[] urls = StringUtils.splitWithCommaOrSemicolon(rpcUrl); | ||
Set<String> urlsSet = new LinkedHashSet<>(Arrays.asList(urls)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
urlSet
} else if (excepts.size() > 0) { | ||
throw excepts.get(0); | ||
} else { | ||
assert providers.isEmpty(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check providers.isEmpty() in line 135
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
empty provider must come here
Change-Id: Ic24ef99b74edf8e95b701bb6509f127a5d4eee91
ec012eb
to
988480e
Compare
Change-Id: I8a8266d7d618410dcff2e882dcbc71c9d71f66e5
3039823
to
8da4366
Compare
Change-Id: I3fe18cd8c893a84a56fb66df651b78c5e0fe5f4b