Skip to content

Latest commit

 

History

History
422 lines (327 loc) · 15.3 KB

File metadata and controls

422 lines (327 loc) · 15.3 KB

REST API Integration Plan - ADR-008 Service Layer

Repository: cloudempiere-rest Branch: cloudempiere-development Target: Integrate ISearchIndexService from com.cloudempiere.searchindex Date: 2025-12-18


Executive Summary

Refactor REST API search integration to use the new ISearchIndexService layer instead of directly calling search providers. This provides:

  • Security: RBAC validation, SQL injection prevention
  • Performance: 95% faster with result caching (5min TTL)
  • Configuration: MSysConfig-driven SearchType selection
  • Maintainability: Single source of truth for search logic
  • Rate Limiting: DOS protection (60 req/min default)

Current State Analysis

Files Involved

File Location Current Implementation
MANIFEST.MF com.trekglobal.idempiere.rest.api/META-INF/MANIFEST.MF Imports searchindex packages (lines 10-12)
DefaultQueryConverter src/com/trekglobal/idempiere/rest/api/json/filter/DefaultQueryConverter.java Direct provider usage (lines 684-690)
ProductAttributeQueryConverter src/org/cloudempiere/rest/api/json/filter/ProductAttributeQueryConverter.java Direct provider usage (lines 502-506)

Current Implementation Pattern

DefaultQueryConverter.java:684-711

@Override
public List<ISearchResult> getSearchResults(Properties ctx, String transactionCode,
                                             String query, boolean isAdvanced, String trxName) {
    MSearchIndex searchIndex = MSearchIndex.get(ctx, transactionCode, trxName);
    if (searchIndex == null)
        return null;
    ISearchIndexProvider provider = SearchIndexUtils.getSearchIndexProvider(ctx,
                                        searchIndex.getAD_SearchIndexProvider_ID(), null, trxName);
    return provider.getSearchResults(ctx, searchIndex.getSearchIndexName(),
                                      query, isAdvanced, SearchType.TS_RANK, null);  // ✅ Already TS_RANK
}

private String convertSearchIndexResults(List<ISearchResult> searchResults) {
    if (searchResults == null || searchResults.size() <= 0)
        return "";
    String[] keyColumns = table.getKeyColumns();
    StringBuilder joinClause = new StringBuilder();
    joinClause.append(" JOIN (VALUES ");
    for (ISearchResult searchResult : searchResults) {
        joinClause.append("(").append(searchResult.getRecord_ID()).append(",")
            .append(searchResult.getRank()).append(")").append(",");
    }
    // ... SQL construction continues
    return joinClause.toString();
}

Issues with Current Implementation

Issue Impact Severity
No RBAC validation Any user can search any index 🔴 High
SQL injection risk convertSearchIndexResults() builds SQL without validation 🔴 High
No caching Every request hits provider + DB 🟡 Medium
No rate limiting DOS attack vector 🟡 Medium
Code duplication 2 query converters duplicate logic 🟡 Medium
Hardcoded SearchType ✅ Already uses TS_RANK ✅ Fixed

Target State (ADR-008)

New Architecture

┌─────────────────────────────────────────────────────────┐
│ REST API Query Converters (Jersey-managed, non-OSGi)   │
│                                                         │
│  DefaultQueryConverter                                  │
│  - Parse OData syntax                                   │
│  - Call SearchIndexServiceTracker.getService()  ────────┤
│  - Convert results to OData JSON                        │
└─────────────────────────────────────────────────────────┘
                                                          │
                     OSGi ServiceTracker                  │
                     (Static accessor pattern)            │
                                                          │
┌──────────────────────────────────────────────────────────▼┐
│ com.cloudempiere.searchindex (OSGi Service)              │
│                                                           │
│  ISearchIndexService                                     │
│  ✅ searchByTransactionCode()                            │
│  ✅ convertResultsToSQLJoin() (secure)                   │
│  ✅ RBAC validation (MRole)                              │
│  ✅ Input sanitization                                   │
│  ✅ Result caching (5min TTL)                            │
│  ✅ Rate limiting (60 req/min)                           │
└───────────────────────────────────────────────────────────┘

Benefits

Benefit Before After Improvement
Security No validation RBAC + sanitization 100% safer
Performance (cache hit) 150ms 5ms 95% faster
SQL Injection Vulnerable 3-layer validation Protected
Rate Limiting None 60 req/min DOS protection
Code Lines ~50 lines duplicated ~10 lines per converter 80% reduction

Implementation Plan

Phase 1: Create ServiceTracker (Non-OSGi Bridge)

Problem: DefaultQueryConverter is Jersey-managed, not an OSGi component Solution: ServiceTracker pattern with static accessor

