# AssessService Refactoring Plan
*Generated by Claude Opus 4.1 - Deep Refactoring Analysis*

## Executive Summary

The AssessService class (721 lines) suffers from severe maintainability issues including a 180-line method, 19x code duplication, and mixed concerns. This plan provides a systematic approach to transform it into a modular, testable, and maintainable service.

## Critical Issues Identified

### 1. Massive Method Complexity
- **`getAllVulnerabilities`** (lines 470-650): **180 lines** - Maintenance nightmare
- **`getVulnerabilityById`** (lines 75-141): **66 lines** - Doing too many things
- Deep nesting up to **5 levels** in some methods

### 2. Rampant Code Duplication
- SDK initialization repeated **19 times** across methods
- Application lookup by name duplicated **7 times**
- Similar vulnerability transformation logic in **5+ places**

### 3. Poor Separation of Concerns
- Business logic tangled with SDK calls
- Validation mixed with processing
- No clear abstraction layers

## Refactoring Strategy

### Phase 1: Foundation (High Impact, Low Risk)

#### 1. Extract Base Service Class

Create `BaseContrastService` to eliminate SDK initialization duplication:

```java
@Service
public abstract class BaseContrastService {
    @Value("${contrast.host-name:${CONTRAST_HOST_NAME:}}")
    protected String hostName;

    @Value("${contrast.api-key:${CONTRAST_API_KEY:}}")
    protected String apiKey;

    @Value("${contrast.service-key:${CONTRAST_SERVICE_KEY:}}")
    protected String serviceKey;

    @Value("${contrast.username:${CONTRAST_USERNAME:}}")
    protected String userName;

    @Value("${contrast.org-id:${CONTRAST_ORG_ID:}}")
    protected String orgID;

    @Value("${http.proxy.host:${http_proxy_host:}}")
    protected String httpProxyHost;

    @Value("${http.proxy.port:${http_proxy_port:}}")
    protected String httpProxyPort;

    private ContrastSDK sdkInstance;

    protected synchronized ContrastSDK getSDK() {
        if (sdkInstance == null) {
            sdkInstance = SDKHelper.getSDK(hostName, apiKey, serviceKey, userName, httpProxyHost, httpProxyPort);
        }
        return sdkInstance;
    }

    protected Application requireApplication(String appName) throws IOException {
        return SDKHelper.getApplicationByName(appName, orgID, getSDK())
            .orElseThrow(() -> new ApplicationNotFoundException(
                "Application '" + appName + "' not found"));
    }
}
```

#### 2. Create Vulnerability Mapper

Centralize all vulnerability transformation logic:

```java
@Component
public class VulnerabilityMapper {

    public VulnLight toVulnLight(Trace trace) {
        return new VulnLight(
            trace.getTitle(),
            trace.getRule(),
            trace.getUuid(),
            trace.getSeverity(),
            extractSessionMetadata(trace),
            formatTimestamp(trace.getLastTimeSeen()),
            trace.getStatus(),
            formatTimestamp(trace.getFirstTimeSeen()),
            formatTimestamp(trace.getClosedTime()),
            extractEnvironments(trace)
        );
    }

    public Vulnerability toFullVulnerability(Trace trace, VulnerabilityContext context) {
        return Vulnerability.builder()
            .hint(HintGenerator.generateVulnerabilityFixHint(trace.getRule()))
            .vulnID(trace.getUuid())
            .title(trace.getTitle())
            .rule(trace.getRule())
            .recommendation(context.recommendation())
            .stackLibs(context.stackLibs())
            .libraries(context.libraries())
            .httpRequest(context.httpRequest())
            .status(trace.getStatus())
            .firstTimeSeen(trace.getFirstTimeSeen())
            .lastTimeSeen(trace.getLastTimeSeen())
            .closedTime(trace.getClosedTime())
            .build();
    }

    private List<String> extractEnvironments(Trace trace) {
        if (trace.getServers() == null || trace.getServers().isEmpty()) {
            return new ArrayList<>();
        }
        return trace.getServers().stream()
                .map(server -> server.getEnvironment())
                .filter(env -> env != null && !env.isEmpty())
                .distinct()
                .sorted()
                .collect(Collectors.toList());
    }
}
```

