Skip to content

Conversation

@FaheemBhatti
Copy link
Contributor

@FaheemBhatti FaheemBhatti commented Oct 18, 2025

fix(registry): add null safety for descriptor, and specificAssetIds in DiscoveryIntegrationAasRegistry

Description of Changes

This pull request fixes a NullPointerException that occurred in the Digital Twin Registry when posting or replacing an AAS descriptor without providing specificAssetIds.

Root Cause

In the class
org.eclipse.digitaltwin.basyx.aasregistry.feature.discovery.integration.DiscoveryIntegrationAasRegistry,
both insertAasDescriptor() and replaceAasDescriptor() methods called:

descr.getSpecificAssetIds().stream()

without verifying whether descr, globalAssetId, or specificAssetIds were null.
When an incoming AAS descriptor did not include these fields, the application threw a NullPointerException and returned HTTP 500 instead of the expected HTTP 201 Created.

Solution Implemented

  • Added defensive null checks for:
    • The specificAssetIds list
  • Introduced early return guard clauses to skip discovery integration logic when specificAssetIds are missing.
  • Safely mapped specificAssetIds using List.of() to avoid null propagation.
  • Improved logging to indicate when discovery integration is skipped due to missing identifiers.
  • Ensured that existing discovery links are cleaned up when replacing a descriptor with no identifiers.

This ensures that:

  • Posting an AAS descriptor without identifiers now returns HTTP 201.
  • No unhandled exceptions occur during descriptor insert or replace operations.
  • The registry behaves gracefully even when asset identifiers are omitted.

Related Issue

[BUG] DTR responds with HTTP500 on valid request #886
Closes #886


BaSyx Configuration for Testing

The issue and fix were verified using a local Docker setup:

docker run -p 8081:8081   -e SPRING_PROFILE=InMemory   eclipsebasyx/digitaltwinregistry:2.0.0-SNAPSHOT

No additional configuration required.

Test Procedure

  1. Start the Digital Twin Registry as shown above.
  2. POST a minimal AAS descriptor without identifiers:
    POST /shell-descriptors
    Content-Type: application/json
    
    {"id": "ta"}
  3. Observe response:
    • Before fix: HTTP 500 (NullPointerException)
    • After fix: HTTP 201 Created
  4. Repeat the POST → observe proper HTTP 409 Conflict (as expected for duplicate IDs).

AAS Files Used for Testing

No AAS files required — testing performed via REST API requests with minimal descriptor payloads such as:

{
  "id": "simple-shell"
}

Optional verification with descriptor including asset identifiers:

{
  "id": "urn:uuid:e5c96ab5-896a-1234-8761-efd74777ca97",
  "idShort": "myAas",
  "specificAssetIds": [
    {
      "name": "manufacturerPartId",
      "value": "123-345-567103"
    }
  ],
  "submodelDescriptors": [
    {
      "id": "e5c96ab5-896a-482c-8761-efd74777ca97",
      "semanticId": {
        "type": "ExternalReference",
        "keys": [
          {
            "type": "GlobalReference",
            "value": "urn:bamm:io.catenax.material_for_recycling:1.1.0#MaterialForRecycling"
          }
        ]
      },
      "endpoints": [
        {
          "interface": "SUBMODEL-3.0",
          "protocolInformation": {
            "href": "https://edc.data.plane/mypath/submodel",
            "endpointProtocol": "HTTP",
            "endpointProtocolVersion": [
              "1.1"
            ],
            "subprotocol": "DSP",
            "subprotocolBody": "id=123;dspEndpoint=http://edc.control.plane/api/v1/dsp",
            "subprotocolBodyEncoding": "plain",
            "securityAttributes": [
              {
                "type": "NONE",
                "key": "NONE",
                "value": "NONE"
              }
            ]
          }
        }
      ]
    }
  ]
}

Both payloads confirm stable operation with and without identifiers.


Additional Information

  • This fix improves the robustness of the discovery integration layer and prevents the registry from returning HTTP 500 errors on valid requests.
  • The same null-safety pattern can be extended to other discovery-related methods in the future.
  • Potential future enhancement: introduce a utility helper (e.g., DiscoveryIntegrationUtils) to encapsulate the null-safe extraction and mapping logic, reducing duplication across registry methods.

Tested environments:

  • BaSyx v2.0.0-SNAPSHOT
  • Docker (InMemory profile)

@aaronzi aaronzi requested a review from Copilot October 19, 2025 07:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a NullPointerException in the Digital Twin Registry that occurred when posting or replacing AAS descriptors without specificAssetIds. The fix adds defensive null checks and early return guards to prevent the application from crashing with HTTP 500 errors.

Key Changes:

  • Added null safety checks for specificAssetIds in both insertAasDescriptor() and replaceAasDescriptor() methods
  • Implemented early return pattern when identifiers are missing, with appropriate debug logging
  • Added cleanup logic to remove existing discovery links when replacing descriptors with no identifiers

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +102 to +106
if (ids == null || ids.isEmpty()) {
log.debug("No specificAssetIds present for AAS '{}', skipping discovery integration update", aasDescriptorId);
discoveryApi.deleteAllAssetLinksById(aasDescriptorId);
return;
}
Copy link

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cleanup logic deleteAllAssetLinksById() is only executed in replaceAasDescriptor() when ids are null/empty, but not in insertAasDescriptor(). This asymmetry could lead to inconsistent behavior. Consider documenting why cleanup is needed only during replacement, or ensure both methods handle the absence of identifiers consistently.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cleanup call to deleteAllAssetLinksById() is intentionally omitted in insertAasDescriptor().

When inserting a new AAS descriptor, the registry guarantees that the descriptor ID is unique and does not already exist in the system. Any attempt to insert a duplicate descriptor results in an HTTP 409 Conflict, preventing residual or stale discovery links from ever being created.

As a result, there is no need to perform a cleanup of discovery links during insertion — such links cannot exist yet.

In contrast, replaceAasDescriptor() explicitly handles cleanup to ensure that any previously registered discovery links are removed when an existing descriptor is updated or replaced with one that no longer contains identifiers.

This intentional asymmetry avoids redundant API calls and ensures consistent behavior while maintaining efficiency and clarity in the discovery integration process.

Furthermore, the internal implementation of the discovery service throws a 404 Not Found error if an attempt is made to delete non-existent links.
Including a deletion call during insertion would therefore generate unnecessary 404 responses and misleading log entries, implying a missing or inconsistent discovery state.
By omitting the cleanup step in insertAasDescriptor(), the registry avoids this confusion and keeps the integration behavior semantically correct.

@aaronzi aaronzi merged commit 93a4f69 into eclipse-basyx:main Oct 20, 2025
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] DTR responds with HTTP500 on valid request

2 participants