This is an automated email from the ASF dual-hosted git repository.
tbonelee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new 2c448a3b2d [ZEPPELIN-6484] Deduplicate IdHashes and add unit tests
2c448a3b2d is described below
commit 2c448a3b2dd3868112235efafac58e7810a41ed0
Author: dae won <[email protected]>
AuthorDate: Thu Aug 6 11:09:19 2026 +0900
[ZEPPELIN-6484] Deduplicate IdHashes and add unit tests
### What is this PR for?
`IdHashes` existed twice with identical behaviour —
`org.apache.zeppelin.util` in `zeppelin-interpreter` and
`org.apache.zeppelin.notebook.utility` in `zeppelin-server` — and neither copy
had any tests.
The ticket left the scope open between "tests only" and "dedupe plus
tests". This PR takes the dedupe path: `notebook.utility` held nothing but that
one file, and `Note.java` was the only thing importing it, so removing it is a
file deletion and an import switch. Splitting that into a second ticket would
cost more than it saves.
`org.apache.zeppelin.util.IdHashes` is kept as the canonical copy, matching
the module dependency direction — `zeppelin-server` already depends on
`zeppelin-interpreter`, not the other way round.
The new `IdHashesTest` pins what `generateId()` guarantees: the character
set (digits 1-9 and A-Z without I, L and O, left out so IDs stay unambiguous
when read by a person), non-emptiness, and uniqueness. `encode()` is private,
so all three go through `generateId()`. The expected character set is spelled
out in the test rather than read back from `IdHashes`, so a change to the
dictionary shows up as a failure instead of being silently followed.
Note IDs are unaffected: `IdHashes` only mints IDs and never parses them,
and the two copies produced identical output, so existing notes keep reading
their stored IDs as before.
### What type of PR is it?
Improvement
### Todos
* [x] - Add `IdHashesTest` against the canonical
`org.apache.zeppelin.util.IdHashes`
* [x] - Remove the `zeppelin-server` `notebook.utility` copy and switch
`Note.java` to the canonical import
* [x] - Rebuild the shaded chain and confirm `zeppelin-server` still builds
### What is the Jira issue?
* [ZEPPELIN-6484](https://issues.apache.org/jira/browse/ZEPPELIN-6484)
### How should this be tested?
New tests:
```
./mvnw package -pl zeppelin-interpreter --am -Dtest=IdHashesTest
-DfailIfNoTests=false
```
`Tests run: 3, Failures: 0, Errors: 0, Skipped: 0`
Dedupe, per the ticket's verification note — rebuild the shaded chain, then
build `zeppelin-server`:
```
./mvnw clean package -pl zeppelin-interpreter,zeppelin-interpreter-shaded
--am -DskipTests
./mvnw package -pl zeppelin-server --am
-Dtest='NoteTest,InterpreterSettingTest' -DfailIfNoTests=false
```
Both succeed. `NoteTest` (8 tests) covers the switched import directly,
since the `Note` constructor is what calls `IdHashes.generateId()`;
`InterpreterSettingTest` (12 tests) covers the canonical copy's other caller.
`apache-rat` reports no unapproved licenses.
### Screenshots (if appropriate)
N/A
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
Closes #5379 from big-cir/ZEPPELIN-6484.
Signed-off-by: ChanHo Lee <[email protected]>
---
.../org/apache/zeppelin/util/IdHashesTest.java | 74 +++++++++++++++++++++
.../java/org/apache/zeppelin/notebook/Note.java | 2 +-
.../apache/zeppelin/notebook/utility/IdHashes.java | 76 ----------------------
3 files changed, 75 insertions(+), 77 deletions(-)
diff --git
a/zeppelin-interpreter/src/test/java/org/apache/zeppelin/util/IdHashesTest.java
b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/util/IdHashesTest.java
new file mode 100644
index 0000000000..5298126f76
--- /dev/null
+++
b/zeppelin-interpreter/src/test/java/org/apache/zeppelin/util/IdHashesTest.java
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zeppelin.util;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import org.junit.jupiter.api.Test;
+
+class IdHashesTest {
+
+ /**
+ * The characters generated IDs are expected to be built from: digits 1-9
and A-Z without
+ * I, L and O. Those three letters and the digit 0 are left out so that an
ID stays
+ * unambiguous when a person reads it off a URL.
+ *
+ * <p>Spelled out here rather than read back from IdHashes so that changing
the dictionary
+ * has to be a deliberate act that updates this test too.
+ */
+ private static final String EXPECTED_CHARACTERS =
"123456789ABCDEFGHJKMNPQRSTUVWXYZ";
+
+ /**
+ * IDs are derived from {@code currentTimeMillis() +
SecureRandom.nextInt()}, so uniqueness
+ * can only be asserted probabilistically. The random term spans 2^32
values, which puts the
+ * chance of a collision within one sample of this size on the order of 1e-4.
+ */
+ private static final int SAMPLE_SIZE = 1000;
+
+ @Test
+ void generatedIdsContainOnlyDictionaryCharacters() {
+ for (int i = 0; i < SAMPLE_SIZE; i++) {
+ String id = IdHashes.generateId();
+ for (char c : id.toCharArray()) {
+ assertTrue(EXPECTED_CHARACTERS.indexOf(c) >= 0,
+ "generated id '" + id + "' contains '" + c + "', which is not in
the dictionary");
+ }
+ }
+ }
+
+ @Test
+ void generatedIdsAreNeverEmpty() {
+ for (int i = 0; i < SAMPLE_SIZE; i++) {
+ assertFalse(IdHashes.generateId().isEmpty(), "generateId() returned an
empty id");
+ }
+ }
+
+ @Test
+ void generatedIdsAreDistinctAcrossManyCalls() {
+ Set<String> ids = new HashSet<>();
+ for (int i = 0; i < SAMPLE_SIZE; i++) {
+ ids.add(IdHashes.generateId());
+ }
+ assertEquals(SAMPLE_SIZE, ids.size(), "generateId() produced duplicate
ids");
+ }
+}
diff --git
a/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/Note.java
b/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/Note.java
index 8b7622e1ef..278fa43ecc 100644
--- a/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/Note.java
+++ b/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/Note.java
@@ -36,11 +36,11 @@ import
org.apache.zeppelin.interpreter.ManagedInterpreterGroup;
import org.apache.zeppelin.interpreter.remote.RemoteAngularObject;
import org.apache.zeppelin.interpreter.remote.RemoteAngularObjectRegistry;
import org.apache.zeppelin.interpreter.thrift.InterpreterCompletion;
-import org.apache.zeppelin.notebook.utility.IdHashes;
import org.apache.zeppelin.scheduler.ExecutorFactory;
import org.apache.zeppelin.scheduler.Job.Status;
import org.apache.zeppelin.user.AuthenticationInfo;
import org.apache.zeppelin.user.Credentials;
+import org.apache.zeppelin.util.IdHashes;
import org.apache.zeppelin.util.Util;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
diff --git
a/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/utility/IdHashes.java
b/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/utility/IdHashes.java
deleted file mode 100644
index 7b0d804de9..0000000000
---
a/zeppelin-server/src/main/java/org/apache/zeppelin/notebook/utility/IdHashes.java
+++ /dev/null
@@ -1,76 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.zeppelin.notebook.utility;
-
-import java.math.BigInteger;
-import java.security.SecureRandom;
-import java.util.ArrayList;
-import java.util.List;
-
-/**
- * Generate Tiny ID.
- */
-public class IdHashes {
- private static final char[] DICTIONARY = new char[] {'1', '2', '3', '4',
'5', '6', '7', '8', '9',
- 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'J', 'K', 'M', 'N', 'P', 'Q', 'R',
'S', 'T', 'U', 'V',
- 'W', 'X', 'Y', 'Z'};
-
- /**
- * encodes the given string into the base of the dictionary provided in the
constructor.
- *
- * @param value the number to encode.
- * @return the encoded string.
- */
- private static String encode(Long value) {
-
- List<Character> result = new ArrayList<>();
- BigInteger base = new BigInteger("" + DICTIONARY.length);
- int exponent = 1;
- BigInteger remaining = new BigInteger(value.toString());
- while (true) {
- BigInteger a = base.pow(exponent); // 16^1 = 16
- BigInteger b = remaining.mod(a); // 119 % 16 = 7 | 112 % 256 = 112
- BigInteger c = base.pow(exponent - 1);
- BigInteger d = b.divide(c);
-
- // if d > dictionary.length, we have a problem. but BigInteger doesnt
have
- // a greater than method :-( hope for the best. theoretically, d is
always
- // an index of the dictionary!
- result.add(DICTIONARY[d.intValue()]);
- remaining = remaining.subtract(b); // 119 - 7 = 112 | 112 - 112 = 0
-
- // finished?
- if (remaining.equals(BigInteger.ZERO)) {
- break;
- }
-
- exponent++;
- }
-
- // need to reverse it, since the start of the list contains the least
significant values
- StringBuffer sb = new StringBuffer();
- for (int i = result.size() - 1; i >= 0; i--) {
- sb.append(result.get(i));
- }
- return sb.toString();
- }
-
- public static String generateId() {
- return encode(System.currentTimeMillis() + new SecureRandom().nextInt());
- }
-}
