[as-smets]

Closes issue #188

To be able to factorize code between search and browse common request parameters

I didn't want the object to be called only BaseRequest so I chose BrowseSearchBaseRequest but not totally convinced

[Copilot]

Pull request overview

This PR refactors the Java client’s browse/search request models by extracting shared request parameters into a common base class, reducing duplication between BrowseRequest and SearchRequest.

Changes:

• Introduces BrowseSearchBaseRequest to hold common browse/search parameters (pagination, facets, sorting, etc.).
• Updates SearchRequest to extend the new base request and removes duplicated fields/accessors.
• Updates BrowseRequest to extend the new base request and removes duplicated fields/accessors.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
constructorio-client/src/main/java/io/constructor/client/SearchRequest.java	Switches search request model to inherit shared parameters from the new base request.
constructorio-client/src/main/java/io/constructor/client/BrowseSearchBaseRequest.java	Adds the new shared base request containing common browse/search fields and accessors.
constructorio-client/src/main/java/io/constructor/client/BrowseRequest.java	Switches browse request model to inherit shared parameters from the new base request.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Alexey-Pavlov]

LGTM! Left a few comments there and there

[as-smets]

thanks for review, I updated with your feedbacks

[evanyan13]

Hi @as-smets,
Nice meeting you! I'm one of the admin for this library
Thank you again for taking the time to contribute our repo. 🙏

I fully agree with the direction. A shared base removes the duplication between the two request classes, and it helps both with building these requests on your side and with maintaining the SDK long term.

That said, I’d like to take it a step further. Merging only search and browse works for these two classes, but the same approach could serve the repo as a whole:

• Many retrieval endpoints share the same core params, for example autocomplete, recommendations.
• The current base class is named and shaped specifically for search and browse. Other endpoints couldn’t reuse it without inheriting setters that don’t apply to them.

My Recommendation: A Two-Layered Base Class Approach.

• ResultRequestBase holds what every result retrieval request supports.
• PlpRequestBase extends ResultRequestBase adds what PLP endpoints need. SearchRequest and BrowseRequest extend this one.
• Later, other retrieval requests can extend whichever layer fits their shape.

Suggested Changes

1. Rename and split BrowseSearchBaseRequest into ResultRequestBase and PlpRequestBase.
2. Group related fields into small param objects, with the flat setters and getters kept as delegates so existing callers don't change. Suggested objects:
   • FilterParams
   • FormatOptions
   • RequestContext
   • PaginationParams
   • SortParams
   • This lets endpoints that support only some of these (like the browse facet requests) reuse the pieces without inheriting setters that they don't support.
3. Keep the accessors for those objects protected, not public, so they don't become part of the public API.

Example: the two new base classes

public abstract class ResultRequestBase {
    private final FilterParams filters = new FilterParams();
    private final FormatOptions format = new FormatOptions();
    private final RequestContext context = new RequestContext();
    private VariationsMap variationsMap;

    // new composed API
    protected FilterParams filters() { return filters; }
    protected FormatOptions format() { return format; }
    protected RequestContext context() { return context; }

    // flat delegates, kept for backward compatibility
    public Map<String, List<String>> getFacets() { return filters.getFacets(); }
    public void setFacets(Map<String, List<String>> f) { filters.setFacets(f); }
    public void setPreFilterExpression(String e) { filters.setPreFilterExpression(e); }
    public void setHiddenFields(List<String> h) { format.setHiddenFields(h); }
    public void setNow(String now) { context.setNow(now); }
    public void setQsParam(String qs) { context.setQsParam(qs); }
    public void setVariationsMap(VariationsMap v) { this.variationsMap = v; }
    // ... remaining getters and setters
}

public abstract class PlpRequestBase extends ResultRequestBase {
    private String section = "Products";
    private final PaginationParams pagination = new PaginationParams();
    private final SortParams sort = new SortParams();

    protected PaginationParams pagination() { return pagination; }
    protected SortParams sort() { return sort; }

    public void setPage(int page) { pagination.setPage(page); }
    public void setResultsPerPage(int n) { pagination.setResultsPerPage(n); }
    public void setOffset(int o) { pagination.setOffset(o); }
    public void setSortBy(String s) { sort.setSortBy(s); }
    public void setSortAscending(boolean a) { sort.setSortAscending(a); }
    // ... remaining getters and setters
}

Example: what a request class looks like afterwards

public class SearchRequest extends PlpRequestBase {
    private final String query;
    private String collectionId;

    public SearchRequest(String query) {
        if (query == null) throw new IllegalArgumentException("query is required");
        this.query = query;
    }
    // getQuery(), getCollectionId(), setCollectionId(), setQuery()
}

---

I would really appreciate your thoughts on this. It will make the request classes much easier to maintain in the long run.
I have prototyped it on top of your branch, and I’m happy to share it if that would help, or to talk through any of the points above.

Thanks again for getting this started! 💯