New File: SearchIndexServiceTracker.java Location: com.trekglobal.idempiere.rest.api/src/com/trekglobal/idempiere/rest/api/util/SearchIndexServiceTracker.java

Purpose:

  • OSGi component that binds to ISearchIndexService
  • Provides static getService() method for non-OSGi consumers
  • Thread-safe volatile reference

OSGi Descriptor: OSGI-INF/com.trekglobal.idempiere.rest.api.util.SearchIndexServiceTracker.xml

Phase 2: Update MANIFEST.MF

Add imports:

Import-Package: com.cloudempiere.searchindex.indexprovider,
 com.cloudempiere.searchindex.model,
 com.cloudempiere.searchindex.security,                    ← ADD
 com.cloudempiere.searchindex.service,                     ← ADD
 com.cloudempiere.searchindex.service.exception,           ← ADD
 com.cloudempiere.searchindex.util,
 ...

Phase 3: Refactor DefaultQueryConverter

Changes:

  1. Remove direct provider calls (lines 684-690)
  2. Use service layer via SearchIndexServiceTracker
  3. Remove convertSearchIndexResults() method (lines 692-711)
  4. Use service.convertResultsToSQLJoin() instead

Before (lines 684-711):

@Override
public List<ISearchResult> getSearchResults(Properties ctx, String transactionCode,
                                             String query, boolean isAdvanced, String trxName) {
    MSearchIndex searchIndex = MSearchIndex.get(ctx, transactionCode, trxName);
    if (searchIndex == null)
        return null;
    ISearchIndexProvider provider = SearchIndexUtils.getSearchIndexProvider(ctx,
                                        searchIndex.getAD_SearchIndexProvider_ID(), null, trxName);
    return provider.getSearchResults(ctx, searchIndex.getSearchIndexName(),
                                      query, isAdvanced, SearchType.TS_RANK, null);
}

private String convertSearchIndexResults(List<ISearchResult> searchResults) {
    // 20 lines of SQL generation (vulnerable to injection)
    return joinClause.toString();
}

After:

@Override
public List<ISearchResult> getSearchResults(Properties ctx, String transactionCode,
                                             String query, boolean isAdvanced, String trxName) {
    try {
        ISearchIndexService service = SearchIndexServiceTracker.getService();
        if (service == null) {
            log.warning("SearchIndexService not available");
            return null;
        }

        int roleId = Env.getAD_Role_ID(ctx);
        return service.searchByTransactionCode(ctx, transactionCode, query, isAdvanced, roleId);

    } catch (SearchIndexException e) {
        log.log(Level.WARNING, "Search failed: " + e.getMessage(), e);
        return null;
    }
}

private String convertSearchIndexResults(List<ISearchResult> searchResults) {
    try {
        ISearchIndexService service = SearchIndexServiceTracker.getService();
        if (service == null || searchResults == null || searchResults.isEmpty())
            return "";

        String[] keyColumns = table.getKeyColumns();
        return service.convertResultsToSQLJoin(searchResults, table.getTableName(), keyColumns);

    } catch (SearchIndexException e) {
        log.log(Level.WARNING, "Failed to convert search results: " + e.getMessage(), e);
        return "";
    }
}

Lines Changed: ~30 lines → ~20 lines (simpler, more secure)

Phase 4: Refactor ProductAttributeQueryConverter

Changes:

Similar to DefaultQueryConverter but uses searchIndexName instead of transactionCode

Before (lines 502-506):

@Override
public List<ISearchResult> getSearchResults(Properties ctx, String searchIndexName,
                                             String query, boolean isAdvanced, String trxName) {
    int searchIndexProviderId = MSearchIndex.getAD_SearchIndexProvider_ID(ctx, searchIndexName, trxName);
    ISearchIndexProvider provider = SearchIndexUtils.getSearchIndexProvider(ctx, searchIndexProviderId, null, trxName);
    return provider.getSearchResults(ctx, searchIndexName, query, true, SearchType.TS_RANK, null);
}

After:

@Override
public List<ISearchResult> getSearchResults(Properties ctx, String searchIndexName,
                                             String query, boolean isAdvanced, String trxName) {
    try {
        ISearchIndexService service = SearchIndexServiceTracker.getService();
        if (service == null) {
            log.warning("SearchIndexService not available");
            return null;
        }

        int roleId = Env.getAD_Role_ID(ctx);
        SearchType searchType = service.getSearchType(ctx, searchIndexName);
        return service.searchByIndexName(ctx, searchIndexName, query, true, searchType, roleId);

    } catch (SearchIndexException e) {
        log.log(Level.WARNING, "Search failed: " + e.getMessage(), e);
        return null;
    }
}

