Skip to content

Commit e83b2b4

Browse files
pratapaditya04pratapaditya04
committed
refactor
1 parent 03eeb93 commit e83b2b4

File tree

3 files changed

+91
-38
lines changed

3 files changed

+91
-38
lines changed

‎gobblin-yarn/src/main/java/org/apache/gobblin/yarn/JarCachePathResolver.java‎

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
package org.apache.gobblin.yarn;
1919

2020
import java.io.IOException;
21-
import java.text.SimpleDateFormat;
2221

2322
import org.apache.hadoop.fs.FileSystem;
2423
import org.apache.hadoop.fs.Path;
@@ -32,51 +31,53 @@
3231

3332

3433
/**
35-
* Resolves the jar cache directory path by validating filesystem state and applying fallback logic.
36-
*
37-
* <p>This class separates the concern of computing the resolved jar cache directory from config initialization,
38-
* making it easier to debug and test. The resolution happens lazily when {@link #resolveJarCachePath()} is called.</p>
34+
* Utility class for resolving the jar cache directory path by validating filesystem on path existence and applying fallback logic.
3935
*
4036
* <p>Resolution logic:</p>
4137
* <ol>
4238
* <li>If JAR_CACHE_DIR is explicitly configured, uses it as-is (for backward compatibility)</li>
4339
* <li>Otherwise, validates JAR_CACHE_ROOT_DIR exists on filesystem</li>
4440
* <li>If not found, tries FALLBACK_JAR_CACHE_ROOT_DIR</li>
45-
* <li>Combines validated root with JAR_CACHE_SUFFIX to form final path</li>
41+
* <li>Combines validated root with JAR_CACHE_SUFFIX (or default suffix) to form final path</li>
4642
* <li>If no valid root found, throws IOException</li>
4743
* </ol>
4844
*/
4945
public class JarCachePathResolver {
5046
private static final Logger LOGGER = LoggerFactory.getLogger(JarCachePathResolver.class);
47+
// Note: Trailing slash will be normalized away by Hadoop Path
48+
private static final String DEFAULT_JAR_CACHE_SUFFIX = ".gobblinCache/gobblin-temporal";
5149

52-
private final Config config;
53-
private final FileSystem fs;
54-
55-
public JarCachePathResolver(Config config, FileSystem fs) {
56-
this.config = config;
57-
this.fs = fs;
50+
// Private constructor to prevent instantiation
51+
private JarCachePathResolver() {
5852
}
5953

6054
/**
6155
* Resolves the jar cache directory path, applying validation and fallback logic.
6256
*
57+
* @param config the configuration
58+
* @param fs the filesystem to use for validation
6359
* @return the resolved jar cache directory path
6460
* @throws IOException if filesystem operations fail or no valid root directory is found
6561
*/
66-
public Path resolveJarCachePath() throws IOException {
67-
// If JAR_CACHE_DIR is explicitly set, use it as-is (backward compatibility)
62+
public static Path resolveJarCachePath(Config config, FileSystem fs) throws IOException {
63+
// If JAR_CACHE_DIR is explicitly set, use it as-is
6864
if (config.hasPath(GobblinYarnConfigurationKeys.JAR_CACHE_DIR)) {
6965
String explicitCacheDir = config.getString(GobblinYarnConfigurationKeys.JAR_CACHE_DIR);
7066
LOGGER.info("Using explicitly configured JAR_CACHE_DIR: {}", explicitCacheDir);
7167
return new Path(explicitCacheDir);
7268
}
7369

70+
// Get suffix from config, or use default if not configured or empty
7471
String suffix = ConfigUtils.getString(config, GobblinYarnConfigurationKeys.JAR_CACHE_SUFFIX, "");
72+
if (suffix == null || suffix.trim().isEmpty()) {
73+
LOGGER.info("JAR_CACHE_SUFFIX not configured or empty, using default: {}", DEFAULT_JAR_CACHE_SUFFIX);
74+
suffix = DEFAULT_JAR_CACHE_SUFFIX;
75+
}
7576

7677
// Try primary root directory
7778
if (config.hasPath(GobblinYarnConfigurationKeys.JAR_CACHE_ROOT_DIR)) {
7879
String rootDir = config.getString(GobblinYarnConfigurationKeys.JAR_CACHE_ROOT_DIR);
79-
Path resolvedPath = validateAndComputePath(rootDir, suffix, GobblinYarnConfigurationKeys.JAR_CACHE_ROOT_DIR);
80+
Path resolvedPath = validateAndComputePath(fs, rootDir, suffix, GobblinYarnConfigurationKeys.JAR_CACHE_ROOT_DIR);
8081
if (resolvedPath != null) {
8182
return resolvedPath;
8283
}
@@ -85,31 +86,32 @@ public Path resolveJarCachePath() throws IOException {
8586
// Try fallback root directory
8687
if (config.hasPath(GobblinYarnConfigurationKeys.FALLBACK_JAR_CACHE_ROOT_DIR)) {
8788
String fallbackRootDir = config.getString(GobblinYarnConfigurationKeys.FALLBACK_JAR_CACHE_ROOT_DIR);
88-
Path resolvedPath = validateAndComputePath(fallbackRootDir, suffix, GobblinYarnConfigurationKeys.FALLBACK_JAR_CACHE_ROOT_DIR);
89+
Path resolvedPath = validateAndComputePath(fs, fallbackRootDir, suffix, GobblinYarnConfigurationKeys.FALLBACK_JAR_CACHE_ROOT_DIR);
8990
if (resolvedPath != null) {
9091
return resolvedPath;
9192
}
9293
}
9394

94-
// No valid root directory found - fail fast
95-
throw new IOException("No valid jar cache root directory found. Please configure "
96-
+ GobblinYarnConfigurationKeys.JAR_CACHE_ROOT_DIR + " or "
97-
+ GobblinYarnConfigurationKeys.FALLBACK_JAR_CACHE_ROOT_DIR
98-
+ " with a valid directory path, or explicitly set "
95+
// No valid root directory found - fail
96+
throw new IOException("No valid jar cache root directory found. Please configure "
97+
+ GobblinYarnConfigurationKeys.JAR_CACHE_ROOT_DIR + " or "
98+
+ GobblinYarnConfigurationKeys.FALLBACK_JAR_CACHE_ROOT_DIR
99+
+ " with a valid directory path, or explicitly set "
99100
+ GobblinYarnConfigurationKeys.JAR_CACHE_DIR);
100101
}
101102

102103
/**
103104
* Validates if the root directory exists and computes the full path with suffix.
104105
*
106+
* @param fs the filesystem to check
105107
* @param rootDir the root directory to validate
106108
* @param suffix the suffix to append
107109
* @param configName the config name for logging
108110
* @return the computed path if valid, null otherwise
109111
* @throws IOException if filesystem operations fail
110112
*/
111113
@VisibleForTesting
112-
Path validateAndComputePath(String rootDir, String suffix, String configName) throws IOException {
114+
static Path validateAndComputePath(FileSystem fs, String rootDir, String suffix, String configName) throws IOException {
113115
Path rootPath = new Path(rootDir);
114116
if (fs.exists(rootPath)) {
115117
// Strip leading '/' from suffix to ensure proper concatenation

‎gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnHelixUtils.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,7 @@ public static void setYarnClassPath(Config config, Configuration yarnConfigurati
216216
*/
217217
public static Path calculatePerMonthJarCachePath(Config config, FileSystem fs) throws IOException {
218218
// Use JarCachePathResolver to resolve the base jar cache directory
219-
JarCachePathResolver resolver = new JarCachePathResolver(config, fs);
220-
Path baseCacheDir = resolver.resolveJarCachePath();
219+
Path baseCacheDir = JarCachePathResolver.resolveJarCachePath(config, fs);
221220

222221
// Append monthly suffix
223222
String monthSuffix = new SimpleDateFormat("yyyy-MM").format(

‎gobblin-yarn/src/test/java/org/apache/gobblin/yarn/JarCachePathResolverTest.java‎

Lines changed: 66 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,7 @@ public void testResolveJarCachePath_ExplicitJarCacheDir() throws IOException {
4949
.withValue(GobblinYarnConfigurationKeys.YARN_APPLICATION_LAUNCHER_START_TIME_KEY,
5050
ConfigValueFactory.fromAnyRef(System.currentTimeMillis()));
5151

52-
JarCachePathResolver resolver = new JarCachePathResolver(config, mockFs);
53-
Path result = resolver.resolveJarCachePath();
52+
Path result = JarCachePathResolver.resolveJarCachePath(config, mockFs);
5453

5554
// Should use explicitly configured JAR_CACHE_DIR
5655
Assert.assertEquals(result.toString(), explicitCacheDir);
@@ -81,8 +80,7 @@ public Boolean answer(InvocationOnMock invocation) {
8180
.withValue(GobblinYarnConfigurationKeys.YARN_APPLICATION_LAUNCHER_START_TIME_KEY,
8281
ConfigValueFactory.fromAnyRef(System.currentTimeMillis()));
8382

84-
JarCachePathResolver resolver = new JarCachePathResolver(config, mockFs);
85-
Path result = resolver.resolveJarCachePath();
83+
Path result = JarCachePathResolver.resolveJarCachePath(config, mockFs);
8684

8785
// Should resolve to root + suffix
8886
Assert.assertEquals(result.toString(), expectedFullPath);
@@ -114,8 +112,7 @@ public Boolean answer(InvocationOnMock invocation) {
114112
.withValue(GobblinYarnConfigurationKeys.YARN_APPLICATION_LAUNCHER_START_TIME_KEY,
115113
ConfigValueFactory.fromAnyRef(System.currentTimeMillis()));
116114

117-
JarCachePathResolver resolver = new JarCachePathResolver(config, mockFs);
118-
Path result = resolver.resolveJarCachePath();
115+
Path result = JarCachePathResolver.resolveJarCachePath(config, mockFs);
119116

120117
// Should resolve to fallback root + suffix
121118
Assert.assertEquals(result.toString(), expectedFullPath);
@@ -140,9 +137,8 @@ public void testResolveJarCachePath_NeitherRootDirExists() throws IOException {
140137
.withValue(GobblinYarnConfigurationKeys.YARN_APPLICATION_LAUNCHER_START_TIME_KEY,
141138
ConfigValueFactory.fromAnyRef(System.currentTimeMillis()));
142139

143-
JarCachePathResolver resolver = new JarCachePathResolver(config, mockFs);
144140
// Should throw IOException when no valid root directory found
145-
resolver.resolveJarCachePath();
141+
JarCachePathResolver.resolveJarCachePath(config, mockFs);
146142
}
147143

148144
@Test
@@ -168,8 +164,7 @@ public Boolean answer(InvocationOnMock invocation) {
168164
.withValue(GobblinYarnConfigurationKeys.YARN_APPLICATION_LAUNCHER_START_TIME_KEY,
169165
ConfigValueFactory.fromAnyRef(System.currentTimeMillis()));
170166

171-
JarCachePathResolver resolver = new JarCachePathResolver(config, mockFs);
172-
Path result = resolver.resolveJarCachePath();
167+
Path result = JarCachePathResolver.resolveJarCachePath(config, mockFs);
173168

174169
// Should resolve to fallback root + suffix
175170
Assert.assertEquals(result.toString(), expectedFullPath);
@@ -185,9 +180,67 @@ public void testResolveJarCachePath_NoRootDirsConfigured() throws IOException {
185180
.withValue(GobblinYarnConfigurationKeys.YARN_APPLICATION_LAUNCHER_START_TIME_KEY,
186181
ConfigValueFactory.fromAnyRef(System.currentTimeMillis()));
187182

188-
JarCachePathResolver resolver = new JarCachePathResolver(config, mockFs);
189183
// Should throw IOException when no root directories are configured
190-
resolver.resolveJarCachePath();
184+
JarCachePathResolver.resolveJarCachePath(config, mockFs);
185+
}
186+
187+
@Test
188+
public void testResolveJarCachePath_DefaultSuffix() throws IOException {
189+
FileSystem mockFs = Mockito.mock(FileSystem.class);
190+
String rootDir = "/user/testuser";
191+
// Note: Hadoop Path normalizes and removes trailing slashes
192+
String expectedFullPath = "/user/testuser/.gobblinCache/gobblin-temporal";
193+
194+
// Mock: Root directory exists
195+
Mockito.when(mockFs.exists(Mockito.any(Path.class))).thenAnswer(new Answer<Boolean>() {
196+
@Override
197+
public Boolean answer(InvocationOnMock invocation) {
198+
Path path = invocation.getArgument(0);
199+
return path.toString().equals(rootDir);
200+
}
201+
});
202+
203+
// Config without JAR_CACHE_SUFFIX - should use default
204+
Config config = ConfigFactory.empty()
205+
.withValue(GobblinYarnConfigurationKeys.JAR_CACHE_ROOT_DIR, ConfigValueFactory.fromAnyRef(rootDir))
206+
.withValue(GobblinYarnConfigurationKeys.JAR_CACHE_ENABLED, ConfigValueFactory.fromAnyRef(true))
207+
.withValue(GobblinYarnConfigurationKeys.YARN_APPLICATION_LAUNCHER_START_TIME_KEY,
208+
ConfigValueFactory.fromAnyRef(System.currentTimeMillis()));
209+
210+
Path result = JarCachePathResolver.resolveJarCachePath(config, mockFs);
211+
212+
// Should use default suffix
213+
Assert.assertEquals(result.toString(), expectedFullPath);
214+
}
215+
216+
@Test
217+
public void testResolveJarCachePath_EmptySuffixUsesDefault() throws IOException {
218+
FileSystem mockFs = Mockito.mock(FileSystem.class);
219+
String rootDir = "/user/testuser";
220+
// Note: Hadoop Path normalizes and removes trailing slashes
221+
String expectedFullPath = "/user/testuser/.gobblinCache/gobblin-temporal";
222+
223+
// Mock: Root directory exists
224+
Mockito.when(mockFs.exists(Mockito.any(Path.class))).thenAnswer(new Answer<Boolean>() {
225+
@Override
226+
public Boolean answer(InvocationOnMock invocation) {
227+
Path path = invocation.getArgument(0);
228+
return path.toString().equals(rootDir);
229+
}
230+
});
231+
232+
// Config with empty JAR_CACHE_SUFFIX - should use default
233+
Config config = ConfigFactory.empty()
234+
.withValue(GobblinYarnConfigurationKeys.JAR_CACHE_ROOT_DIR, ConfigValueFactory.fromAnyRef(rootDir))
235+
.withValue(GobblinYarnConfigurationKeys.JAR_CACHE_SUFFIX, ConfigValueFactory.fromAnyRef(""))
236+
.withValue(GobblinYarnConfigurationKeys.JAR_CACHE_ENABLED, ConfigValueFactory.fromAnyRef(true))
237+
.withValue(GobblinYarnConfigurationKeys.YARN_APPLICATION_LAUNCHER_START_TIME_KEY,
238+
ConfigValueFactory.fromAnyRef(System.currentTimeMillis()));
239+
240+
Path result = JarCachePathResolver.resolveJarCachePath(config, mockFs);
241+
242+
// Should use default suffix when configured suffix is empty
243+
Assert.assertEquals(result.toString(), expectedFullPath);
191244
}
192245

193246
@Test
@@ -213,8 +266,7 @@ public Boolean answer(InvocationOnMock invocation) {
213266
.withValue(GobblinYarnConfigurationKeys.YARN_APPLICATION_LAUNCHER_START_TIME_KEY,
214267
ConfigValueFactory.fromAnyRef(System.currentTimeMillis()));
215268

216-
JarCachePathResolver resolver = new JarCachePathResolver(config, mockFs);
217-
Path result = resolver.resolveJarCachePath();
269+
Path result = JarCachePathResolver.resolveJarCachePath(config, mockFs);
218270

219271
// Should normalize suffix by stripping leading '/' to avoid absolute path issue
220272
Assert.assertEquals(result.toString(), expectedFullPath);

0 commit comments

Comments
 (0)