Skip to content
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.

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

Add External Source (and External Event) Attribute Validation #1606

Open
wants to merge 25 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
4060cc5
Update external source related tables for attributes
JosephVolosin Nov 7, 2024
48c85d0
Add attributes to derived_events
JosephVolosin Nov 11, 2024
1e4b19c
Expose attributes in procedural payload for events
JosephVolosin Nov 12, 2024
78caddb
update migration
pranav-super Nov 13, 2024
f1aaeb4
update permissions on external_event_type and external_source_type
pranav-super Nov 13, 2024
12121fc
update database tests
pranav-super Nov 13, 2024
22beb50
update database tests (2)
pranav-super Nov 13, 2024
1aad755
add allowed_event_types table, metadata broken
pranav-super Nov 13, 2024
bfbc853
fix metadata - name collision
pranav-super Nov 13, 2024
cb73a98
add attributes to sources and events, separate from querying
pranav-super Nov 14, 2024
6f4404d
update scheduling tests
pranav-super Nov 14, 2024
878e1ca
start end to end gateway tests
pranav-super Nov 14, 2024
9cef1d4
finish end to end gateway tests
pranav-super Nov 14, 2024
5297eb8
finish end to end gateway tests (2)
pranav-super Nov 14, 2024
40f8d44
allow cascading to allowed table when deleting external source type
pranav-super Nov 15, 2024
0067b56
fix variable name
pranav-super Nov 15, 2024
c4e5e1d
remove all references to external_source_type_allowed_event_types.sql
pranav-super Nov 18, 2024
8521550
update e2e tests
pranav-super Nov 25, 2024
631969a
update e2e tests again
pranav-super Nov 25, 2024
50488d1
update migration
pranav-super Nov 25, 2024
a69ea0c
minor fix to attribute retrieval in scheduling
pranav-super Nov 27, 2024
a5981e9
clean up sql formatting
pranav-super Dec 23, 2024
61a66a3
minor correction to external_source.sql
pranav-super Dec 30, 2024
dff3cb2
update tests
pranav-super Dec 30, 2024
f4853ad
update tests
pranav-super Jan 6, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,20 @@ void afterAll() throws SQLException, IOException, InterruptedException {
final static String DERIVATION_GROUP = "Test Default";
final static String EVENT_TYPE = "Test";
final static String CREATED_AT = "2024-01-01T00:00:00Z";
final static String EMPTY_ATTRIBUTES = "{}";
final static String EMPTY_ATTRIBUTE_SCHEMA = "{}";
//endregion


//region Records
protected record ExternalEvent(String key, String event_type_name, String source_key, String derivation_group_name, String start_time, String duration) {
protected record ExternalEvent(String key, String event_type_name, String source_key, String derivation_group_name, String start_time, String duration, String attributes) {
// no attributes
ExternalEvent(String key, String event_type_name, String source_key, String derivation_group_name, String start_time, String duration) {
this(key, event_type_name, source_key, derivation_group_name, start_time, duration, "{}");
}

ExternalEvent(String key, String start_time, String duration, ExternalSource source) {
this(key, EVENT_TYPE, source.key(), source.derivation_group_name(), start_time, duration);
this(key, EVENT_TYPE, source.key(), source.derivation_group_name(), start_time, duration, "{}");
}
}
protected record ExternalSource(String key, String source_type_name, String derivation_group_name, String valid_at, String start_time, String end_time, String created_at){}
Expand All @@ -73,30 +80,30 @@ protected record PlanDerivationGroup(int plan_id, String derivation_group_name,


//region Helper Functions
protected void insertExternalEventType(String event_type_name) throws SQLException {
protected void insertExternalEventType(String event_type_name, String attribute_schema) throws SQLException {
try(final var statement = connection.createStatement()) {
// create the event type
statement.executeUpdate(
// language=sql
"""
INSERT INTO
merlin.external_event_type
VALUES ('%s');
""".formatted(event_type_name)
VALUES ('%s', '%s');
""".formatted(event_type_name, attribute_schema)
);
}
}

protected void insertExternalSourceType(String source_type_name) throws SQLException {
protected void insertExternalSourceType(String source_type_name, String attribute_schema) throws SQLException {
try(final var statement = connection.createStatement()) {
// create the source type
statement.executeUpdate(
// language=sql
"""
INSERT INTO
merlin.external_source_type
VALUES ('%s');
""".formatted(source_type_name)
VALUES ('%s', '%s');
""".formatted(source_type_name, attribute_schema)
);
}
}
Expand Down Expand Up @@ -301,9 +308,9 @@ protected void upload_source(String dg, boolean skip_types) throws SQLException
if (!skip_types) {
String[] externalEventTypes = {"DerivationA", "DerivationB", "DerivationC", "DerivationD"};
for (String eventType : externalEventTypes) {
insertExternalEventType(eventType);
insertExternalEventType(eventType, EMPTY_ATTRIBUTE_SCHEMA);
}
insertExternalSourceType(SOURCE_TYPE);
insertExternalSourceType(SOURCE_TYPE, EMPTY_ATTRIBUTE_SCHEMA);
}

// Fourth, insert derivation group
Expand Down Expand Up @@ -331,10 +338,10 @@ protected void upload_source(String dg, boolean skip_types) throws SQLException

protected void insertStandardTypes() throws SQLException {
// insert external event type
insertExternalEventType(EVENT_TYPE);
insertExternalEventType(EVENT_TYPE, EMPTY_ATTRIBUTE_SCHEMA);

// insert external source type
insertExternalSourceType(SOURCE_TYPE);
insertExternalSourceType(SOURCE_TYPE, EMPTY_ATTRIBUTE_SCHEMA);

// insert derivation group
insertDerivationGroup(DERIVATION_GROUP, SOURCE_TYPE);
Expand All @@ -357,7 +364,7 @@ class DerivedEventsTests {
* separate, extracted tests that don't really evaluate events and instead just that sources play together
* correctly. As such we test "empty" sources to make sure their overlapped windows work correctly.
* We add events that span a very short DURATION simply so that the sources show up in derived_events, but we aren't
* testing any properties of said events.
* testing any attributes of said events.
*/
@Nested
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
Expand Down Expand Up @@ -1402,7 +1409,7 @@ void rule4() throws SQLException {
}

/**
* This class separates a few tests that don't test a particular rule, but rather general properties of the derivation
* This class separates a few tests that don't test a particular rule, but rather general attributes of the derivation
* process.
*/
@Nested
Expand Down Expand Up @@ -1777,7 +1784,7 @@ void endTimeGEstartTime() throws SQLException {

// add source type and derivation group
// create the source type
insertExternalSourceType(endBeforeStart.source_type_name());
insertExternalSourceType(endBeforeStart.source_type_name(), EMPTY_ATTRIBUTE_SCHEMA);

// create the derivation_group
insertDerivationGroup(endBeforeStart.derivation_group_name(), endBeforeStart.source_type_name());
Expand Down Expand Up @@ -1925,7 +1932,7 @@ void deleteDGwithRemainingSource() throws SQLException {

// insert the source and all relevant groups and types
// create a source type
insertExternalSourceType(SOURCE_TYPE);
insertExternalSourceType(SOURCE_TYPE, EMPTY_ATTRIBUTE_SCHEMA);

// create a Derivation Group
insertDerivationGroup(DERIVATION_GROUP, SOURCE_TYPE);
Expand Down Expand Up @@ -1970,7 +1977,7 @@ void externalSourceTypeMatchDerivationGroup() throws SQLException {

// add types
// create a source type
insertExternalSourceType(SOURCE_TYPE);
insertExternalSourceType(SOURCE_TYPE, EMPTY_ATTRIBUTE_SCHEMA);

// create a Derivation Group
insertDerivationGroup(DERIVATION_GROUP, SOURCE_TYPE);
Expand Down Expand Up @@ -2024,7 +2031,7 @@ void deleteSourceTypeWithRemainingSource() throws SQLException {

// add types
// create a source type
insertExternalSourceType(SOURCE_TYPE);
insertExternalSourceType(SOURCE_TYPE, EMPTY_ATTRIBUTE_SCHEMA);

// create a Derivation Group
insertDerivationGroup(DERIVATION_GROUP, SOURCE_TYPE);
Expand Down Expand Up @@ -2178,7 +2185,7 @@ void associateDerivationGroupWithBasePlan() throws SQLException {
);
final List<PlanDerivationGroup> actualResultsInitial = getPlanDerivationGroupAssociations();

// first, check other properties
// first, check other attributes
assertEqualsAsideFromLastAcknowledged(expectedResultsInitial, actualResultsInitial);

// insert a source to the changing derivation group
Expand All @@ -2205,7 +2212,7 @@ void associateDerivationGroupWithBasePlan() throws SQLException {
);
final List<PlanDerivationGroup> actualResults = getPlanDerivationGroupAssociations();

// first, check other properties
// first, check other attributes
assertEquals(expectedResults.size(), actualResults.size());
assertTrue(actualResults.containsAll(expectedResults));

Expand Down Expand Up @@ -2233,7 +2240,7 @@ void associateDerivationGroupWithBasePlan() throws SQLException {
assertTrue(postUpdateResults.get(2).last_acknowledged_at().compareTo(
actualResultsInitial.get(2).last_acknowledged_at()) > 0);

// check the other properties
// check the other attributes
assertEqualsAsideFromLastAcknowledged(actualResultsInitial, postUpdateResults);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ select_permissions:
insert_permissions:
- role: aerie_admin
permission:
columns: [key, event_type_name, source_key, derivation_group_name, start_time, duration]
columns: [key, event_type_name, source_key, derivation_group_name, start_time, duration, attributes]
check: {}
- role: user
permission:
columns: [key, event_type_name, source_key, derivation_group_name, start_time, duration]
columns: [key, event_type_name, source_key, derivation_group_name, start_time, duration, attributes]
check: {}
delete_permissions:
- role: aerie_admin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,7 @@ select_permissions:
insert_permissions:
- role: aerie_admin
permission:
columns: [name]
check: {}
- role: user
permission:
columns: [name]
columns: [name, attribute_schema]
check: {}
delete_permissions:
- role: aerie_admin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ select_permissions:
insert_permissions:
- role: aerie_admin
permission:
columns: [key, source_type_name, valid_at, start_time, end_time, derivation_group_name, created_at]
columns: [key, source_type_name, valid_at, start_time, end_time, derivation_group_name, created_at, attributes]
check: {}
set:
owner: "x-hasura-user-id"
- role: user
permission:
columns: [key, source_type_name, valid_at, start_time, end_time, derivation_group_name, created_at]
columns: [key, source_type_name, valid_at, start_time, end_time, derivation_group_name, created_at, attributes]
check: {}
set:
owner: "x-hasura-user-id"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,7 @@ select_permissions:
insert_permissions:
- role: aerie_admin
permission:
columns: [name]
check: {}
- role: user
permission:
columns: [name]
columns: [name, attribute_schema]
check: {}
delete_permissions:
- role: aerie_admin
Expand Down
20 changes: 17 additions & 3 deletions deployment/hasura/migrations/Aerie/11_external_events/up.sql
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
create table merlin.external_source_type (
name text not null,
attribute_schema jsonb not null,

constraint external_source_type_pkey
primary key (name)
Expand All @@ -13,8 +14,12 @@ comment on table merlin.external_source_type is e''
comment on column merlin.external_source_type.name is e''
'The identifier for this external_source_type, as well as its name.';

comment on column merlin.external_source_type.attribute_schema is e''
'The JSON schema used to validate attributes for sources using this source type.';

create table merlin.external_event_type (
name text not null,
attribute_schema jsonb not null,

constraint external_event_type_pkey
primary key (name)
Expand All @@ -26,6 +31,9 @@ comment on table merlin.external_event_type is e''
comment on column merlin.external_event_type.name is e''
'The identifier for this external_event_type, as well as its name.';

comment on column merlin.external_event_type.attribute_schema is e''
'The JSON schema used to validate attributes for events using this event type.';

create table merlin.derivation_group (
name text not null,
source_type_name text not null,
Expand Down Expand Up @@ -64,6 +72,7 @@ create table merlin.external_source (
CHECK (end_time > start_time),
created_at timestamp with time zone default now() not null,
owner text,
attributes jsonb,

constraint external_source_pkey
primary key (key, derivation_group_name),
Expand Down Expand Up @@ -107,6 +116,8 @@ comment on column merlin.external_source.created_at is e''
'This column is used primarily for documentation purposes, and has no associated functionality.';
comment on column merlin.external_source.owner is e''
'The user who uploaded the external source.';
comment on column merlin.external_source.attributes is e''
'Additional data captured from the original external event, in key/pair form.';

-- make sure new sources' source_type match that of their derivation group!
create function merlin.external_source_type_matches_dg_on_add()
Expand Down Expand Up @@ -137,6 +148,7 @@ create table merlin.external_event (
derivation_group_name text not null,
start_time timestamp with time zone not null,
duration interval not null,
attributes jsonb,

constraint external_event_pkey
primary key (key, source_key, derivation_group_name, event_type_name),
Expand Down Expand Up @@ -170,12 +182,14 @@ comment on column merlin.external_event.start_time is e''
'The start time (in _plan_ time, NOT planner time), of the range that this source describes.';
comment on column merlin.external_event.duration is e''
'The span of time of this external event.';
comment on column merlin.external_event.attributes is e''
'Additional data captured from the original external event, in key/pair form.';

create trigger check_external_event_duration_is_nonnegative_trigger
before insert or update on merlin.external_event
for each row
when (new.duration < '0')
execute function util_functions.raise_duration_is_negative();
for each row
when (new.duration < '0')
execute function util_functions.raise_duration_is_negative();

create function merlin.check_external_event_boundaries()
returns trigger
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ create table merlin.external_event (
derivation_group_name text not null,
start_time timestamp with time zone not null,
duration interval not null,
attributes jsonb not null default '{}',

constraint external_event_pkey
primary key (key, source_key, derivation_group_name, event_type_name),
Expand Down Expand Up @@ -38,6 +39,8 @@ comment on column merlin.external_event.start_time is e''
'The start time (in _plan_ time, NOT planner time), of the range that this source describes.';
comment on column merlin.external_event.duration is e''
'The span of time of this external event.';
comment on column merlin.external_event.attributes is e''
'Additional data captured from the original external event, in key/pair form.';

create trigger check_external_event_duration_is_nonnegative_trigger
before insert or update on merlin.external_event
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
create table merlin.external_event_type (
name text not null,
attribute_schema jsonb not null default '{}',

constraint external_event_type_pkey
primary key (name)
Expand All @@ -10,3 +11,6 @@ comment on table merlin.external_event_type is e''

comment on column merlin.external_event_type.name is e''
'The identifier for this external_event_type, as well as its name.';

comment on column merlin.external_event_type.attribute_schema is e''
'The JSON schema used to validate attributes for events using this event type.';
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ create table merlin.external_source (
CHECK (end_time > start_time),
created_at timestamp with time zone default now() not null,
owner text,
attributes jsonb not null default '{}',

constraint external_source_pkey
primary key (key, derivation_group_name),
Expand Down Expand Up @@ -51,6 +52,8 @@ comment on column merlin.external_source.created_at is e''
'This column is used primarily for documentation purposes, and has no associated functionality.';
comment on column merlin.external_source.owner is e''
'The user who uploaded the external source.';
comment on column merlin.external_source.attributes is e''
'Additional data captured from the original external event, in key/pair form.';

-- make sure new sources' source_type match that of their derivation group!
create function merlin.external_source_type_matches_dg_on_add()
Expand Down Expand Up @@ -96,7 +99,7 @@ create function merlin.external_source_pdg_ack_update()
returns trigger
language plpgsql as $$
begin
update merlin.plan_derivation_group set "acknowledged" = false
update merlin.plan_derivation_group set acknowledged = false
where plan_derivation_group.derivation_group_name = NEW.derivation_group_name;
return new;
end;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
create table merlin.external_source_type (
name text not null,
attribute_schema jsonb not null default '{}',

constraint external_source_type_pkey
primary key (name)
Expand All @@ -12,3 +13,6 @@ comment on table merlin.external_source_type is e''

comment on column merlin.external_source_type.name is e''
'The identifier for this external_source_type, as well as its name.';

comment on column merlin.external_source_type.attribute_schema is e''
'The JSON schema used to validate attributes for sources using this source type.';
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ select distinct on (event_key, derivation_group_name)
output.duration,
output.start_time,
output.source_range,
output.valid_at
output.valid_at,
output.attributes
from (
-- select the events from the sources and include them as they fit into the ranges determined by sub
select
Expand All @@ -25,7 +26,8 @@ from (
s.derivation_group_name,
ee.start_time,
s.source_range,
s.valid_at
s.valid_at,
ee.attributes
from merlin.external_event ee
join (
with base_ranges as (
Expand Down
Loading
Loading