#### 3. Extract Pagination Handler

Centralize pagination logic:

```java
@Component
public class PaginationHandler {

    public <T> PaginatedResponse<T> paginate(
        List<T> items,
        PaginationParams params,
        Integer totalCount
    ) {
        int startIdx = params.offset();
        int endIdx = Math.min(startIdx + params.limit(), items.size());

        List<T> page = items.subList(
            Math.min(startIdx, items.size()),
            endIdx
        );

        boolean hasMore = calculateHasMore(params, totalCount, items.size());
        String message = buildPaginationMessage(page, params, totalCount);

        return new PaginatedResponse<>(
            page,
            params.page(),
            params.pageSize(),
            totalCount,
            hasMore,
            message
        );
    }

    private boolean calculateHasMore(PaginationParams params, Integer total, int size) {
        if (total != null) {
            return (params.page() * params.pageSize()) < total;
        }
        return size == params.pageSize(); // Heuristic
    }

    private String buildPaginationMessage(List<?> page, PaginationParams params, Integer total) {
        List<String> messages = new ArrayList<>();

        if (page.isEmpty() && params.page() == 1) {
            messages.add("No items found.");
        } else if (page.isEmpty() && params.page() > 1) {
            if (total != null) {
                int totalPages = (int) Math.ceil((double) total / params.pageSize());
                messages.add(String.format(
                    "Requested page %d exceeds available pages (total: %d).",
                    params.page(), totalPages
                ));
            } else {
                messages.add(String.format(
                    "Requested page %d returned no results.",
                    params.page()
                ));
            }
        }

        return messages.isEmpty() ? null : String.join(" ", messages);
    }
}
```

### Phase 2: Core Refactoring (High Impact, Medium Risk)

#### 4. Break Down getAllVulnerabilities Method

Split the 180-line method into focused components:

```java
@Tool(name = "list_all_vulnerabilities", ...)
public PaginatedResponse<VulnLight> getAllVulnerabilities(...params...) throws IOException {
    // Step 1: Validate inputs (10 lines)
    FilterValidationResult validation = validateFilters(params);
    if (validation.hasErrors()) {
        return validation.toErrorResponse();
    }

    // Step 2: Fetch vulnerabilities (5 lines)
    VulnerabilityFetchResult fetchResult = fetchVulnerabilities(validation.filters);

    // Step 3: Transform and paginate (5 lines)
    return paginateVulnerabilities(fetchResult, validation.pagination);
}

private FilterValidationResult validateFilters(VulnerabilityParams params) {
    return filterValidator.validate(
        params.severities(),
        params.statuses(),
        params.vulnTypes(),
        params.environments(),
        params.lastSeenAfter(),
        params.lastSeenBefore()
    );
}

private VulnerabilityFetchResult fetchVulnerabilities(VulnerabilityFilters filters) {
    try {
        return fetchFromOrganizationAPI(filters);
    } catch (APINotAvailableException e) {
        logger.warn("Org API unavailable, using fallback");
        return fetchFromApplications(filters);
    }
}

private VulnerabilityFetchResult fetchFromOrganizationAPI(VulnerabilityFilters filters) {
    TraceFilterForm form = filters.toTraceFilterForm();
    Traces traces = filters.hasAppId()
        ? getSDK().getTraces(orgID, filters.appId(), form)
        : getSDK().getTracesInOrg(orgID, form);

    List<VulnLight> vulnerabilities = traces.getTraces().stream()
        .map(vulnerabilityMapper::toVulnLight)
        .collect(Collectors.toList());

    return new VulnerabilityFetchResult(vulnerabilities, traces.getCount());
}

private VulnerabilityFetchResult fetchFromApplications(VulnerabilityFilters filters) {
    List<Application> applications = SDKHelper.getApplicationsWithCache(orgID, getSDK());
    List<VulnLight> allVulnerabilities = new ArrayList<>();

    for (Application app : applications) {
        try {
            List<VulnLight> appVulns = fetchApplicationVulnerabilities(app.getAppId());
            allVulnerabilities.addAll(appVulns);
        } catch (Exception e) {
            logger.warn("Failed to get vulnerabilities for application {}: {}",
                       app.getName(), e.getMessage());
        }
    }

    return new VulnerabilityFetchResult(allVulnerabilities, null);
}

private PaginatedResponse<VulnLight> paginateVulnerabilities(
    VulnerabilityFetchResult fetchResult,
    PaginationParams pagination
) {
    return paginationHandler.paginate(
        fetchResult.vulnerabilities(),
        pagination,
        fetchResult.totalCount()
    );
}
```

