[APIM] Fix file path issue#1353
Conversation
📝 WalkthroughWalkthroughThe changes address cross-platform path handling in import-export functionality. In 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
import-export-cli/utils/fileIOUtils.go (1)
416-420: Consider removing the temp file iftmp.Close()fails.
ioutil.TempFilehas already created the file on disk by the timeClose()is called. IfClose()returns an error, the function exits without invoking the cleanup path, leaving the temp file behind. A best-effortos.Remove(tmpPath)on the error branch would keep cleanup symmetric.♻️ Proposed adjustment
// Read the filename and close the file to avoid file locking in windows tmpPath := tmp.Name() if err := tmp.Close(); err != nil { + _ = os.Remove(tmpPath) return "", err, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@import-export-cli/utils/fileIOUtils.go` around lines 416 - 420, When tmp.Close() fails after creating the temp file (created via ioutil.TempFile), ensure the temp file is removed on the error path: call os.Remove(tmpPath) (best-effort, ignore its error) before returning the close error so the temp file is not left behind; update the error branch that currently returns "", err, nil to first attempt os.Remove(tmpPath) and then return the original error.import-export-cli/impl/importAPIPolicy.go (1)
82-94: Usefilepath.JoinfororiginalFilePathto stay consistent with the cross-platform fix.Line 83 still concatenates with a hardcoded
"/". Whilefilepath.Exthappens to tolerate this, the path construction is inconsistent with the platform-independent change just made above and could trip up future readers or any added path operations.♻️ Proposed adjustment
for _, file := range files { - originalFilePath := tmpPath + "/" + file.Name() + originalFilePath := filepath.Join(tmpPath, file.Name()) ext := filepath.Ext(originalFilePath)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@import-export-cli/impl/importAPIPolicy.go` around lines 82 - 94, The path for originalFilePath is built with string concatenation which is not cross-platform; in the loop in importAPIPolicy.go (the code that constructs originalFilePath and computes ext), replace the string join of tmpPath + "/" + file.Name() with filepath.Join(tmpPath, file.Name()) so path handling is consistent with other uses of filepath; update any related variable references (originalFilePath and ext calculation using filepath.Ext(originalFilePath)) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@import-export-cli/impl/importAPIPolicy.go`:
- Around line 82-94: The path for originalFilePath is built with string
concatenation which is not cross-platform; in the loop in importAPIPolicy.go
(the code that constructs originalFilePath and computes ext), replace the string
join of tmpPath + "/" + file.Name() with filepath.Join(tmpPath, file.Name()) so
path handling is consistent with other uses of filepath; update any related
variable references (originalFilePath and ext calculation using
filepath.Ext(originalFilePath)) accordingly.
In `@import-export-cli/utils/fileIOUtils.go`:
- Around line 416-420: When tmp.Close() fails after creating the temp file
(created via ioutil.TempFile), ensure the temp file is removed on the error
path: call os.Remove(tmpPath) (best-effort, ignore its error) before returning
the close error so the temp file is not left behind; update the error branch
that currently returns "", err, nil to first attempt os.Remove(tmpPath) and then
return the original error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 44a273d5-1fd8-456d-88f9-a4d763a557d7
📒 Files selected for processing (2)
import-export-cli/impl/importAPIPolicy.goimport-export-cli/utils/fileIOUtils.go
Related to wso2/api-manager#4963