Skip to content

feature/create-default-observer#75

Merged
fvilla merged 7 commits into
developfrom
feature/create-default-observer
May 12, 2026
Merged

feature/create-default-observer#75
fvilla merged 7 commits into
developfrom
feature/create-default-observer

Conversation

@fvilla
Copy link
Copy Markdown
Contributor

@fvilla fvilla commented May 11, 2026

No description provided.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 11, 2026

Greptile Summary

This PR changes KlabService.declareContextScope to return a DigitalTwin.Configuration instead of a raw String ID, enabling richer context-scope negotiation: the server can now return the scope ID, an observer, notifications, and an isEmpty() failure flag together in one object. A secondary change refactors KnowledgeGraphNeo4j.initializeContext to accept the full Configuration directly rather than individual parameters.

  • API contract change: declareContextScope return type updated across the interface, BaseServiceClient, BaseService, RuntimeService, ResolverService, and the REST controller; all implementations now return serviceContextScope.getConfiguration() or a properly-built DigitalTwin.Configuration.empty(...) on failure.
  • Client-side propagation: ClientContextScope.setFromConfiguration applies the server-returned ID and notifications; resolveDefaultObserver exists as a named method but is entirely TODO stubs, so the observer field returned by the server is never acted upon in this PR.
  • Neo4j refactor: initializeContext simplified by passing the whole Configuration; spatial layer name now uses only the last dot-segment of the context ID, consistent between creation and deletion but a silent change from the previous full-ID naming scheme.

Confidence Score: 3/5

Client-side scope plumbing defects from prior rounds remain unaddressed, risking silent registration of failed context scopes.

The prior review threads identified concrete defects — wrong ordering of the null guard and setFromConfiguration, the setId(null) malformed-URL path, and resolveDefaultObserver being wholly unimplemented for the feature this branch is named after — none of which appear addressed in the current head commit.

ClientSessionScope.java and ClientContextScope.java — the guard ordering, null-ID URL construction, and isEmpty() check logic all need to be revisited before merging.

Important Files Changed

Filename Overview
klab.core.api/src/main/java/org/integratedmodelling/klab/api/digitaltwin/DigitalTwin.java Adds isEmpty() to Configuration interface and a static empty(Notification...) factory; cleans up unused imports.
klab.core.api/src/main/java/org/integratedmodelling/klab/api/digitaltwin/impl/ConfigurationBuilder.java Adds empty flag and observer field support to the builder; constructor wiring and empty() builder method are correct.
klab.core.api/src/main/java/org/integratedmodelling/klab/api/digitaltwin/impl/ConfigurationImpl.java Adds empty boolean field with getter/setter and wires up the previously-dead observer field through the constructor.
klab.core.api/src/main/java/org/integratedmodelling/klab/api/services/KlabService.java Return type of declareContextScope changed from String to DigitalTwin.Configuration; Javadoc updated to reflect the new semantics.
klab.core.common/src/main/java/org/integratedmodelling/common/services/client/BaseServiceClient.java Updated to deserialize a DigitalTwin.Configuration from the POST response; exception and null cases now return empty() configurations.
klab.core.common/src/main/java/org/integratedmodelling/common/services/client/scope/ClientContextScope.java Adds setFromConfiguration to propagate ID, empty flag, and notifications from the server response; resolveDefaultObserver remains entirely TODO stubs.
klab.core.common/src/main/java/org/integratedmodelling/common/services/client/scope/ClientSessionScope.java Scope registration now uses id.isEmpty() from the returned Configuration; issues around guard ordering and null safety flagged in prior review threads remain.
klab.core.services/src/main/java/org/integratedmodelling/klab/services/base/BaseService.java Return type updated; delegates to serviceContextScope.getConfiguration(). Change is straightforward.
klab.services.resolver/src/main/java/org/integratedmodelling/klab/services/resolver/ResolverService.java Return type updated; ret.getId() now correctly passed to Resource.resourceRegexFor instead of the whole Configuration object.
klab.services.runtime.server/src/main/java/org/integratedmodelling/klab/services/runtime/server/controllers/RuntimeServerController.java Controller endpoint now returns DigitalTwin.Configuration; fallback returns empty(). Successful path accesses id.getId() without an isEmpty() guard.
klab.services.runtime/src/main/java/org/integratedmodelling/klab/services/runtime/RuntimeService.java Return type updated; scope ID is always set before the configuration is returned.
klab.services.runtime/src/main/java/org/integratedmodelling/klab/services/runtime/neo4j/KnowledgeGraphNeo4JClient.java Simplified call site: individual parameters replaced by the whole DigitalTwin.Configuration object.
klab.services.runtime/src/main/java/org/integratedmodelling/klab/services/runtime/neo4j/KnowledgeGraphNeo4j.java Refactored initializeContext to accept a Configuration object; spatial layer name now uses only the last dot-segment of the context ID, consistently applied in both creation and deletion.

Sequence Diagram

sequenceDiagram
    participant CS as ClientSessionScope
    participant BSC as BaseServiceClient
    participant RSC as RuntimeServerController
    participant RS as RuntimeService
    participant CCS as ClientContextScope

    CS->>BSC: declareContextScope(contextScope, sessionScope, userScope)
    BSC->>RSC: POST /create-context (ScopeRequest)
    RSC->>RS: klabService().declareContextScope(ret, sessionScope, userScope)
    RS-->>RSC: DigitalTwin.Configuration (with ID set)
    RSC->>RSC: setupMessaging(federation, id.getId(), queues)
    RSC->>RSC: initializeAgents(id.getId())
    RSC-->>BSC: DigitalTwin.Configuration (JSON)
    BSC->>BSC: setupMessaging(contextScope, sessionScope, config.getId())
    BSC-->>CS: DigitalTwin.Configuration
    CS->>CCS: setFromConfiguration(configuration)
    CCS->>CCS: setId(config.getId()) [sets URL too]
    CCS->>CCS: notifications.addAll(config.getNotifications())
    CS->>CS: if !id.isEmpty() register scope
Loading

Reviews (6): Last reviewed commit: "Refactor session scope initialization an..." | Re-trigger Greptile

fvilla added 3 commits May 11, 2026 21:11
… and incorporate notifications in `DigitalTwin.Configuration`.
…alTwin.Configuration`. Optimize null checks and streamline `initializeContext` parameters.
Comment on lines 470 to +477
public void resolveDefaultObserver() {

if (getFederation() != null
&& getFederation().getId().equals(Federation.LOCAL_FEDERATION_ID)) {
// TODO
}
// TODO
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 resolveDefaultObserver is entirely unimplemented

Both branches of this method are empty // TODO stubs, meaning the "create-default-observer" feature that this branch is named for is never actually invoked. Any caller that expects the method to hydrate or apply the observer field from the DigitalTwin.Configuration will silently receive an uninitialized observer. If this is intentional placeholder work, a // TODO on the feature-level tracking ticket (or a clearly-disabled call site) would prevent future confusion.

@fvilla fvilla merged commit d9af16d into develop May 12, 2026
2 checks passed
@fvilla fvilla deleted the feature/create-default-observer branch May 12, 2026 12:35
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.

1 participant