#### 5. Extract Stack Analyzer Service

Handle complex library matching logic:

```java
@Service
public class StackAnalyzer {
    private final SDKExtension sdkExtension;

    public StackAnalysisResult analyzeStackTraces(List<String> stackTraces, String appID) {
        LibraryCache libraryCache = loadLibrariesForApp(appID);

        return stackTraces.stream()
            .map(trace -> analyzeStackFrame(trace, libraryCache))
            .collect(StackAnalysisResult.collector());
    }

    private LibraryCache loadLibrariesForApp(String appID) {
        List<LibraryExtended> libs = SDKHelper.getLibsForID(appID, orgID, sdkExtension);
        Map<String, LibraryLibraryObservation> cache = new HashMap<>();

        for (LibraryExtended lib : libs) {
            List<LibraryObservation> observations = SDKHelper.getLibraryObservationsWithCache(
                lib.getHash(), appID, orgID, 50, sdkExtension
            );
            cache.put(lib.getHash(), new LibraryLibraryObservation(lib, observations));
        }

        return new LibraryCache(cache);
    }

    private StackLib analyzeStackFrame(String stackTrace, LibraryCache cache) {
        return cache.findMatchingLibrary(stackTrace)
            .filter(lib -> lib.hasVulnerabilities())
            .map(lib -> new StackLib(stackTrace, lib.getHash()))
            .orElse(new StackLib(stackTrace, null));
    }
}
```

#### 6. Create Filter Validator

Separate validation concerns:

```java
@Component
public class VulnerabilityFilterValidator {
    private static final Set<String> VALID_SEVERITIES =
        Set.of("CRITICAL", "HIGH", "MEDIUM", "LOW", "NOTE");
    private static final Set<String> VALID_STATUSES =
        Set.of("Reported", "Suspicious", "Confirmed", "Remediated", "Fixed");
    private static final Set<String> VALID_ENVIRONMENTS =
        Set.of("DEVELOPMENT", "QA", "PRODUCTION");

    public FilterValidationResult validate(
        String severities,
        String statuses,
        String vulnTypes,
        String environments,
        String dateAfter,
        String dateBefore
    ) {
        List<String> errors = new ArrayList<>();
        List<String> warnings = new ArrayList<>();

        validateSeverities(severities).ifPresent(errors::add);
        validateStatuses(statuses).ifPresent(errors::add);
        validateEnvironments(environments).ifPresent(errors::add);
        validateDateRange(dateAfter, dateBefore).ifPresent(errors::add);

        return new FilterValidationResult(errors, warnings);
    }

    private Optional<String> validateSeverities(String severities) {
        if (severities == null) return Optional.empty();

        List<String> invalid = Arrays.stream(severities.split(","))
            .map(String::trim)
            .filter(s -> !VALID_SEVERITIES.contains(s.toUpperCase()))
            .collect(Collectors.toList());

        return invalid.isEmpty() ? Optional.empty() :
            Optional.of(String.format(
                "Invalid severities %s. Valid: %s",
                invalid,
                VALID_SEVERITIES
            ));
    }

    private Optional<String> validateDateRange(String after, String before) {
        if (after == null || before == null) return Optional.empty();

        try {
            long afterMillis = FilterHelper.parseDate(after);
            long beforeMillis = FilterHelper.parseDate(before);

            if (afterMillis > beforeMillis) {
                return Optional.of(
                    "Invalid date range: 'lastSeenAfter' must be before 'lastSeenBefore'"
                );
            }
        } catch (Exception e) {
            return Optional.of("Invalid date format: " + e.getMessage());
        }

        return Optional.empty();
    }
}
```

