[GitHub] [incubator-streampark] 1996fanrui commented on a diff in pull request #2327: Remove AssertUtils to fix license issues

["1996fanrui (via GitHub)" <gi...@apache.org>]

1996fanrui commented on code in PR #2327:
URL: https://github.com/apache/incubator-streampark/pull/2327#discussion_r1104282826


##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/RoleServiceImpl.java:
##########
@@ -86,7 +86,7 @@ public void deleteRole(Long roleId) {
                 () ->
                     new IllegalArgumentException(String.format("Role id [%s] not found", roleId)));
     List<Long> userIdsByRoleId = memberService.findUserIdsByRoleId(roleId);
-    AssertUtils.isTrue(

Review Comment:
   `Utils.required` will throw `IllegalArgumentException`, but old check isn't.
   
   Too many checks are replaced to the `Utils.required` due to release, so it's better to add a TODO here. In the long run, it is unreasonable for all checks to throw `IllegalArgumentException`.



##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/UserServiceImpl.java:
##########
@@ -238,7 +238,6 @@ public List<User> findByAppOwner(Long teamId) {
    */
   @Override
   public Map<String, Object> generateFrontendUserInfo(User user, Long teamId, JWTToken token) {
-    AssertUtils.checkNotNull(user);

Review Comment:
   Why remove it instead of add a new method here?



##########
streampark-common/src/main/scala/org/apache/streampark/common/util/Utils.scala:
##########
@@ -44,14 +53,20 @@ object Utils {
 
   def isEmpty(elem: Any): Boolean = !notEmpty(elem)
 
-  def uuid(): String = UUID.randomUUID().toString.replaceAll("-", "")
+  def required(expression: Boolean): Unit = {
+    if (!expression) {
+      throw new IllegalArgumentException
+    }
+  }
 
-  def require(requirement: Boolean, message: String): Unit = {
-    if (!requirement) {
-      throw new IllegalArgumentException(s"requirement failed: $message")
+  def required(expression: Boolean, errorMessage: Any): Unit = {

Review Comment:
   How about `checkArgument` due to it throw `IllegalArgumentException` when `expression == false`?



-- 
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: issues-unsubscribe@streampark.apache.org

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