Skip to content

Add region property to site and device metadata with automatic propogation #1332

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
noursaidi wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add region property to site and device metadata with automatic propogation #1332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion gencode/java/udmi/schema/Location.java
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 10 additions & 1 deletion gencode/java/udmi/schema/SiteMetadata.java
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions schema/model_system.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,14 @@
"style": "bold"
}
},
"region": {
"description": "The region according to the site model in which the device is installed in",
Copy link
Copy Markdown
Collaborator

@grafnu grafnu May 14, 2026

Choose a reason for hiding this comment

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

why is this needed in the device model? When is a device ever not in the region for the entire site?

Copy link
Copy Markdown
Collaborator Author

@noursaidi noursaidi May 14, 2026

Choose a reason for hiding this comment

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

It will shouldn't ever be different, but it needs to be in the device metadata it's describing the device, and we know which region the device is in (the site metadata describes the site). The use cases would be filtering both messages in pubsub where we can add this as an attribute, and also device, where we filter metadata for devices). We can do elaborate data relationship/joins, but it's better and preferable for all the data to be in the device metadata

Copy link
Copy Markdown
Collaborator

@grafnu grafnu May 14, 2026

Choose a reason for hiding this comment

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

Ok, I think I understand now. My concern is that it ends up in the pre-normalized metadata too, which then makes it redundant info. Can you add a simple check (should just be a line or two) in registrar that generates a warning if it's in the pre-normalized metadata?

Also, should have a test for this in tests/sites somewhre -- doesnt' need to be new, but just add a region into one of the existing sites, so the effects of this are obvious. (Also then can add a field into a metadata.json file somewhere and see the error). Unit tests for things like this aren't always that useful since they're way too spot focused, which is why I'm looking for the site-level tests.

"type": "string",
"$presentation": {
"display": "show",
"style": "bold"
}
},
"panel": {
"description": "The reference of the panel where the device is installed in",
"type": "string",
Expand Down
8 changes: 8 additions & 0 deletions schema/site_metadata.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@
"style": "bold"
}
},
"region": {
"description": "Region for the site",
"type": "string",
"$presentation": {
"display": "show",
"style": "bold"
}
},
"name": {
"description": "Name of the site or building",
"type": "string",
Expand Down
12 changes: 11 additions & 1 deletion validator/src/main/java/com/google/daq/mqtt/registrar/LocalDevice.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static com.google.udmi.util.GeneralUtils.ifNotNullGet;
import static com.google.udmi.util.GeneralUtils.ifNotNullThen;
import static com.google.udmi.util.GeneralUtils.ifNotTrueThen;
import static com.google.udmi.util.GeneralUtils.ifNullThen;
import static com.google.udmi.util.GeneralUtils.ifTrueThen;
import static com.google.udmi.util.GeneralUtils.isTrue;
import static com.google.udmi.util.GeneralUtils.writeString;
Expand Down Expand Up @@ -94,8 +95,11 @@
import udmi.schema.Envelope.SubFolder;
import udmi.schema.Envelope.SubType;
import udmi.schema.GatewayModel;
import udmi.schema.Location;
import udmi.schema.Metadata;
import udmi.schema.PointPointsetModel;
import udmi.schema.SiteMetadata;
import udmi.schema.SystemModel;


class LocalDevice implements SiteDevice {
Expand Down Expand Up @@ -852,9 +856,15 @@ public LocalDevice duplicate(String newId) {
return new LocalDevice(siteModel, newId, schemas, generation, deviceKind);
}

public void preprocessMetadata() {
public void preprocessMetadata(SiteMetadata siteMetadata) {
ifTrueWarn(catchToNull(() -> metadata.cloud.config.static_file) != null,
"Disallowed cloud.config.static_file defined");

ifNotNullThen(siteMetadata.region, region -> {
ifNullThen(metadata.system, () -> metadata.system = new SystemModel());
ifNullThen(metadata.system.location, () -> metadata.system.location = new Location());
ifNullThen(metadata.system.location.region, () -> metadata.system.location.region = region);
});
}

private void ifTrueWarn(boolean condition, String message) {
Expand Down
3 changes: 2 additions & 1 deletion validator/src/main/java/com/google/daq/mqtt/registrar/Registrar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1461,9 +1461,10 @@ private Set<String> proxiedChildren(String gatewayId) {
}

private void preprocessDeviceMetadata(Map<String, LocalDevice> workingDevices) {
SiteMetadata siteMetadata = getSiteMetadata();
workingDevices.values().forEach(localDevice -> {
try {
localDevice.preprocessMetadata();
localDevice.preprocessMetadata(siteMetadata);
} catch (ValidationError error) {
throw new RuntimeException("While preprocessing metadata", error);
} catch (Exception e) {
Expand Down
29 changes: 29 additions & 0 deletions validator/src/test/java/com/google/daq/mqtt/registrar/LocalDeviceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.Map;
import org.junit.Test;
import udmi.schema.Metadata;
import udmi.schema.SiteMetadata;

/**
* Unit tests for LocalDevice.
Expand Down Expand Up @@ -56,4 +57,32 @@ private LocalDevice getTestInstance(String sitePath) {
rawMetadata.gateway = null;
return new LocalDevice(siteModel, DEVICE_ID, SCHEMAS, null, DeviceKind.SIMPLE);
}

@Test
public void preprocessMetadataRegion() {
LocalDevice localDevice = getTestInstance(EMPTY_DEFAULTS_SITE);
SiteMetadata siteMetadata = new SiteMetadata();
siteMetadata.region = "test-region";

assertNull("device region is initially null",
catchToNull(() -> localDevice.getMetadata().system.location.region));

localDevice.preprocessMetadata(siteMetadata);

assertEquals("device region is populated from site metadata",
"test-region", localDevice.getMetadata().system.location.region);
}

@Test
public void preprocessMetadataRegionExisting() {
LocalDevice localDevice = getTestInstance(EMPTY_DEFAULTS_SITE);
localDevice.getMetadata().system.location.region = "existing-region";
SiteMetadata siteMetadata = new SiteMetadata();
siteMetadata.region = "test-region";

localDevice.preprocessMetadata(siteMetadata);

assertEquals("existing device region is preserved",
"existing-region", localDevice.getMetadata().system.location.region);
}
}
Loading