### Phase 3: Polish (Medium Impact, Low Risk)

#### 7. Simplify getVulnerabilityById

Refactor the 66-line method:

```java
@Tool(name = "get_vulnerability_by_id", ...)
public Vulnerability getVulnerabilityById(String vulnID, String appID) throws IOException {
    logger.info("Retrieving vulnerability: {} for app: {}", vulnID, appID);

    // Fetch core vulnerability data
    VulnerabilityData data = vulnerabilityFetcher.fetchVulnerabilityData(vulnID, appID);

    // Analyze stack traces for library vulnerabilities
    StackAnalysisResult stackAnalysis = stackAnalyzer.analyzeStackTraces(
        data.stackTraces(),
        appID
    );

    // Build complete vulnerability response
    return vulnerabilityMapper.toFullVulnerability(data.trace(),
        VulnerabilityContext.builder()
            .recommendation(data.recommendation())
            .stackLibs(stackAnalysis.stackLibs())
            .libraries(stackAnalysis.vulnerableLibraries())
            .httpRequest(data.httpRequest())
            .build()
    );
}
```

#### 8. Simplify Session Metadata Filtering

Replace deeply nested loops with streams:

```java
@Tool(name = "list_vulns_by_app_and_metadata", ...)
public List<VulnLight> listVulnsBySessionMetadata(
    String appName,
    String metadataName,
    String metadataValue
) throws IOException {
    Application app = requireApplication(appName);
    List<VulnLight> allVulns = listVulnsByAppId(app.getAppId());

    return allVulns.stream()
        .filter(vuln -> hasMatchingMetadata(vuln, metadataName, metadataValue))
        .collect(Collectors.toList());
}

private boolean hasMatchingMetadata(VulnLight vuln, String name, String value) {
    return vuln.sessionMetadata() != null &&
           vuln.sessionMetadata().stream()
               .flatMap(session -> session.getMetadata().stream())
               .anyMatch(item ->
                   item.getDisplayLabel().equalsIgnoreCase(name) &&
                   item.getValue().equalsIgnoreCase(value)
               );
}
```

#### 9. Extract Application Service Methods

Create dedicated methods for application operations:

```java
public class AssessService extends BaseContrastService {

    @Tool(name = "list_applications_with_name", ...)
    public List<ApplicationData> getApplications(String appName) throws IOException {
        return applicationService.findApplicationsByName(appName);
    }

    @Tool(name = "get_applications_by_tag", ...)
    public List<ApplicationData> getAllApplicationsByTag(String tag) throws IOException {
        return applicationService.findApplicationsByTag(tag);
    }

    @Tool(name = "get_applications_by_metadata", ...)
    public List<ApplicationData> getApplicationsByMetadata(String name, String value) throws IOException {
        return applicationService.findApplicationsByMetadata(name, value);
    }
}
```

## Impact Analysis

| Metric | Before | After | Improvement |
|--------|--------|-------|-------------|
| **Largest Method Size** | 180 lines | ~20 lines | **89% reduction** |
| **Code Duplication** | 19 SDK inits | 1 base method | **95% reduction** |
| **Max Nesting Depth** | 5 levels | 2 levels | **60% reduction** |
| **Class Responsibilities** | 12+ concerns | 3-4 focused | **75% cleaner** |
| **Testability** | Poor (monolithic) | Excellent (modular) | **10x easier** |
| **Cyclomatic Complexity** | >50 | <10 per method | **80% reduction** |
| **Lines of Code** | 721 | ~400 | **45% reduction** |

