jt2594838 commented on code in PR #16432:
URL: https://github.com/apache/iotdb/pull/16432#discussion_r2357988262
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/auth/role/LocalFileRoleAccessor.java:
##########
@@ -301,6 +318,7 @@ public static List<String> getEntityStrings(String[] names)
{
}
retList.addAll(set);
}
+ retList.remove("user_id");
Review Comment:
<img width="954" height="281" alt="image"
src="https://github.com/user-attachments/assets/1b8d2ba2-371a-43b4-b2eb-bcdfc660b573";
/>
I think it has already been filtered.
##########
iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/schema/CNPhysicalPlanGenerator.java:
##########
@@ -200,9 +200,12 @@ private void generateUserRolePhysicalPlan(final boolean
isUser) {
int tag = dataInputStream.readInt();
boolean fromOldVersion = tag < 0;
String user;
- if (fromOldVersion) {
+ if (tag < 0) {
user = readString(dataInputStream, STRING_ENCODING, strBufferLocal, -1
* tag);
+ } else if (tag == 1) {
+ user = readString(dataInputStream, STRING_ENCODING, strBufferLocal);
} else {
+ dataInputStream.readLong(); // userId
Review Comment:
Why is not UserId processed in the branch where isUser = true?
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/auth/entity/User.java:
##########
@@ -56,6 +58,13 @@ public User(String name, String password) {
this.roleSet = new HashSet<>();
}
+ public User(String name, String password, long userId) {
+ super(name);
+ this.password = password;
+ this.userId = userId;
+ this.roleSet = new HashSet<>();
+ }
+
Review Comment:
Why not remove the constructor where the userId is not provided?
Is any case legal that creates a user without an ID?
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/auth/role/LocalFileRoleAccessor.java:
##########
@@ -223,6 +223,23 @@ public Role loadEntity(String entityName) throws
IOException {
}
}
+ @Override
+ public long loadUserId() throws IOException {
+ File userIdFile = checkFileAvailable("user_id", "");
+ if (userIdFile == null) {
+ return -1;
+ }
+ FileInputStream inputStream = new FileInputStream(userIdFile);
+ try (DataInputStream dataInputStream =
+ new DataInputStream(new BufferedInputStream(inputStream))) {
+ return dataInputStream.readLong();
+ } catch (Exception e) {
+ throw new IOException(e);
+ } finally {
+ strBufferLocal.remove();
+ }
Review Comment:
How about just storing the number in the file name, which, in my opinion,
would make the logic a little simpler.
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/auth/user/BasicUserManager.java:
##########
@@ -102,6 +106,20 @@ private void initAdmin() throws AuthException {
LOGGER.info("Admin initialized");
}
+ private void initUserId() {
+ try {
+ long maxUserId = this.accessor.loadUserId();
+ if (maxUserId == -1 || maxUserId < 10000) {
+ nextUserId = 10000;
+ } else {
+ nextUserId = maxUserId;
+ }
+ } catch (IOException e) {
+ LOGGER.warn("meet error in load max userId.");
Review Comment:
Give the exception
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/auth/user/BasicUserManager.java:
##########
@@ -128,7 +146,15 @@ public boolean createUser(
}
lock.writeLock(username);
try {
- user = new User(username, enableEncrypt ?
AuthUtils.encryptPassword(password) : password);
+ long userid = 0;
+ if (username.equals("root")) {
Review Comment:
Use a constant.
##########
iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/auth/entity/User.java:
##########
@@ -73,7 +82,15 @@ public void addRole(String roleName) {
roleSet.add(roleName);
}
+ public void setUserId(long userId) {
+ this.userId = userId;
+ }
Review Comment:
Why provide this method? UserId should be immutable.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
