Fix: PATCH /api/components/{id} silently resets visibility to EVERYONE

[Aman-Cool]

Any PATCH /api/components/{id} request that omits the visbility field silently overwrites the component's existing visibility to EVERYONE, regardless of what it was before. A component previously restricted to ME_AND_MODERATORS or PRIVATE becomes world-visible after any routine PATCH; updating a description, a homepage, anything; with no warning in the response.

The fix follows the same pattern already used by updateProject(): accept the raw Map<String, Object> request body, guard each field with reqBodyMap.containsKey(), and only apply fields the caller actually sent.

• Root cause: The Thrift IDL declares optional Visibility visbility = sw360.Visibility.EVERYONE on ComponentDTO. Thrift's Java generator initialises this field to Visibility.EVERYONE in the generated no-arg constructor, so after Jackson deserialises a PATCH body that omits visbility, the DTO already carries EVERYONE ; indistinguishable from an explicit "visbility": "EVERYONE". The merge loop in updateComponent() checks fieldValue != null, and EVERYONE is not null, so it unconditionally overwrites the stored visibility on every PATCH. With a plain Java POJO the default would be null, the guard would hold, and the bug would not exist. This is a direct consequence of Thrift IDL default injection, and is part of the motivation behind the proposed GSoC 2026 project Migrate SW360 from Apache Thrift to direct Spring Bean injection.
• No new dependencies added or updated.

Issue: Fixes silent visibility reset on PATCH /api/components/{id} ; visbility silently overwritten to EVERYONE whenever the field is omitted from the request body.

---

Changes

File	What changed
ComponentController.java	patchComponent() now takes @RequestBody Map<String, Object> reqBodyMap; deserialises to ComponentDTO via convertToComponentDTO() using the shared ObjectMapper (preserves xssPreventionModule) with FAIL_ON_UNKNOWN_PROPERTIES=false
RestControllerHelper.java	updateComponent() receives reqBodyMap and skips any field not present via reqBodyMap.containsKey(field.getFieldName()); convertToComponent() now unconditionally copies visbility from the DTO
ComponentSpecTest.java	Two regression tests added covering the two cases below

---

Suggest Reviewer

@GMishx @amritkv @rudra-superrr @bibhuti230185

---

How To Test?

1. Create a component with restricted visibility:

POST /api/components
{ "name": "TestComp", "componentType": "OSS", "visbility": "ME_AND_MODERATORS" }

2. PATCH without a visbility field:

PATCH /api/components/{id}
{ "description": "routine update" }

Before: GET returns "visbility": "EVERYONE" ; restriction silently lost.
After: GET returns "visbility": "ME_AND_MODERATORS" ; preserved.

3. PATCH with an explicit "visbility": "EVERYONE" ; should update correctly (this case was also broken by the previous sentinel-based attempt at a fix and is now covered by the regression tests).

---

Checklist

Must:

• All related issues are referenced in commit messages and in PR

[Aman-Cool]

Context: Why this bug exists, and what's coming

This PR fixes one specific production bug, but it's worth documenting the broader picture for reviewers who want to understand the root cause more deeply and why a more comprehensive structural fix is on the horizon.

---

What else is currently broken by the Thrift layer

This visibility reset is one symptom of a deeper architectural issue. The following production problems all trace directly to Apache Thrift and exist in the codebase today:

Silent transport failures returned as 404 Not Found
Sw360ComponentService.splitComponents() line 334 has a catch (TException ignored) that swallows all Thrift transport errors and falls through to return "component not found". When the backend Thrift server is unreachable, callers get a misleading 404 instead of a 503. There is no log entry. The failure is completely invisible.

Wrong HTTP status codes for backend failures
RestExceptionHandler maps RuntimeException -> 400 BAD REQUEST. Any unhandled runtime exception from the backend, including legitimate server-side failures; surfaces to API consumers as a client error. This causes automated systems and CI pipelines to treat backend outages as their own bugs.

Null transport silently constructed on connection failure
In ThriftClients.makeProtocol(), if THttpClient construction throws TTransportException, the catch block proceeds to create TCompactProtocol(null) ; a protocol wrapping a null transport. The subsequent RPC call throws a NullPointerException rather than a meaningful connection error, making the failure very hard to diagnose in production logs.