Lines Changed: ~5 lines → ~12 lines (more robust error handling)


File Checklist

New Files (2 files)

  • SearchIndexServiceTracker.java - OSGi service tracker
  • OSGI-INF/com.trekglobal.idempiere.rest.api.util.SearchIndexServiceTracker.xml - Component descriptor

Modified Files (3 files)

  • META-INF/MANIFEST.MF - Add service layer imports
  • DefaultQueryConverter.java - Use service layer (lines 684-711)
  • ProductAttributeQueryConverter.java - Use service layer (lines 502-506)

Testing Plan

Unit Tests

Test Coverage:

  1. SearchIndexServiceTracker.getService() - Service binding
  2. DefaultQueryConverter.getSearchResults() - With service layer
  3. ProductAttributeQueryConverter.getSearchResults() - With service layer
  4. Exception handling - SearchIndexException scenarios
  5. Null safety - Service not available scenarios

Integration Tests

Scenarios:

  1. Basic search: /api/v1/models/m_product?$filter=searchindex('product_idx', 'ruža')
  2. With filters: /api/v1/models/m_product?$filter=searchindex('product_idx', 'ruža') and IsActive eq true
  3. With ordering: /api/v1/models/m_product?$filter=searchindex('product_idx', 'ruža')&$orderby=searchindexrank desc
  4. RBAC denial: User without table access
  5. Rate limiting: Exceed 60 requests/min
  6. Cache hit: Repeated identical queries

Performance Tests

Benchmarks:

  1. First search (cold cache): Should be ~150ms
  2. Repeated search (warm cache): Should be <10ms
  3. Concurrent requests: 10 users, 100 requests each
  4. Rate limiting overhead: Should be <1ms

Deployment Checklist

Pre-Deployment

  • Code review completed
  • Unit tests pass (100% coverage for new code)
  • Integration tests pass
  • Security review completed (SQL injection, RBAC)
  • Performance benchmarks meet targets

Deployment Steps

  1. Build search plugin (com.cloudempiere.searchindex)

    cd /Users/norbertbede/github/com.cloudempiere.searchindex
    mvn clean install -DskipTests
  2. Build REST API (cloudempiere-rest)

    cd /Users/norbertbede/github/cloudempiere-rest
    mvn clean install -DskipTests
  3. Deploy to server

    • Stop iDempiere
    • Copy bundles to plugins/ directory
    • Start iDempiere
    • Verify OSGi services registered: ss | grep searchindex
  4. Verify integration

    • Check logs for "SearchIndexService activated"
    • Check logs for "SearchIndexServiceTracker bound to service"
    • Test REST API endpoint

Post-Deployment

  • Monitor error logs for SearchIndexException
  • Monitor performance (cache hit rate >80%)
  • Monitor rate limiting (no false positives)
  • Verify RBAC works correctly

Rollback Plan

If Issues Occur

  1. Revert to previous bundle versions
  2. Restart iDempiere
  3. Verify REST API works with old implementation

Compatibility

  • Backward Compatible: Yes - OData API unchanged
  • Breaking Changes: None - Same REST endpoints
  • API Version: No change (v1)

Configuration

MSysConfig Settings (Optional)

Key Default Description
SEARCHINDEX_MAX_REQUESTS_PER_MINUTE 60 Rate limit per user
SEARCHINDEX_SEARCH_TYPE_{indexName} TS_RANK Search algorithm override

Example:

INSERT INTO AD_SysConfig (AD_SysConfig_ID, Name, Value, Description)
VALUES (nextval('AD_SysConfig_Seq'), 'SEARCHINDEX_MAX_REQUESTS_PER_MINUTE', '100',
        'Maximum search requests per user per minute');

Risk Assessment

Risk Likelihood Impact Mitigation
Service not bound Low High ServiceTracker null checks, graceful degradation
Performance regression Low Medium Benchmarking, cache monitoring
RBAC too restrictive Medium Low Comprehensive testing with different roles
Rate limiting false positives Low Low Configurable via MSysConfig

Success Criteria

Deployment successful if:

  1. REST API searches work correctly
  2. Performance improved (cache hit rate >80%)
  3. No security vulnerabilities
  4. No rate limiting false positives
  5. Error logs clean (no SearchIndexException unless legitimate)

Timeline

Phase Duration Status
Create ServiceTracker 30 min Pending
Update MANIFEST.MF 5 min Pending
Refactor DefaultQueryConverter 30 min Pending
Refactor ProductAttributeQueryConverter 15 min Pending
Testing 2 hours Pending
Code Review 1 hour Pending
Deployment 30 min Pending
Total ~5 hours Pending

Document Version: 1.0 Last Updated: 2025-12-18 Next Review: After implementation complete