Skip to content

Commit 86e94f3

Browse files
authored
[ZEPPELIN-6242] Refactor initAuth in Neo4jConnectionManager for safer enum parsing and default handling
### What is this PR for? This PR improves the Neo4j authentication initialization logic by providing a default authentication type of `NONE` when the `neo4j.auth.type` property is absent. It enhances enum parsing with explicit try-catch handling to deliver clear, user-friendly error messages for unsupported or invalid authentication types. Additionally, a defensive default branch is not removed in the switch statement to satisfy the compiler and guard against unforeseen enum values. ### What type of PR is it? Refactoring ### Todos * [x] Provide default `NONE` value when `neo4j.auth.type` is missing * [x] Wrap enum parsing in try-catch with explicit error messages * [X] Add unit test for the modified code. ### What is the Jira issue? * [ZEPPELIN-6242](https://issues.apache.org/jira/browse/ZEPPELIN-6242) ### How should this be tested? * Add new unit test for the modified code. * Tests for missing auth type defaulting to NONE, invalid auth types throwing clear exceptions, and successful BASIC/NONE authentications should be confirmed ### 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 #4975 from eunhwa99/ZEPPELIN-6242. Signed-off-by: Philipp Dallig <philipp.dallig@gmail.com>
1 parent 573fc4e commit 86e94f3

2 files changed

Lines changed: 107 additions & 3 deletions

File tree

neo4j/src/main/java/org/apache/zeppelin/graph/neo4j/Neo4jConnectionManager.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,16 @@ public Neo4jConnectionManager(Properties properties) {
8585
}
8686

8787
private AuthToken initAuth(Properties properties) {
88-
String authType = properties.getProperty(NEO4J_AUTH_TYPE);
89-
switch (Neo4jAuthType.valueOf(authType.toUpperCase())) {
88+
String authType = properties.getProperty(NEO4J_AUTH_TYPE, "NONE").toUpperCase();
89+
90+
Neo4jAuthType neo4jAuthType;
91+
try {
92+
neo4jAuthType = Neo4jAuthType.valueOf(authType);
93+
} catch (IllegalArgumentException e) {
94+
throw new IllegalArgumentException("Unsupported Neo4j authentication type: " + authType, e);
95+
}
96+
97+
switch (neo4jAuthType) {
9098
case BASIC:
9199
String username = properties.getProperty(NEO4J_AUTH_USER);
92100
String password = properties.getProperty(NEO4J_AUTH_PASSWORD);
@@ -95,7 +103,7 @@ private AuthToken initAuth(Properties properties) {
95103
LOGGER.debug("Creating NONE authentication");
96104
return AuthTokens.none();
97105
default:
98-
throw new RuntimeException("Neo4j authentication type not supported");
106+
throw new RuntimeException("Unexpected Neo4j authentication type");
99107
}
100108
}
101109

@@ -154,3 +162,4 @@ private Set<String> extractParams(String cypherQuery, Pattern pattern, String re
154162
return keys;
155163
}
156164
}
165+
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.zeppelin.graph.neo4j;
19+
20+
import static org.junit.jupiter.api.Assertions.assertNotNull;
21+
import static org.junit.jupiter.api.Assertions.assertThrows;
22+
import static org.junit.jupiter.api.Assertions.assertTrue;
23+
24+
import java.util.Properties;
25+
import org.junit.jupiter.api.Test;
26+
27+
class Neo4jConnectionManagerTest {
28+
29+
private static final String NEO4J_AUTH_TYPE = "neo4j.auth.type";
30+
private static final String NEO4J_AUTH_USER = "neo4j.auth.user";
31+
private static final String NEO4J_AUTH_PASSWORD = "neo4j.auth.password";
32+
33+
34+
@Test
35+
void testCreateNeo4jConnectionManagerSuccess() {
36+
Properties props = new Properties();
37+
props.setProperty(NEO4J_AUTH_TYPE, "BASIC");
38+
props.setProperty(NEO4J_AUTH_USER, "admin");
39+
props.setProperty(NEO4J_AUTH_PASSWORD, "secret");
40+
41+
Neo4jConnectionManager neo4jConnectionManager = new Neo4jConnectionManager(props);
42+
43+
assertNotNull(neo4jConnectionManager);
44+
}
45+
46+
@Test
47+
void testCreateNeo4jConnectionManagerDefaultNoneAuthWhenMissingTypeSuccess() {
48+
Properties props = new Properties();
49+
props.setProperty(NEO4J_AUTH_USER, "admin");
50+
props.setProperty(NEO4J_AUTH_PASSWORD, "secret");
51+
52+
Neo4jConnectionManager neo4jConnectionManager = new Neo4jConnectionManager(props);
53+
54+
assertNotNull(neo4jConnectionManager);
55+
}
56+
57+
@Test
58+
void testCreateNeo4jConnectionAuthTypeCaseInsensitiveSuccess() {
59+
Properties props = new Properties();
60+
props.setProperty(NEO4J_AUTH_TYPE, "basic"); // lowercase intentionally
61+
props.setProperty(NEO4J_AUTH_USER, "user");
62+
props.setProperty(NEO4J_AUTH_PASSWORD, "pw");
63+
64+
Neo4jConnectionManager neo4jConnectionManager = new Neo4jConnectionManager(props);
65+
66+
assertNotNull(neo4jConnectionManager);
67+
}
68+
69+
@Test
70+
void testCreateNeo4jConnectionManagerInvalidAuthTypeFail() {
71+
Properties props = new Properties();
72+
props.setProperty(NEO4J_AUTH_TYPE, "INVALID");
73+
props.setProperty(NEO4J_AUTH_USER, "admin");
74+
props.setProperty(NEO4J_AUTH_PASSWORD, "secret");
75+
76+
Exception exception = assertThrows(IllegalArgumentException.class, () ->
77+
new Neo4jConnectionManager(props)
78+
);
79+
80+
assertTrue(exception.getMessage().contains("Unsupported Neo4j authentication type: INVALID"));
81+
}
82+
83+
@Test
84+
void testCreateNeo4jConnectionWithMissingUsernameFail() {
85+
Properties props = new Properties();
86+
props.setProperty(NEO4J_AUTH_TYPE, "BASIC");
87+
props.setProperty(NEO4J_AUTH_PASSWORD, "secret");
88+
89+
Exception exception = assertThrows(NullPointerException.class, () ->
90+
new Neo4jConnectionManager(props)
91+
);
92+
93+
assertTrue(exception.getMessage().contains("Username can't be null"));
94+
}
95+
}

0 commit comments

Comments
 (0)