No connection pooling, a new HTTP connection per RPC call
Every ThriftClients.makeXxxClient() call constructs a brand-new THttpClient backed by a fresh HTTP connection. Under moderate load this creates significant connection churn, and THRIFT_READ_TIMEOUT = 600,000 ms (10 minutes) means threads can be held for up to 10 minutes waiting on a stalled backend call.

_Fields / TBase API hidden field-iteration bugs
RestControllerHelper.updateComponent() and ThriftUtils.copyField() iterate fields using the Thrift-generated _Fields enum and the TBase API (getFieldValue, setFieldValue, isSet). This API is opaque, there is no compile-time safety. Adding or renaming a field in the .thrift IDL can silently change merge behaviour without touching the merge logic. The visibility reset this PR fixes is exactly that kind of hidden side effect.

~1,200 lines of Jackson MixIn boilerplate serving no business purpose
JacksonCustomizations.java contains 30+ @JsonIgnoreProperties MixIn classes that exist solely to suppress Thrift-generated pseudo-properties (isSetXxx getters, setXxxIsSet setters) from leaking into JSON. Every new Thrift field requires a MixIn update or it risks appearing in API responses unexpectedly.

---

How the GSoC 2026 migration fixes all of the above

The GSoC 2026 project Migrate SW360 from Apache Thrift to direct Spring Bean injection replaces the Thrift RPC layer with direct @Service injection across all modules. Once complete:

• Silent transport failures -> typed exceptions propagate correctly. There is no transport layer. SW360Exception and typed exceptions propagate through the Spring call stack directly to @ControllerAdvice. The catch (TException ignored) antipattern is eliminated structurally ; there is no TException to catch.

• Wrong HTTP status codes -> correct mapping enforced by type. Backend exceptions become typed Spring/Java exceptions. SW360Exception (carrying a RequestStatus) maps to its correct HTTP status. Nothing can fall into the wrong exception handler by accident.

• Null transport bug -> eliminated. ThriftClients and its makeProtocol() method are removed entirely. No THttpClient, no null-transport NullPointerException.

• No connection pooling -> no connections at all. Direct @Service method calls are in-process. No HTTP connections, no pooling concerns, no 10-minute read timeouts holding threads.

• _Fields / TBase API -> plain Java field access. The _Fields merge loop in RestControllerHelper can be replaced with explicit, compile-time-checked field access. The opaque field iteration that caused this bug and could cause similar bugs on any future field addition, is replaced with code the compiler can reason about.

• 1,200 lines of MixIn boilerplate -> deleted. Plain Java POJOs serialised by Jackson need no Thrift pseudo-property suppression. The entire Thrift-suppression layer in JacksonCustomizations.java can be removed.

• This visibility bug cannot recur by construction. Plain Java POJO fields default to null, not Thrift IDL-defined defaults. The merge-loop null-guard works correctly for every field without needing a reqBodyMap.containsKey() workaround. The fix in this PR becomes unnecessary and will be cleaned up as part of the RestControllerHelper rewrite.

[GMishx]

Need some clarification.

[GMishx]

Why involve objectMapper here? Spring already does it for us if we simply tell the parameter type???

[Aman-Cool]

The Map is needed because we need to know which fields the caller actually included in the request body. If we declare @RequestBody ComponentDTO and let Spring deserialize directly, Thrift's generated no-arg constructor has already initialised visbility to EVERYONE before Jackson sets any fields; so after deserialization there's no way to tell "field was omitted" from "field was explicitly sent as EVERYONE". That's the root cause of the original bug.

As for the objectMapper usage; the shared bean is created as new ObjectMapper() with no FAIL_ON_UNKNOWN_PROPERTIES configuration, so it defaults to strict/true. The .copy().configure(FAIL_ON_UNKNOWN_PROPERTIES, false) is needed to avoid failing on any unrecognised key in the PATCH body. That would apply equally to @RequestBody JsonNode + treeToValue, so the objectMapper step can't be dropped either way.

@RequestBody Map<String, Object> is also the existing pattern across ProjectController, ReleaseController, and PackageController for the same field-presence reason, so I kept it consistent with those.