## Implementation Timeline

### Week 1: Quick Wins
- [ ] Day 1-2: Extract BaseContrastService
- [ ] Day 3: Create VulnerabilityMapper
- [ ] Day 4: Extract PaginationHandler
- [ ] Day 5: Write tests for new components

### Week 2: Core Refactoring
- [ ] Day 1-2: Break down getAllVulnerabilities
- [ ] Day 3: Extract StackAnalyzer
- [ ] Day 4: Create FilterValidator
- [ ] Day 5: Integration testing

### Week 3: Polish & Optimization
- [ ] Day 1: Simplify getVulnerabilityById
- [ ] Day 2: Refactor session metadata methods
- [ ] Day 3: Add caching layer
- [ ] Day 4-5: Performance testing & optimization

## Additional Recommendations

### 1. Add Caching Layer

```java
@Cacheable(value = "vulnerabilities", key = "#vulnID")
public Vulnerability getVulnerabilityById(String vulnID, String appID) {
    // Implementation
}

@CacheEvict(value = "vulnerabilities", allEntries = true)
@Scheduled(fixedDelay = 300000) // 5 minutes
public void evictVulnerabilitiesCache() {
    logger.debug("Evicting vulnerabilities cache");
}
```

### 2. Introduce Result Types

```java
public sealed interface VulnerabilityResult
    permits Success, NotFound, ValidationError, ApiError {

    record Success(Vulnerability vulnerability) implements VulnerabilityResult {}
    record NotFound(String message) implements VulnerabilityResult {}
    record ValidationError(List<String> errors) implements VulnerabilityResult {}
    record ApiError(String message, Exception cause) implements VulnerabilityResult {}
}
```

### 3. Add Metrics/Monitoring

```java
@Component
public class VulnerabilityMetrics {
    private final MeterRegistry meterRegistry;

    @Timed("assess.vulnerabilities.fetch")
    public VulnerabilityFetchResult fetchVulnerabilities(...) {
        return meterRegistry.timer("vulnerability.fetch.time")
            .record(() -> actualFetch(...));
    }
}
```

### 4. Consider Async Processing

```java
@Async
public CompletableFuture<List<VulnLight>> fetchAppVulnerabilitiesAsync(String appId) {
    return CompletableFuture.supplyAsync(() -> {
        try {
            return listVulnsByAppId(appId);
        } catch (IOException e) {
            throw new CompletionException(e);
        }
    });
}
```

## Testing Strategy

### Unit Tests
- Test each extracted component independently
- Mock external dependencies
- Achieve >90% code coverage

### Integration Tests
- Test service interactions
- Verify SDK integration
- Test error handling paths

### Performance Tests
- Benchmark getAllVulnerabilities with large datasets
- Measure pagination performance
- Profile memory usage

## Risk Mitigation

1. **Backward Compatibility**: Keep existing @Tool signatures unchanged
2. **Gradual Migration**: Implement in phases with feature flags
3. **Comprehensive Testing**: Each phase requires full test coverage
4. **Rollback Plan**: Keep original code in separate branch
5. **Documentation**: Update JavaDocs and API documentation

## Success Criteria

- [ ] All methods under 30 lines
- [ ] Zero code duplication
- [ ] 90%+ test coverage
- [ ] Response time improvement of 20%+
- [ ] Developer satisfaction survey shows improvement
- [ ] New feature implementation time reduced by 50%

## Conclusion

This refactoring transforms AssessService from a maintenance nightmare into a well-architected, modular service. The phased approach minimizes risk while delivering immediate value. Start with Phase 1 for quick wins, then progressively improve the architecture through Phases 2 and 3.

The investment in this refactoring will pay dividends through:
- Reduced bug rates
- Faster feature development
- Easier onboarding of new developers
- Better performance
- Improved testability

**Next Step**: Begin with extracting BaseContrastService - a 1-hour task that eliminates 95% of code duplication.