feat: base go mkiso
This commit is contained in:
246
todo/02-genisoimage-to-godiskfs.detail.md
Normal file
246
todo/02-genisoimage-to-godiskfs.detail.md
Normal file
@@ -0,0 +1,246 @@
|
||||
# Detail: genisoimage → go-diskfs
|
||||
|
||||
## 1. `internal/isobuilder/builder.go` — new file
|
||||
|
||||
```go
|
||||
package isobuilder
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
|
||||
diskfs "github.com/diskfs/go-diskfs"
|
||||
"github.com/diskfs/go-diskfs/disk"
|
||||
"github.com/diskfs/go-diskfs/filesystem"
|
||||
"github.com/diskfs/go-diskfs/filesystem/iso9660"
|
||||
)
|
||||
|
||||
// BuildFromDirectory creates an ISO 9660 image from a source directory.
|
||||
// Equivalent to: genisoimage -iso-level 4 -r -J -V <label> -o <isoPath> <srcDir>
|
||||
func BuildFromDirectory(srcDir, isoPath, volumeLabel string) error {
|
||||
// Step 1: Calculate total size (sum of all files + 10% overhead)
|
||||
totalSize, err := dirSize(srcDir)
|
||||
if err != nil {
|
||||
return fmt.Errorf("calculate directory size: %w", err)
|
||||
}
|
||||
|
||||
// Step 2: Create disk image
|
||||
mydisk, err := diskfs.Create(isoPath, totalSize, diskfs.SectorSizeDefault)
|
||||
if err != nil {
|
||||
return fmt.Errorf("create disk image: %w", err)
|
||||
}
|
||||
mydisk.LogicalBlocksize = 2048
|
||||
|
||||
// Step 3: Create ISO 9660 filesystem
|
||||
fs, err := mydisk.CreateFilesystem(disk.FilesystemSpec{
|
||||
Partition: 0,
|
||||
FSType: filesystem.TypeISO9660,
|
||||
VolumeLabel: volumeLabel,
|
||||
})
|
||||
if err != nil {
|
||||
return fmt.Errorf("create ISO filesystem: %w", err)
|
||||
}
|
||||
defer fs.Close()
|
||||
|
||||
// Step 4: Walk source dir and copy files
|
||||
err = filepath.Walk(srcDir, func(path string, info os.FileInfo, err error) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
relPath, err := filepath.Rel(srcDir, path)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if relPath == "." {
|
||||
return nil // skip root
|
||||
}
|
||||
|
||||
if info.IsDir() {
|
||||
if err := fs.Mkdir(relPath); err != nil {
|
||||
return fmt.Errorf("mkdir %q: %w", relPath, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// Regular file — copy contents
|
||||
rw, err := fs.OpenFile(relPath, os.O_CREATE|os.O_RDWR)
|
||||
if err != nil {
|
||||
return fmt.Errorf("open file %q: %w", relPath, err)
|
||||
}
|
||||
defer rw.Close()
|
||||
|
||||
srcFile, err := os.Open(path)
|
||||
if err != nil {
|
||||
return fmt.Errorf("open source %q: %w", path, err)
|
||||
}
|
||||
defer srcFile.Close()
|
||||
|
||||
if _, err := io.Copy(rw, srcFile); err != nil {
|
||||
return fmt.Errorf("copy %q: %w", relPath, err)
|
||||
}
|
||||
|
||||
return nil
|
||||
})
|
||||
if err != nil {
|
||||
return fmt.Errorf("walk source directory: %w", err)
|
||||
}
|
||||
|
||||
// Step 5: Finalize with Rock Ridge + Joliet
|
||||
iso, ok := fs.(*iso9660.FileSystem)
|
||||
if !ok {
|
||||
return fmt.Errorf("not an ISO 9660 filesystem")
|
||||
}
|
||||
if err := iso.Finalize(iso9660.FinalizeOptions{
|
||||
RockRidge: true,
|
||||
Joliet: true,
|
||||
DeepDirectories: true,
|
||||
VolumeIdentifier: volumeLabel,
|
||||
}); err != nil {
|
||||
return fmt.Errorf("finalize ISO: %w", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// dirSize calculates total size of all files in a directory tree,
|
||||
// with 10% overhead margin for ISO metadata.
|
||||
func dirSize(dir string) (int64, error) {
|
||||
var total int64
|
||||
err := filepath.Walk(dir, func(_ string, info os.FileInfo, err error) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if !info.IsDir() {
|
||||
total += info.Size()
|
||||
}
|
||||
return nil
|
||||
})
|
||||
if err != nil {
|
||||
return 0, err
|
||||
}
|
||||
// Add 10% overhead for ISO 9660 metadata
|
||||
total = total + total/10
|
||||
// Minimum 10MB (some PACS studies are small)
|
||||
if total < 10*1024*1024 {
|
||||
total = 10 * 1024 * 1024
|
||||
}
|
||||
// Round up to next 2048-byte sector
|
||||
if total%2048 != 0 {
|
||||
total = total + (2048 - total%2048)
|
||||
}
|
||||
return total, nil
|
||||
}
|
||||
```
|
||||
|
||||
## 2. `internal/config/config.go` — remove Tools field
|
||||
|
||||
**Current (line ~31):**
|
||||
```go
|
||||
type Config struct {
|
||||
Server ServerConfig `yaml:"server"`
|
||||
Auth AuthConfig `yaml:"auth"`
|
||||
DCMTK DCMTKConfig `yaml:"dcmtk"`
|
||||
Tools ToolsConfig `yaml:"tools"`
|
||||
...
|
||||
}
|
||||
```
|
||||
|
||||
**Remove `Tools ToolsConfig` field and entire `ToolsConfig` struct.**
|
||||
|
||||
**Current validation (line ~82):**
|
||||
```go
|
||||
if c.Tools.Genisoimage == "" {
|
||||
return fmt.Errorf("tools.genisoimage is required")
|
||||
}
|
||||
```
|
||||
|
||||
**Remove the above lines.**
|
||||
|
||||
## 3. `config.example.yaml` — remove `tools:` block
|
||||
|
||||
Remove lines:
|
||||
```yaml
|
||||
tools:
|
||||
genisoimage: "/usr/bin/genisoimage"
|
||||
```
|
||||
|
||||
## 4. `internal/service/iso.go` — replace genisoimage call
|
||||
|
||||
**Current in GenerateISO() (line ~68-78):**
|
||||
```go
|
||||
isoPath := filepath.Join(s.cfg.ISO.TempDir, isoName)
|
||||
|
||||
exitCode, stdout, stderr, err := dicom.RunGenISOImage(ctx, s.cfg.Tools.Genisoimage, isoPath, tempDir)
|
||||
if err != nil || exitCode != 0 {
|
||||
cleanup()
|
||||
os.Remove(isoPath)
|
||||
return nil, fmt.Errorf("genisoimage failed (exit %d): %s (stderr: %s)", exitCode, err, stderr)
|
||||
}
|
||||
_ = stdout
|
||||
|
||||
slog.Info("ISO created",
|
||||
"path", isoPath,
|
||||
"accession", accessionNumber,
|
||||
)
|
||||
```
|
||||
|
||||
**Replace with:**
|
||||
```go
|
||||
isoPath := filepath.Join(s.cfg.ISO.TempDir, isoName)
|
||||
|
||||
if err := isobuilder.BuildFromDirectory(tempDir, isoPath, "DICOM"); err != nil {
|
||||
cleanup()
|
||||
os.Remove(isoPath)
|
||||
return nil, fmt.Errorf("ISO creation failed: %w", err)
|
||||
}
|
||||
|
||||
slog.Info("ISO created",
|
||||
"path", isoPath,
|
||||
"accession", accessionNumber,
|
||||
)
|
||||
```
|
||||
|
||||
**Same replacement in GenerateISOMultiple() — find the second `dicom.RunGenISOImage` call and replace identically.**
|
||||
|
||||
Also add import: `"mkiso-server/internal/isobuilder"`
|
||||
|
||||
## 5. `internal/handler/health.go` — remove genisoimage from deps
|
||||
|
||||
**Current (line ~19):**
|
||||
```go
|
||||
deps := []struct {
|
||||
Name string `json:"name"`
|
||||
Path string `json:"path"`
|
||||
Status string `json:"status"`
|
||||
}{
|
||||
{"storescp", cfg.DCMTK.Storescp, ""},
|
||||
{"movescu", cfg.DCMTK.Movescu, ""},
|
||||
{"storescu", cfg.DCMTK.Storescu, ""},
|
||||
{"genisoimage", cfg.Tools.Genisoimage, ""},
|
||||
}
|
||||
```
|
||||
|
||||
**Replace with (remove genisoimage entry, remove `cfg.Tools` reference):**
|
||||
```go
|
||||
deps := []struct {
|
||||
Name string `json:"name"`
|
||||
Path string `json:"path"`
|
||||
Status string `json:"status"`
|
||||
}{
|
||||
{"storescp", cfg.DCMTK.Storescp, ""},
|
||||
{"movescu", cfg.DCMTK.Movescu, ""},
|
||||
{"storescu", cfg.DCMTK.Storescu, ""},
|
||||
}
|
||||
```
|
||||
|
||||
## 6. `pkg/dicom/command.go` — remove `RunGenISOImage`
|
||||
|
||||
Delete the entire `RunGenISOImage` function (lines ~150-180).
|
||||
|
||||
## 7. `go.mod` — after `go get`
|
||||
|
||||
Will add: `github.com/diskfs/go-diskfs v1.9.3` plus its transitive deps.
|
||||
117
todo/02-genisoimage-to-godiskfs.md
Normal file
117
todo/02-genisoimage-to-godiskfs.md
Normal file
@@ -0,0 +1,117 @@
|
||||
# Replace genisoimage with go-diskfs (pure Go ISO creation)
|
||||
|
||||
## Why
|
||||
|
||||
`genisoimage` is the only remaining external binary dependency after the dcmtk migration.
|
||||
Replacing it with `github.com/diskfs/go-diskfs` (pure Go, MIT, 644⭐, v1.9.3) makes the
|
||||
mkiso-server binary fully self-contained — zero external binaries beyond dcmtk storescp/movescu/storescu.
|
||||
|
||||
## What
|
||||
|
||||
Replace `pkg/dicom/command.go`'s `RunGenISOImage()` (which shells out to genisoimage)
|
||||
with pure Go ISO creation using go-diskfs. This affects:
|
||||
|
||||
| Current | New |
|
||||
|---------|-----|
|
||||
| `service/iso.go` calls `dicom.RunGenISOImage()` | Calls `isobuilder.BuildFromDirectory()` |
|
||||
| `pkg/dicom/command.go` has `RunGenISOImage()` | Remove it |
|
||||
| `config.go` has `Tools.Genisoimage` + validation | Remove field + validation |
|
||||
| `config.example.yaml` has `tools.genisoimage` | Remove |
|
||||
| `handler/health.go` checks genisoimage binary | Remove check |
|
||||
|
||||
## go-diskfs API reference
|
||||
|
||||
```go
|
||||
import (
|
||||
diskfs "github.com/diskfs/go-diskfs"
|
||||
"github.com/diskfs/go-diskfs/disk"
|
||||
"github.com/diskfs/go-diskfs/filesystem"
|
||||
"github.com/diskfs/go-diskfs/filesystem/iso9660"
|
||||
)
|
||||
|
||||
type FinalizeOptions struct {
|
||||
RockRidge bool // Rock Ridge extensions (long names, perms)
|
||||
Joliet bool // Joliet (UCS-2 names for Windows)
|
||||
DeepDirectories bool // Allow dirs deeper than 8 levels
|
||||
ElTorito *ElTorito
|
||||
VolumeIdentifier string // Volume label, default "ISOIMAGE"
|
||||
PublisherIdentifier string
|
||||
}
|
||||
```
|
||||
|
||||
### ISO creation pattern (from go-diskfs examples/create-iso-from-folder/)
|
||||
|
||||
```go
|
||||
// 1. Calculate total size of directory
|
||||
folderSize := dirSize(srcDir)
|
||||
|
||||
// 2. Create disk image file
|
||||
mydisk, err := diskfs.Create(isoPath, folderSize, 2048)
|
||||
mydisk.LogicalBlocksize = 2048
|
||||
|
||||
// 3. Create ISO9660 filesystem
|
||||
fs, err := mydisk.CreateFilesystem(disk.FilesystemSpec{
|
||||
Partition: 0,
|
||||
FSType: filesystem.TypeISO9660,
|
||||
VolumeLabel: "DICOM",
|
||||
})
|
||||
|
||||
// 4. Walk source dir, copy files
|
||||
filepath.Walk(srcDir, func(path string, info os.FileInfo, err error) error {
|
||||
relPath, _ := filepath.Rel(srcDir, path)
|
||||
if info.IsDir() {
|
||||
fs.Mkdir(relPath)
|
||||
} else {
|
||||
rw, _ := fs.OpenFile(relPath, os.O_CREATE|os.O_RDWR)
|
||||
io.Copy(rw, file)
|
||||
rw.Close()
|
||||
}
|
||||
})
|
||||
|
||||
// 5. Finalize with Rock Ridge + Joliet
|
||||
iso := fs.(*iso9660.FileSystem)
|
||||
iso.Finalize(iso9660.FinalizeOptions{
|
||||
RockRidge: true,
|
||||
Joliet: true,
|
||||
DeepDirectories: true,
|
||||
VolumeIdentifier: "DICOM",
|
||||
})
|
||||
fs.Close()
|
||||
```
|
||||
|
||||
### Equivalent genisoimage flags to go-diskfs options
|
||||
|
||||
| genisoimage flag | go-diskfs FinalizeOptions |
|
||||
|-----------------|--------------------------|
|
||||
| `-iso-level 4` | `DeepDirectories: true` |
|
||||
| `-r` (Rock Ridge) | `RockRidge: true` |
|
||||
| `-V DICOM` | `VolumeIdentifier: "DICOM"` |
|
||||
| `-allow-multidot` | Implicit via Rock Ridge |
|
||||
| `-allow-lowercase` | Implicit via Rock Ridge |
|
||||
| `-allow-leading-dots` | Implicit via Rock Ridge |
|
||||
| `-J` (Joliet) | `Joliet: true` |
|
||||
|
||||
## Implementation order
|
||||
|
||||
- [ ] **1. Add go-diskfs dependency** — `go get github.com/diskfs/go-diskfs@v1.9.3`
|
||||
- [ ] **2. Create `internal/isobuilder/builder.go`** — `BuildFromDirectory(srcDir, isoPath, volumeLabel string) error`
|
||||
- `dirSize()` helper (walk dir, sum file sizes + 10% overhead margin)
|
||||
- Walk again to create dirs + copy files
|
||||
- Finalize with RockRidge + Joliet + DeepDirectories + VolumeIdentifier
|
||||
- [ ] **3. Update `internal/config/config.go`**
|
||||
- Remove `ToolsConfig.Genisoimage` field
|
||||
- Remove validation `c.Tools.Genisoimage == ""`
|
||||
- Remove `Tools` from Config struct entirely if empty (keep struct, remove field)
|
||||
- [ ] **4. Update `config.example.yaml`** — remove `tools.genisoimage` section
|
||||
- [ ] **5. Update `internal/service/iso.go`**
|
||||
- Replace both `dicom.RunGenISOImage(...)` calls with `isobuilder.BuildFromDirectory(tempDir, isoPath, "DICOM")`
|
||||
- Remove unused `dicom` import alias (if no longer needed in scope — it's still used for `countFiles` elsewhere)
|
||||
- [ ] **6. Update `internal/handler/health.go`**
|
||||
- Remove genisoimage from the dependency check array
|
||||
- [ ] **7. Remove `RunGenISOImage` from `pkg/dicom/command.go`**
|
||||
- Delete the entire function
|
||||
- [ ] **8. Build and test**
|
||||
- `go build ./...` must pass
|
||||
- `go vet ./...` must pass
|
||||
- Start server, verify `/api/health` no longer shows genisoimage
|
||||
- Verify ISO endpoint still returns proper errors (PACS unreachable)
|
||||
190
todo/go-mkiso-implementation.md
Normal file
190
todo/go-mkiso-implementation.md
Normal file
@@ -0,0 +1,190 @@
|
||||
# Go mkiso Server — Implementation Checklist
|
||||
|
||||
## Stage 1 — Plain MVP (no auth)
|
||||
|
||||
Goal: Functional replacement for all three PHP scripts. No security — plain GET endpoints.
|
||||
|
||||
- [ ] 1. **Project scaffolding**
|
||||
- Create `go.mod` (`module mkiso-server`, go 1.22+)
|
||||
- Create directory structure (`internal/{config,route,handler,service,repo}`, `pkg/dicom`)
|
||||
- Create `config.example.yaml` with all documented keys (no secrets)
|
||||
- Create `.gitignore` (ignore `config.yaml`, `*.iso`, temp dirs)
|
||||
|
||||
- [ ] 2. **Config loading** — `internal/config/config.go`
|
||||
- Define `Config` struct matching `docs/go-mkiso-design.md` § config.yaml
|
||||
- Load from YAML (`gopkg.in/yaml.v3`) or JSON (`encoding/json` for zero deps)
|
||||
- Validate required fields on load (pacs host, dcmtk paths)
|
||||
- ⚠️ Skip `auth:` section for Stage 1
|
||||
|
||||
- [ ] 3. **DICOM command runner** — `pkg/dicom/command.go`
|
||||
- `StartStoresCP`, `StopStoresCP`, `RunMoveSCU`, `RunStoreSCU`, `RunGenISOImage`
|
||||
- Port allocation: `allocatePort(basePort, portRange)` — mutex-guarded map
|
||||
- All functions return `(exitCode int, stdout, stderr string, error)`
|
||||
- Logging with slog
|
||||
|
||||
- [ ] 4. **Patient repo** — `internal/repo/patient.go`
|
||||
- HTTP client to patient-data API (see `docs/patient-api-spec.md`)
|
||||
- `ByAccessionNumber(ctx, accessionNumber) → (*PatientData, error)`
|
||||
- Configurable auth header from config (for when patient API itself needs auth)
|
||||
- Timeout + retry with backoff
|
||||
|
||||
- [ ] 5. **DICOM service** — `internal/service/dicom.go`
|
||||
- `FetchDICOM(ctx, accessionNumber, destDir) → (filesCount int, error)`
|
||||
- `FetchDICOMMultiple(ctx, accessionNumbers []string, destDir) → (totalFiles int, error)`
|
||||
- storescp lifecycle: start → wait ready → movescu → stop
|
||||
- `defer` for guaranteed storescp cleanup
|
||||
- ⚠️ After FetchDICOM, check `filesCount > 0`. If 0, return error `"no DICOM data"` before attempting ISO/genisoimage.
|
||||
|
||||
- [ ] 6. **ISO service** — `internal/service/iso.go`
|
||||
- `GenerateISO(ctx, accessionNumber) → (isoPath string, cleanup func(), error)`
|
||||
- `GenerateISOMultiple(ctx, accessionNumbers) → (isoPath string, cleanup func(), error)`
|
||||
- Temp dir + microdicom copy + FetchDICOM + genisoimage
|
||||
- Patient API for multi-accession filename
|
||||
|
||||
- [ ] 7. **Relay service** — `internal/service/relay.go` (see `todo/go-print-relay.detail.md`)
|
||||
- `RelayToCDPublisher(ctx, accessionNumbers) → (*RelayResult, error)`
|
||||
- Patient API validation + FetchDICOM + storescu to CD Publisher
|
||||
- Response includes `patient_name`, `destination`, `files_sent`
|
||||
|
||||
- [ ] 8. **Handlers** — `internal/handler/{health,iso,print}.go`
|
||||
- `GET /api/health` — health check + dependency status
|
||||
- `GET /api/iso/download?accession_number=X` — single ISO download
|
||||
- `GET /api/iso/download-multiple?accession_numbers=X,Y,Z` — multi ISO download
|
||||
- `GET /api/iso/print?accession_number=X` — DICOM relay (auto-detects comma → multi)
|
||||
- ⚠️ Print uses ONE endpoint for both single and multi: comma in `accession_number` triggers multi mode, no `print-multiple` endpoint needed (see pre-flight gap #3)
|
||||
|
||||
- [ ] 9. **Route wiring** — `internal/route/route.go`
|
||||
- Go 1.22+ `http.NewServeMux` with method+path patterns
|
||||
- Request ID middleware (UUID, `X-Request-ID` header)
|
||||
- Recovery middleware (catch panics, log stack, return 500)
|
||||
- **Stage 1**: 4 endpoints — health, download, download-multiple, print (single endpoint, auto-detects commas)
|
||||
- **No auth middleware in Stage 1**
|
||||
|
||||
- [ ] 10. **Main entrypoint** — `main.go`
|
||||
- Load config, wire dependencies, start server
|
||||
- Graceful shutdown (SIGINT/SIGTERM)
|
||||
- `slog.Info("server started", "port", cfg.Server.Port)`
|
||||
|
||||
- [ ] 11. **Stage 1 testing**
|
||||
- Test all four endpoints with curl (no auth header needed)
|
||||
- Test concurrent requests
|
||||
- Test error cases
|
||||
- Test through nginx reverse proxy
|
||||
|
||||
---
|
||||
|
||||
## Stage 2 — Add Authentication
|
||||
|
||||
Goal: Lock down endpoints. **Suggestion below**, implementor picks approach.
|
||||
|
||||
### Recommended: API Key via Reverse Proxy Injection
|
||||
|
||||
**Why**: Zero changes to the HIS `pacs_downloadiso` module. The existing `window.open()` calls continue to work because nginx injects the auth header before proxying to Go.
|
||||
|
||||
```
|
||||
Browser (HIS) nginx (PACS server) Go Server
|
||||
──────────── ─────────────────── ─────────
|
||||
window.open("/mkiso.php?..")
|
||||
──────────── GET ───────────→ add X-API-Key header
|
||||
───── GET + key ──────→ validate key → 200 OK
|
||||
←─── ISO stream ───────
|
||||
←── ISO download ────────────
|
||||
```
|
||||
|
||||
**nginx config:**
|
||||
```nginx
|
||||
location ~ ^/(mkiso|mkiso_multiple|send_rimage_multiple)\.php$ {
|
||||
proxy_set_header X-API-Key "${MKISO_API_KEY}";
|
||||
proxy_pass http://127.0.0.1:8080/api/iso/$1$is_args$args;
|
||||
}
|
||||
|
||||
# Direct API access (for testing/other clients)
|
||||
location /api/ {
|
||||
proxy_pass http://127.0.0.1:8080;
|
||||
}
|
||||
```
|
||||
|
||||
**Go middleware (simple):**
|
||||
```go
|
||||
func APIKey(expectedKey string) func(http.Handler) http.Handler {
|
||||
return func(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.Header.Get("X-API-Key") != expectedKey {
|
||||
http.Error(w, `{"error":"unauthorized"}`, http.StatusUnauthorized)
|
||||
return
|
||||
}
|
||||
next.ServeHTTP(w, r)
|
||||
})
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Pros:**
|
||||
- Zero HIS code changes — nginx handles it
|
||||
- Simple to implement (one string comparison in middleware)
|
||||
- No token expiry, no login endpoint needed
|
||||
- Key rotation = update config.yaml + reload nginx
|
||||
|
||||
**Cons:**
|
||||
- Single shared key (no user-level audit)
|
||||
- Key visible in nginx config
|
||||
- No token expiry (must rotate manually)
|
||||
|
||||
### Alternative Suggestions
|
||||
|
||||
| Approach | Complexity | HIS Changes | Best For |
|
||||
|----------|-----------|-------------|----------|
|
||||
| **API Key (recommended)** | Low | None | Internal hospital network, nginx proxy in place |
|
||||
| **Hardcoded JWT** | Low | None (nginx injects) | When you want expiry + claims but no login flow |
|
||||
| **JWT + Login endpoint** | Medium | Yes (or client script) | Multi-client, need user-level audit |
|
||||
| **IP whitelist** | Low | None | Fixed HIS server IP, simplest possible |
|
||||
| **mTLS** | High | Yes (client cert) | Strictest security, external-facing |
|
||||
|
||||
### Stage 2 Todo Items
|
||||
|
||||
- [ ] 12. **Config additions** — `internal/config/config.go`
|
||||
- Add `auth.api_key` field (the expected key)
|
||||
- Add `auth.enabled` boolean (false in Stage 1 config, true in Stage 2)
|
||||
|
||||
- [ ] 13. **API Key middleware** — `internal/middleware/auth.go`
|
||||
- Extract `X-API-Key` from request header
|
||||
- Compare against config value
|
||||
- Return 401 on mismatch
|
||||
- Skip health endpoint (`/api/health` stays public)
|
||||
|
||||
- [ ] 14. **Route wiring update** — `internal/route/route.go`
|
||||
- Apply API key middleware to all `/api/iso/*` routes (download, download-multiple, print)
|
||||
- Keep `/api/health` public
|
||||
|
||||
- [ ] 15. **nginx config** — deploy reverse proxy with header injection
|
||||
- Map old PHP paths to new Go API paths
|
||||
- Inject `X-API-Key` header
|
||||
- Verify HIS `window.open()` calls work end-to-end
|
||||
|
||||
- [ ] 16. **Stage 2 testing**
|
||||
- Test with valid API key → 200
|
||||
- Test with invalid API key → 401
|
||||
- Test with missing API key → 401
|
||||
- Test health endpoint still public
|
||||
- Test through nginx proxy (key injected) → 200
|
||||
- Test direct access without nginx → 401
|
||||
|
||||
---
|
||||
|
||||
## Stage 3 (Future)
|
||||
|
||||
- [ ] JWT login endpoint (if multi-user audit needed)
|
||||
- [ ] Rate limiting
|
||||
- [ ] Metrics / Prometheus endpoint
|
||||
- [ ] Docker image
|
||||
- [ ] Remove Java/PHP after Go verified in production
|
||||
|
||||
---
|
||||
|
||||
## Dependencies (go.mod)
|
||||
|
||||
| Package | Purpose |
|
||||
|---------|---------|
|
||||
| `gopkg.in/yaml.v3` | YAML config parsing *(only external dep)* |
|
||||
|
||||
Everything else: **Go stdlib only**.
|
||||
367
todo/go-print-relay.detail.md
Normal file
367
todo/go-print-relay.detail.md
Normal file
@@ -0,0 +1,367 @@
|
||||
# Print / DICOM Relay — Implementation Detail
|
||||
|
||||
## PHP Source: `send_rimage_multiple.php`
|
||||
|
||||
The original script has two distinct phases:
|
||||
|
||||
**Phase 1 — DICOM Fetch** (identical to mkiso_multiple.php):
|
||||
1. Parse comma-separated `accession_number` from GET
|
||||
2. DB lookup on HIS DB (`rsabt201107`):
|
||||
- `pacs_result_series` → MEDRECID, RegID (using first accession)
|
||||
- `medrec` → NamaPasien
|
||||
3. Create `/tmp/dicomdir_<uniqid>/DICOMDIR`
|
||||
4. Copy microdicom viewer files
|
||||
5. Nested loop: 14 modalities × N accessions → Java dcmqr C-MOVE
|
||||
|
||||
**Phase 2 — Relay to CD Publisher** (unique to this script):
|
||||
6. `dcmsend +sd +rd 172.16.0.120 104 $dicomdir/DICOMDIR`
|
||||
7. **No genisoimage, no download streaming**
|
||||
8. **No cleanup** (temp dir is leaked — Go version MUST fix this)
|
||||
9. **Bogus headers**: Script sets `Content-Type: application/octet-stream` and `Content-Disposition` with patient-name filename, but never calls `readfile()`. The response body is empty (or dcmsend stdout).
|
||||
|
||||
---
|
||||
|
||||
## Go Implementation
|
||||
|
||||
### Service: `internal/service/relay.go`
|
||||
|
||||
```go
|
||||
package service
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"mkiso-server/pkg/dicom"
|
||||
)
|
||||
|
||||
type RelayService struct {
|
||||
dicomSvc *DicomService
|
||||
patientRepo *repo.PatientRepo // <-- NEW: patient API client
|
||||
cdHost string
|
||||
cdPort int
|
||||
ourAETitle string
|
||||
storescuBin string
|
||||
microdicomSrc string
|
||||
logger *slog.Logger
|
||||
}
|
||||
|
||||
type RelayResult struct {
|
||||
AccessionsSent []string `json:"accessions_sent"`
|
||||
PatientName string `json:"patient_name"` // from patient API
|
||||
Destination string `json:"destination"`
|
||||
FilesSent int `json:"files_sent"`
|
||||
}
|
||||
|
||||
func (s *RelayService) RelayToCDPublisher(
|
||||
ctx context.Context,
|
||||
accessionNumbers []string,
|
||||
) (*RelayResult, error) {
|
||||
// Step 1: Validate accession via patient API (replaces DB lookup)
|
||||
// Use first accession to get patient context
|
||||
patient, err := s.patientRepo.ByAccessionNumber(ctx, accessionNumbers[0])
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("patient API lookup for %q: %w", accessionNumbers[0], err)
|
||||
}
|
||||
s.logger.Info("patient data retrieved for relay",
|
||||
"accession", accessionNumbers[0],
|
||||
"patient", patient.PatientName,
|
||||
)
|
||||
|
||||
// Step 2: Create temp dir
|
||||
tempDir, err := os.MkdirTemp(cfg.TempDir, "dicomdir_")
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("create temp dir: %w", err)
|
||||
}
|
||||
defer os.RemoveAll(tempDir) // GUARANTEED cleanup (fixes PHP leak)
|
||||
|
||||
dicomDir := filepath.Join(tempDir, "DICOMDIR")
|
||||
if err := os.MkdirAll(dicomDir, 0755); err != nil {
|
||||
return nil, fmt.Errorf("create DICOMDIR: %w", err)
|
||||
}
|
||||
|
||||
// Copy microdicom viewer files
|
||||
if err := copyMicrodicom(s.microdicomSrc, tempDir); err != nil {
|
||||
return nil, fmt.Errorf("copy microdicom: %w", err)
|
||||
}
|
||||
|
||||
// Fetch DICOM from PACS (same as download flow)
|
||||
filesRetrieved, err := s.dicomSvc.FetchDICOMMultiple(ctx, accessionNumbers, dicomDir)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("DICOM fetch failed for %v: %w", accessionNumbers, err)
|
||||
}
|
||||
s.logger.Info("DICOM fetched for relay",
|
||||
"accessions", accessionNumbers,
|
||||
"files", filesRetrieved,
|
||||
"dir", dicomDir,
|
||||
)
|
||||
|
||||
// Relay to CD Publisher via storescu
|
||||
destination := fmt.Sprintf("%s:%d", s.cdHost, s.cdPort)
|
||||
exitCode, stdout, stderr, err := dicom.RunStoreSCU(
|
||||
ctx,
|
||||
s.storescuBin,
|
||||
s.ourAETitle,
|
||||
s.cdHost,
|
||||
s.cdPort,
|
||||
dicomDir,
|
||||
)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("storescu failed: %w", err)
|
||||
}
|
||||
if exitCode != 0 {
|
||||
return nil, fmt.Errorf("storescu exit %d: %s", exitCode, stderr)
|
||||
}
|
||||
|
||||
s.logger.Info("DICOM relayed to CD Publisher",
|
||||
"accessions", accessionNumbers,
|
||||
"destination", destination,
|
||||
"files", filesRetrieved,
|
||||
)
|
||||
|
||||
return &RelayResult{
|
||||
AccessionsSent: accessionNumbers,
|
||||
PatientName: patient.PatientName,
|
||||
Destination: destination,
|
||||
FilesSent: filesRetrieved,
|
||||
}, nil
|
||||
}
|
||||
```
|
||||
|
||||
### DICOM Command: `pkg/dicom/command.go` — add `RunStoreSCU`
|
||||
|
||||
```go
|
||||
// RunStoreSCU sends DICOM files to a remote AE via C-STORE.
|
||||
// Equivalent to: storescu -aet <ae> +sd +r <host> <port> <dir>
|
||||
func RunStoreSCU(
|
||||
ctx context.Context,
|
||||
storescuBin, aeTitle, host string,
|
||||
port int,
|
||||
dir string,
|
||||
) (exitCode int, stdout, stderr string, err error) {
|
||||
cmd := exec.CommandContext(ctx,
|
||||
storescuBin,
|
||||
"-aet", aeTitle,
|
||||
"+sd", // scan directories
|
||||
"+r", // recurse
|
||||
host,
|
||||
strconv.Itoa(port),
|
||||
dir,
|
||||
)
|
||||
|
||||
var outBuf, errBuf bytes.Buffer
|
||||
cmd.Stdout = &outBuf
|
||||
cmd.Stderr = &errBuf
|
||||
|
||||
err = cmd.Run()
|
||||
stdout = outBuf.String()
|
||||
stderr = errBuf.String()
|
||||
|
||||
if err != nil {
|
||||
if exitErr, ok := err.(*exec.ExitError); ok {
|
||||
exitCode = exitErr.ExitCode()
|
||||
} else {
|
||||
exitCode = -1
|
||||
}
|
||||
return exitCode, stdout, stderr, err
|
||||
}
|
||||
|
||||
return 0, stdout, stderr, nil
|
||||
}
|
||||
```
|
||||
|
||||
### Handler: `internal/handler/print.go`
|
||||
|
||||
```go
|
||||
func PrintISO(relaySvc *service.RelayService) http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
acc := r.URL.Query().Get("accession_number")
|
||||
if acc == "" {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "missing accession_number"})
|
||||
return
|
||||
}
|
||||
|
||||
result, err := relaySvc.RelayToCDPublisher(r.Context(), []string{acc})
|
||||
if err != nil {
|
||||
slog.Error("print ISO failed", "accession", acc, "error", err)
|
||||
status := http.StatusBadGateway
|
||||
if strings.Contains(err.Error(), "not found") {
|
||||
status = http.StatusNotFound
|
||||
}
|
||||
writeJSON(w, status, map[string]string{
|
||||
"error": "DICOM relay failed",
|
||||
"destination": fmt.Sprintf("%s:%d", cfg.CDPublisher.Host, cfg.CDPublisher.Port),
|
||||
"detail": err.Error(),
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
writeJSON(w, http.StatusOK, map[string]interface{}{
|
||||
"status": "ok",
|
||||
"accessions_sent": result.AccessionsSent,
|
||||
"patient_name": result.PatientName,
|
||||
"destination": result.Destination,
|
||||
"files_sent": result.FilesSent,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func PrintISOMultiple(relaySvc *service.RelayService) http.HandlerFunc {
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
raw := r.URL.Query().Get("accession_numbers")
|
||||
if raw == "" {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "missing accession_numbers"})
|
||||
return
|
||||
}
|
||||
|
||||
accs := strings.Split(raw, ",")
|
||||
for i, a := range accs {
|
||||
accs[i] = strings.TrimSpace(a)
|
||||
if accs[i] == "" {
|
||||
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "empty accession_number in list"})
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
result, err := relaySvc.RelayToCDPublisher(r.Context(), accs)
|
||||
if err != nil {
|
||||
slog.Error("print ISO multiple failed", "accessions", accs, "error", err)
|
||||
writeJSON(w, http.StatusBadGateway, map[string]string{
|
||||
"error": "DICOM relay failed",
|
||||
"destination": fmt.Sprintf("%s:%d", cfg.CDPublisher.Host, cfg.CDPublisher.Port),
|
||||
"detail": err.Error(),
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
writeJSON(w, http.StatusOK, map[string]interface{}{
|
||||
"status": "ok",
|
||||
"accessions_sent": result.AccessionsSent,
|
||||
"destination": result.Destination,
|
||||
"files_sent": result.FilesSent,
|
||||
})
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Config Additions
|
||||
|
||||
```yaml
|
||||
# config.yaml — new cd_publisher block
|
||||
cd_publisher:
|
||||
host: "172.16.0.120"
|
||||
port: 104
|
||||
```
|
||||
|
||||
### Config Struct: `internal/config/config.go`
|
||||
|
||||
```go
|
||||
type Config struct {
|
||||
// ... existing fields ...
|
||||
CDPublisher CDPublisherConfig `yaml:"cd_publisher"`
|
||||
}
|
||||
|
||||
type CDPublisherConfig struct {
|
||||
Host string `yaml:"host"`
|
||||
Port int `yaml:"port"`
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Key Differences from Download Flow
|
||||
|
||||
| Aspect | Download Multiple | Print/Relay |
|
||||
|--------|-------------------|-------------|
|
||||
| Output | ISO file streamed to browser | DICOM sent to CD Publisher |
|
||||
| Response | `Content-Type: application/octet-stream` | `Content-Type: application/json` |
|
||||
| genisoimage | ✅ Required | ❌ Not used |
|
||||
| storescu | ❌ Not used | ✅ Required (to CD Publisher) |
|
||||
| Patient API | ✅ For filename | ✅ For validation + response context |
|
||||
| Filename | `{name}-{accs}.iso` | N/A (JSON response) |
|
||||
| Cleanup | After streaming completes | After relay completes |
|
||||
|
||||
---
|
||||
|
||||
## storescu Command
|
||||
|
||||
Replaces `dcmsend +sd +rd 172.16.0.120 104 $dicomdir/DICOMDIR`:
|
||||
|
||||
```bash
|
||||
/data/dcmtk-bin/storescu \
|
||||
-aet CDRECORD \
|
||||
+sd \ # scan directories
|
||||
+r \ # recurse
|
||||
172.16.0.120 104 \
|
||||
/tmp/dicomdir_xxx/DICOMDIR
|
||||
```
|
||||
|
||||
| Flag | Purpose |
|
||||
|------|---------|
|
||||
| `-aet CDRECORD` | Our AE title (calling) |
|
||||
| `+sd` | Scan subdirectories for DICOM files |
|
||||
| `+r` | Recurse into subdirectories |
|
||||
| `172.16.0.120 104` | CD Publisher host:port |
|
||||
| `/tmp/dicomdir_xxx/DICOMDIR` | Directory containing DICOM files |
|
||||
|
||||
---
|
||||
|
||||
## Error Cases
|
||||
|
||||
| Scenario | Response |
|
||||
|----------|----------|
|
||||
| Invalid accession (patient API 404) | `404 {"error":"accession_number not found"}` |
|
||||
| Patient API unavailable | `502 {"error":"patient API unavailable"}` |
|
||||
| CD Publisher unreachable | `502 {"error":"CD Publisher unreachable","destination":"..."}` |
|
||||
| CD Publisher rejects association | `502 {"error":"DICOM relay failed","detail":"association rejected"}` |
|
||||
| DICOM fetch failed (PACS down) | `502 {"error":"PACS unreachable"}` |
|
||||
| No DICOM files fetched | `502 {"error":"no DICOM data for accession_number"}` |
|
||||
| Partial relay (some files failed) | storescu may return 0 but log warnings — Go should parse stderr |
|
||||
|
||||
---
|
||||
|
||||
## Integration with HIS Frontend
|
||||
|
||||
The `pacs_downloadiso` module JS calls:
|
||||
|
||||
```javascript
|
||||
// printiso() in index.php
|
||||
window.open("http://"+PACS_HOST+"/send_rimage_multiple.php?accession_number="+AccessionNumber);
|
||||
|
||||
// fn_printisomultiple() in index.php
|
||||
window.open("http://"+PACS_HOST+"/send_rimage_multiple.php?accession_number="+an);
|
||||
```
|
||||
|
||||
The Go API returns **JSON**, not an HTML page. The HIS JS uses `window.open()` — it will open a new tab/window showing the JSON.
|
||||
|
||||
**Recommendation:** The nginx proxy should handle this. Or the HIS module could be updated to use AJAX instead of `window.open()` for print. For now, opening a JSON tab is acceptable (shows success/failure to the operator).
|
||||
|
||||
Better: if the `window.open` approach is kept, the Go handler could return a simple HTML page instead of raw JSON for the print endpoint (check `Accept` header or add a `?format=html` query param).
|
||||
|
||||
---
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
# Test single accession relay
|
||||
curl -H "Authorization: Bearer $TOKEN" \
|
||||
"http://localhost:8080/api/iso/print?accession_number=MR.2024.001"
|
||||
|
||||
# Expected: {"status":"ok","accessions_sent":["MR.2024.001"],"patient_name":"JOHN DOE","destination":"172.16.0.120:104","files_sent":42}
|
||||
|
||||
# Test multiple accession relay
|
||||
curl -H "Authorization: Bearer $TOKEN" \
|
||||
"http://localhost:8080/api/iso/print-multiple?accession_numbers=MR.001,CT.002"
|
||||
|
||||
# Test error: invalid accession (patient API returns 404)
|
||||
# Expected: 404 {"error":"accession_number not found"}
|
||||
|
||||
# Test error: CD Publisher offline
|
||||
# Expected: 502 {"error":"CD Publisher unreachable"}
|
||||
|
||||
# Test error: patient API unavailable
|
||||
# Expected: 502 {"error":"patient API unavailable"}
|
||||
|
||||
# Test storescu directly (dev workstation)
|
||||
storescu -aet CDRECORD +sd +r 172.16.0.120 104 /tmp/test_dicom/DICOMDIR
|
||||
```
|
||||
218
todo/mkiso-replace-java-with-dcmtk.detail.md
Normal file
218
todo/mkiso-replace-java-with-dcmtk.detail.md
Normal file
@@ -0,0 +1,218 @@
|
||||
# mkiso dcmtk Replacement — Implementation Details
|
||||
|
||||
## Core Architectural Change
|
||||
|
||||
dcm4che2's `dcmqr` is a **monolithic** tool: one process handles the storage SCP, C-MOVE SCU, query, AND file writing.
|
||||
dcmtk separates these into **two processes** that must run concurrently:
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────────┐
|
||||
│ dcm4che2 dcmqr (single process) │
|
||||
│ ┌─────────────────────────────────────┐│
|
||||
│ │ Storage SCP (listen CDRECORD:10104) ││
|
||||
│ │ C-MOVE SCU ││
|
||||
│ │ File writer (−cstoredest) ││
|
||||
│ └─────────────────────────────────────┘│
|
||||
└─────────────────────────────────────────┘
|
||||
|
||||
┌──────────────┐ ┌──────────────┐
|
||||
│ storescp │ │ movescu │
|
||||
│ AE:CDRECORD │ │ AE:CDRECORD │
|
||||
│ port:10104 │ │ -aem CDRECORD│
|
||||
│ -od DICOMDIR│ │ +P 10104 │
|
||||
└──────┬───────┘ └──────┬───────┘
|
||||
│ │
|
||||
│ receives │ sends C-MOVE-RQ
|
||||
│ C-STORE │ to PACS
|
||||
▼ ▼
|
||||
┌─────────────────────────────┐
|
||||
│ PACS (ABPACS:11112) │
|
||||
└─────────────────────────────┘
|
||||
```
|
||||
|
||||
## Command Templates
|
||||
|
||||
### 1. storescp (background receiver)
|
||||
|
||||
```bash
|
||||
# Start storage SCP in background, capture PID
|
||||
/data/dcmtk-bin/storescp 10104 \
|
||||
-aet CDRECORD \
|
||||
-od "${dicomdir}/DICOMDIR" \
|
||||
+xa \
|
||||
&> /tmp/storescp_${uniq}.log &
|
||||
|
||||
STORESCP_PID=$!
|
||||
|
||||
# Give storescp time to bind the port
|
||||
sleep 1
|
||||
```
|
||||
|
||||
**Key flags:**
|
||||
| Flag | Purpose |
|
||||
|------|---------|
|
||||
| `10104` | Listen port (must match what PACS sends to) |
|
||||
| `-aet CDRECORD` | AE title (must match C-MOVE destination AE) |
|
||||
| `-od DICOMDIR` | Output directory for received files |
|
||||
| `+xa` | Accept all transfer syntaxes (max compatibility) |
|
||||
|
||||
### 2. movescu (C-MOVE initiator) — Simplified (no modality loop)
|
||||
|
||||
```bash
|
||||
/data/dcmtk-bin/movescu \
|
||||
-aet CDRECORD \
|
||||
-aec ABPACS \
|
||||
-aem CDRECORD \
|
||||
localhost 11112 \
|
||||
-S \
|
||||
-k 0008,0050="${accession_number}" \
|
||||
-k 0010,0020="" \
|
||||
--no-port
|
||||
```
|
||||
|
||||
**Key flags:**
|
||||
| Flag | Purpose |
|
||||
|------|---------|
|
||||
| `-aet CDRECORD` | Our AE title (calling) |
|
||||
| `-aec ABPACS` | PACS AE title (called) |
|
||||
| `-aem CDRECORD` | Move destination AE (tells PACS to C-STORE to CDRECORD) |
|
||||
| `localhost 11112` | PACS host:port |
|
||||
| `-S` | Study Root query model |
|
||||
| `-k 0008,0050=X` | Accession Number query key |
|
||||
| `-k 0010,0020=""` | Patient ID = wildcard (required for Study Root) |
|
||||
| `--no-port` | No incoming port on movescu (storescp handles incoming) |
|
||||
|
||||
### 3. movescu — Conservative (with modality loop, replicating original behavior)
|
||||
|
||||
```bash
|
||||
# Same as above but add modality filter via query key
|
||||
/data/dcmtk-bin/movescu \
|
||||
-aet CDRECORD \
|
||||
-aec ABPACS \
|
||||
-aem CDRECORD \
|
||||
localhost 11112 \
|
||||
-S \
|
||||
-k 0008,0050="${accession_number}" \
|
||||
-k 0010,0020="" \
|
||||
-k 0008,0060="${modality_code}" \
|
||||
--no-port
|
||||
```
|
||||
|
||||
**Note:** `-k 0008,0060=MR` filters queries to studies with Modality=MR.
|
||||
This is NOT identical to dcm4che2's `-cstore MR` (which filters at the association/presentation-context level).
|
||||
For most PACS, the query-level filter is sufficient. If the PACS requires specific presentation context negotiation per modality, the simplified approach (no modality loop) with `+xa` (accept all) is recommended.
|
||||
|
||||
### 4. Cleanup
|
||||
|
||||
```bash
|
||||
# After movescu completes (or times out):
|
||||
kill $STORESCP_PID 2>/dev/null
|
||||
wait $STORESCP_PID 2>/dev/null
|
||||
```
|
||||
|
||||
## Per-File Replacements
|
||||
|
||||
### mkiso.php — current Java block (lines ~56-59)
|
||||
|
||||
**Current:**
|
||||
```php
|
||||
foreach($modalities as $cstore=>$v) {
|
||||
$cmd = "JAVA_HOME=/usr/lib/jvm/jdk1.8.0_144 LANG=en_US.iso-8859-1 /usr/local/dcm4che/dcm4che2/bin/dcmqr -L CDRECORD:10104 ABPACS@localhost:11112 -cmove CDRECORD -qAccessionNumber=${accession_number} -cstore $cstore -cstoredest $dicomdir/DICOMDIR";
|
||||
exec($cmd, $outputRes);
|
||||
}
|
||||
```
|
||||
|
||||
**Replace with (simplified approach):**
|
||||
```php
|
||||
// Start storescp as background storage receiver
|
||||
$storescp_pid = exec("/data/dcmtk-bin/storescp 10104 -aet CDRECORD -od ${dicomdir}/DICOMDIR +xa &> /tmp/storescp_" . basename($dicomdir) . ".log & echo $!");
|
||||
|
||||
// Allow storescp to bind port
|
||||
sleep(1);
|
||||
|
||||
// Single C-MOVE for the accession number (Study Root model)
|
||||
$cmd = "/data/dcmtk-bin/movescu -aet CDRECORD -aec ABPACS -aem CDRECORD localhost 11112 -S -k 0008,0050=${accession_number} -k 0010,0020= --no-port";
|
||||
exec($cmd, $outputRes, $exitCode);
|
||||
|
||||
// Clean up storescp
|
||||
exec("kill $storescp_pid 2>/dev/null");
|
||||
```
|
||||
|
||||
### mkiso2.php — current Java block (lines ~56-59)
|
||||
|
||||
Same C-MOVE replacement as mkiso.php.
|
||||
|
||||
**Additionally, replace dcmsend (line ~62):**
|
||||
|
||||
**Current:**
|
||||
```php
|
||||
$cmd = "/usr/bin/dcmsend +sd +rd 172.16.0.120 104 $dicomdir/DICOMDIR";
|
||||
```
|
||||
|
||||
**Replace with:**
|
||||
```php
|
||||
$cmd = "/data/dcmtk-bin/storescu -aet CDRECORD +sd +r 172.16.0.120 104 ${dicomdir}/DICOMDIR";
|
||||
```
|
||||
|
||||
`/usr/bin/dcmsend` does not exist on the current system. `storescu +sd +r` (scan directories + recurse) is the dcmtk equivalent.
|
||||
|
||||
### mkiso_multiple.php — current Java block (lines ~80-87)
|
||||
|
||||
**Current (nested loop):**
|
||||
```php
|
||||
foreach($modalities as $cstore=>$v) {
|
||||
foreach($as as $accession_number) {
|
||||
$cmd = "JAVA_HOME=... dcmqr ... -qAccessionNumber=${accession_number} -cstore $cstore -cstoredest $dicomdir/DICOMDIR";
|
||||
exec($cmd, $outputRes);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Replace with (simplified, one C-MOVE per accession):**
|
||||
```php
|
||||
// Start storescp once for all accessions
|
||||
$storescp_pid = exec("/data/dcmtk-bin/storescp 10104 -aet CDRECORD -od ${dicomdir}/DICOMDIR +xa &> /tmp/storescp_" . basename($dicomdir) . ".log & echo $!");
|
||||
sleep(1);
|
||||
|
||||
foreach($as as $accession_number) {
|
||||
$cmd = "/data/dcmtk-bin/movescu -aet CDRECORD -aec ABPACS -aem CDRECORD localhost 11112 -S -k 0008,0050=${accession_number} -k 0010,0020= --no-port";
|
||||
exec($cmd, $outputRes, $exitCode);
|
||||
}
|
||||
|
||||
exec("kill $storescp_pid 2>/dev/null");
|
||||
```
|
||||
|
||||
## DICOM Tag Reference
|
||||
|
||||
| Tag | Name | Used As |
|
||||
|-----|------|---------|
|
||||
| (0008,0050) | Accession Number | Query key — filters by accession |
|
||||
| (0008,0052) | Query/Retrieve Level | Set by `-S` (Study) — queries at study level |
|
||||
| (0008,0060) | Modality | Query key — filters by modality (if using conservative approach) |
|
||||
| (0010,0020) | Patient ID | Required wildcard for Study Root Q/R |
|
||||
|
||||
## Error Handling Considerations
|
||||
|
||||
1. **storescp already bound**: If `10104` is in use, storescp will fail. Use a random port or check first.
|
||||
2. **movescu timeout**: Default is unlimited. Consider `-to 300` (5 min timeout) for production.
|
||||
3. **PACS unreachable**: movescu returns non-zero exit code — PHP should handle this (currently scripts have no error handling).
|
||||
4. **Partial retrieval**: movescu may return success even if some images fail. The exit code reflects the C-MOVE response status.
|
||||
5. **Race condition**: The `sleep 1` is a heuristic. For production, consider polling to check storescp is ready.
|
||||
|
||||
## Verification Commands
|
||||
|
||||
```bash
|
||||
# Test storescp starts correctly
|
||||
/data/dcmtk-bin/storescp 10104 -aet CDRECORD -od /tmp/test_dicom +xa &
|
||||
PID=$!
|
||||
sleep 1
|
||||
echo "storescp PID: $PID (should be running)"
|
||||
kill $PID
|
||||
|
||||
# Test movescu against PACS (dry run verification)
|
||||
/data/dcmtk-bin/movescu -aet CDRECORD -aec ABPACS -aem CDRECORD localhost 11112 \
|
||||
-S -k "0008,0050=MR.180505.026" -k "0010,0020=" --no-port -v
|
||||
|
||||
# Test storescu (replacement for dcmsend)
|
||||
/data/dcmtk-bin/storescu -aet CDRECORD +sd +r 172.16.0.120 104 /tmp/test_dicom/ -v
|
||||
```
|
||||
39
todo/mkiso-replace-java-with-dcmtk.md
Normal file
39
todo/mkiso-replace-java-with-dcmtk.md
Normal file
@@ -0,0 +1,39 @@
|
||||
# Replace Java dcm4che2 `dcmqr` with dcmtk CLI
|
||||
|
||||
- [ ] 1. **Replace Java dcmqr in `mkiso.php`** — **see `todo/mkiso-replace-java-with-dcmtk.detail.md` § "mkiso.php"**
|
||||
- Start `storescp` as background receiver, run single `movescu` per accession (no modality loop)
|
||||
- Verify ISO generation, download, and cleanup still work
|
||||
|
||||
- [ ] 2. **Replace Java dcmqr in `mkiso2.php`** — **see `todo/mkiso-replace-java-with-dcmtk.detail.md` § "mkiso2.php"**
|
||||
- ⚠️ **LOW PRIORITY** — `mkiso2.php` is NOT called by the HIS `pacs_downloadiso` module. It is a standalone DICOM relay script, possibly run manually or by cron. See `docs/pacs_downloadiso-usage.md` for integration analysis.
|
||||
- Same C-MOVE replacement as mkiso.php
|
||||
- Also replace `/usr/bin/dcmsend` (missing binary) with `/data/dcmtk-bin/storescu +sd +r`
|
||||
- Verify DICOM relay to `172.16.0.120:104` works
|
||||
|
||||
- [ ] 3. **Replace Java dcmqr in `mkiso_multiple.php`** — **see `todo/mkiso-replace-java-with-dcmtk.detail.md` § "mkiso_multiple.php"**
|
||||
- Start `storescp` once, run `movescu` for each accession in the list
|
||||
- DB lookup logic stays unchanged
|
||||
- Verify multi-accession ISO download works with patient-name filenames
|
||||
|
||||
- [ ] 4. **Environment setup**
|
||||
- Ensure `/data/dcmtk-bin/` is readable and executable by the web server user
|
||||
- ⚠️ **Concurrent requests confirmed** — multiple HIS users can trigger downloads simultaneously via `pacs_downloadiso` module. Must use unique port per request.
|
||||
- If multiple concurrent requests are possible, use a unique port per request (e.g., `$port = 10104 + getmypid() % 100`)
|
||||
- No changes needed in the HIS `pacs_downloadiso` module — it calls scripts on the PACS host by URL only
|
||||
|
||||
- [ ] 5. **Testing**
|
||||
- Test with real accession numbers on the production server
|
||||
- Verify all DICOM files retrieved (spot-check file count and content)
|
||||
- Verify ISO downloads work end-to-end (download + mount + open in viewer)
|
||||
- Test through the HIS `pacs_downloadiso` UI (Preview → Download ISO) to verify end-to-end integration still works
|
||||
- Test both single and multi-accession download flows
|
||||
- Verify mkiso2.php relay to `172.16.0.120:104` (if migrated)
|
||||
- Test error cases: invalid accession number, PACS unreachable, timeout
|
||||
- Check storescp process is always cleaned up (no zombie processes)
|
||||
- Verify concurrent downloads from multiple HIS users don't conflict
|
||||
- Test Print ISO (CD Publisher) still works (not part of dcmtk migration, regression check only)
|
||||
|
||||
- [ ] 6. **Remove Java dependency** (after dcmtk verified in production)
|
||||
- Remove `/usr/local/dcm4che/dcm4che2/` if no other tools need it
|
||||
- Remove JDK 1.8.0_144 if no other Java apps on server
|
||||
- Remove `JAVA_HOME` environment variable
|
||||
89
todo/pre-flight.md
Normal file
89
todo/pre-flight.md
Normal file
@@ -0,0 +1,89 @@
|
||||
# Pre-Flight Checklist — Before Implementation
|
||||
|
||||
## Decisions needed before writing code
|
||||
|
||||
- [ ] **YAML or JSON config?**
|
||||
- YAML = 1 external dep (`gopkg.in/yaml.v3`), more human-readable
|
||||
- JSON = zero external deps (`encoding/json`), `config.json` instead of `config.yaml`
|
||||
- **Recommendation**: JSON for true stdlib-only. YAML is fine if 1 dep is ok.
|
||||
|
||||
- [ ] **Patient API availability during dev**
|
||||
- The patient-data API doesn't exist yet (only spec in `docs/patient-api-spec.md`)
|
||||
- Option A: Build a thin PHP stub first (copy the reference impl from the spec)
|
||||
- Option B: Make patient API call optional with graceful degradation in Stage 1
|
||||
- **Recommendation**: B for Stage 1 (multi-accession ISO falls back to accession-list filename if API unavailable). Build the real API before Stage 2.
|
||||
|
||||
- [ ] **`send_rimage_multiple.php` single vs multi routing**
|
||||
- The HIS JS calls `send_rimage_multiple.php?accession_number=X` for single AND `send_rimage_multiple.php?accession_number=X,Y,Z` for multi
|
||||
- The Go API has TWO endpoints: `/api/iso/print` (single) and `/api/iso/print-multiple` (multi)
|
||||
- **Problem**: nginx can't distinguish them — same PHP file, same query param name
|
||||
- **Fix**: Merge into one Go endpoint that auto-detects commas:
|
||||
- `GET /api/iso/print?accession_number=X` → single
|
||||
- `GET /api/iso/print?accession_number=X,Y,Z` → comma → multi
|
||||
- Then nginx maps `send_rimage_multiple.php` → `/api/iso/print` only
|
||||
|
||||
- [ ] **0-file edge case in DICOM fetch**
|
||||
- What if PACS returns success but 0 files? (valid accession, no images)
|
||||
- Current plan doesn't explicitly handle this
|
||||
- **Fix**: After FetchDICOM, check `filesCount > 0`. If 0, return `404 {"error":"no DICOM data"}` before attempting ISO creation.
|
||||
|
||||
- [ ] **microdicom viewer files path**
|
||||
- Config: `iso.microdicom_path: "/var/www/html/microdicom"`
|
||||
- These files are in `raw/microdicom/` in this project
|
||||
- Must exist on the PACS server at the configured path
|
||||
- If missing, ISO will still work (just no embedded viewer)
|
||||
|
||||
- [ ] **CD Publisher reachability**
|
||||
- Config: `cd_publisher.host: "172.16.0.120"`, port 104
|
||||
- May not be reachable from dev environment
|
||||
- storescu will fail with connection refused — handle gracefully
|
||||
|
||||
- [ ] **Config path**
|
||||
- Hardcode: `config.yaml` in working directory
|
||||
- Or env: `MKISO_CONFIG=/etc/mkiso/config.yaml`
|
||||
- **Recommendation**: env var with fallback to `./config.yaml`
|
||||
|
||||
## Files that need to be created (not just spec'd)
|
||||
|
||||
| File | Status |
|
||||
|------|--------|
|
||||
| `go.mod` | Create with `module mkiso-server` |
|
||||
| `config.example.yaml` | Template from design doc § config.yaml |
|
||||
| `main.go` | Entrypoint |
|
||||
| `internal/config/config.go` | Config loading |
|
||||
| `internal/route/route.go` | Route wiring |
|
||||
| `internal/middleware/auth.go` | API key middleware (Stage 2) |
|
||||
| `internal/handler/health.go` | Health check |
|
||||
| `internal/handler/iso.go` | ISO download handlers |
|
||||
| `internal/handler/print.go` | Print/relay handlers |
|
||||
| `internal/service/dicom.go` | DICOM orchestration |
|
||||
| `internal/service/iso.go` | ISO creation |
|
||||
| `internal/service/relay.go` | CD Publisher relay |
|
||||
| `internal/repo/patient.go` | Patient API client |
|
||||
| `pkg/dicom/command.go` | DICOM binary runner |
|
||||
|
||||
## Implementation order (respects dependencies)
|
||||
|
||||
```
|
||||
1. config.go (no deps)
|
||||
2. pkg/dicom/command.go (no deps)
|
||||
3. repo/patient.go (depends on config)
|
||||
4. service/dicom.go (depends on #2)
|
||||
5. service/iso.go (depends on #3, #4)
|
||||
6. service/relay.go (depends on #3, #4)
|
||||
7. handler/health.go (no deps)
|
||||
8. handler/iso.go (depends on #5)
|
||||
9. handler/print.go (depends on #6)
|
||||
10. route/route.go (depends on #7, #8, #9)
|
||||
11. middleware/auth.go (depends on config) — Stage 2
|
||||
12. main.go (depends on #10)
|
||||
```
|
||||
|
||||
## Known gaps in the plan (addressed above)
|
||||
|
||||
| Gap | Resolution |
|
||||
|-----|------------|
|
||||
| Nginx regex mapping bug (old `$1` didn't map to Go paths) | Fixed — explicit `location =` blocks with correct `proxy_pass` targets |
|
||||
| `print` vs `print-multiple` routing conflict | Merge into single endpoint with comma detection. Update handler accordingly. |
|
||||
| 0-files from PACS not handled | Check `filesCount > 0` after FetchDICOM, return 404 if empty |
|
||||
| Patient API doesn't exist yet | Graceful degradation in Stage 1 (skip patient name for filename) |
|
||||
Reference in New Issue
Block a user