Skip to content

Commit 098498a

Browse files
pratapaditya04pratapaditya04
committed
refactor
1 parent 03eeb93 commit 098498a

File tree

3 files changed

+85
-38
lines changed

3 files changed

+85
-38
lines changed

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

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -32,51 +32,48 @@
3232

3333

3434
/**
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>
35+
* Utility class for resolving the jar cache directory path by validating filesystem on path existence and applying fallback logic.
3936
*
4037
* <p>Resolution logic:</p>
4138
* <ol>
4239
* <li>If JAR_CACHE_DIR is explicitly configured, uses it as-is (for backward compatibility)</li>
4340
* <li>Otherwise, validates JAR_CACHE_ROOT_DIR exists on filesystem</li>
4441
* <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>
42+
* <li>Combines validated root with JAR_CACHE_SUFFIX (or default suffix) to form final path</li>
4643
* <li>If no valid root found, throws IOException</li>
4744
* </ol>
4845
*/
4946
public class JarCachePathResolver {
5047
private static final Logger LOGGER = LoggerFactory.getLogger(JarCachePathResolver.class);
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

74-
String suffix = ConfigUtils.getString(config, GobblinYarnConfigurationKeys.JAR_CACHE_SUFFIX, "");
70+
// Get suffix from config, or use default if not configured or empty
71+
String suffix = ConfigUtils.getString(config, GobblinYarnConfigurationKeys.JAR_CACHE_SUFFIX, DEFAULT_JAR_CACHE_SUFFIX);
7572

7673
// Try primary root directory
7774
if (config.hasPath(GobblinYarnConfigurationKeys.JAR_CACHE_ROOT_DIR)) {
7875
String rootDir = config.getString(GobblinYarnConfigurationKeys.JAR_CACHE_ROOT_DIR);
79-
Path resolvedPath = validateAndComputePath(rootDir, suffix, GobblinYarnConfigurationKeys.JAR_CACHE_ROOT_DIR);
76+
Path resolvedPath = validateAndComputePath(fs, rootDir, suffix, GobblinYarnConfigurationKeys.JAR_CACHE_ROOT_DIR);
8077
if (resolvedPath != null) {
8178
return resolvedPath;
8279
}
@@ -85,31 +82,32 @@ public Path resolveJarCachePath() throws IOException {
8582
// Try fallback root directory
8683
if (config.hasPath(GobblinYarnConfigurationKeys.FALLBACK_JAR_CACHE_ROOT_DIR)) {
8784
String fallbackRootDir = config.getString(GobblinYarnConfigurationKeys.FALLBACK_JAR_CACHE_ROOT_DIR);
88-
Path resolvedPath = validateAndComputePath(fallbackRootDir, suffix, GobblinYarnConfigurationKeys.FALLBACK_JAR_CACHE_ROOT_DIR);
85+
Path resolvedPath = validateAndComputePath(fs, fallbackRootDir, suffix, GobblinYarnConfigurationKeys.FALLBACK_JAR_CACHE_ROOT_DIR);
8986
if (resolvedPath != null) {
9087
return resolvedPath;
9188
}
9289
}
9390

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 "
91+
// No valid root directory found - fail
92+
throw new IOException("No valid jar cache root directory found. Please configure "
93+
+ GobblinYarnConfigurationKeys.JAR_CACHE_ROOT_DIR + " or "
94+
+ GobblinYarnConfigurationKeys.FALLBACK_JAR_CACHE_ROOT_DIR
95+
+ " with a valid directory path, or explicitly set "
9996
+ GobblinYarnConfigurationKeys.JAR_CACHE_DIR);
10097
}
10198

10299
/**
103100
* Validates if the root directory exists and computes the full path with suffix.
104101
*
102+
* @param fs the filesystem to check
105103
* @param rootDir the root directory to validate
106104
* @param suffix the suffix to append
107105
* @param configName the config name for logging
108106
* @return the computed path if valid, null otherwise
109107
* @throws IOException if filesystem operations fail
110108
*/
111109
@VisibleForTesting
112-
Path validateAndComputePath(String rootDir, String suffix, String configName) throws IOException {
110+
static Path validateAndComputePath(FileSystem fs, String rootDir, String suffix, String configName) throws IOException {
113111
Path rootPath = new Path(rootDir);
114112
if (fs.exists(rootPath)) {
115113
// 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: 64 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,65 @@ 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+
String expectedFullPath = "/user/testuser/.gobblinCache/gobblin-temporal/";
192+
193+
// Mock: Root directory exists
194+
Mockito.when(mockFs.exists(Mockito.any(Path.class))).thenAnswer(new Answer<Boolean>() {
195+
@Override
196+
public Boolean answer(InvocationOnMock invocation) {
197+
Path path = invocation.getArgument(0);
198+
return path.toString().equals(rootDir);
199+
}
200+
});
201+
202+
// Config without JAR_CACHE_SUFFIX - should use default
203+
Config config = ConfigFactory.empty()
204+
.withValue(GobblinYarnConfigurationKeys.JAR_CACHE_ROOT_DIR, ConfigValueFactory.fromAnyRef(rootDir))
205+
.withValue(GobblinYarnConfigurationKeys.JAR_CACHE_ENABLED, ConfigValueFactory.fromAnyRef(true))
206+
.withValue(GobblinYarnConfigurationKeys.YARN_APPLICATION_LAUNCHER_START_TIME_KEY,
207+
ConfigValueFactory.fromAnyRef(System.currentTimeMillis()));
208+
209+
Path result = JarCachePathResolver.resolveJarCachePath(config, mockFs);
210+
211+
// Should use default suffix
212+
Assert.assertEquals(result.toString(), expectedFullPath);
213+
}
214+
215+
@Test
216+
public void testResolveJarCachePath_EmptySuffixUsesDefault() throws IOException {
217+
FileSystem mockFs = Mockito.mock(FileSystem.class);
218+
String rootDir = "/user/testuser";
219+
String expectedFullPath = "/user/testuser/.gobblinCache/gobblin-temporal/";
220+
221+
// Mock: Root directory exists
222+
Mockito.when(mockFs.exists(Mockito.any(Path.class))).thenAnswer(new Answer<Boolean>() {
223+
@Override
224+
public Boolean answer(InvocationOnMock invocation) {
225+
Path path = invocation.getArgument(0);
226+
return path.toString().equals(rootDir);
227+
}
228+
});
229+
230+
// Config with empty JAR_CACHE_SUFFIX - should use default
231+
Config config = ConfigFactory.empty()
232+
.withValue(GobblinYarnConfigurationKeys.JAR_CACHE_ROOT_DIR, ConfigValueFactory.fromAnyRef(rootDir))
233+
.withValue(GobblinYarnConfigurationKeys.JAR_CACHE_SUFFIX, ConfigValueFactory.fromAnyRef(""))
234+
.withValue(GobblinYarnConfigurationKeys.JAR_CACHE_ENABLED, ConfigValueFactory.fromAnyRef(true))
235+
.withValue(GobblinYarnConfigurationKeys.YARN_APPLICATION_LAUNCHER_START_TIME_KEY,
236+
ConfigValueFactory.fromAnyRef(System.currentTimeMillis()));
237+
238+
Path result = JarCachePathResolver.resolveJarCachePath(config, mockFs);
239+
240+
// Should use default suffix when configured suffix is empty
241+
Assert.assertEquals(result.toString(), expectedFullPath);
191242
}
192243

193244
@Test
@@ -213,8 +264,7 @@ public Boolean answer(InvocationOnMock invocation) {
213264
.withValue(GobblinYarnConfigurationKeys.YARN_APPLICATION_LAUNCHER_START_TIME_KEY,
214265
ConfigValueFactory.fromAnyRef(System.currentTimeMillis()));
215266

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

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

0 commit comments

Comments
 (0)