Restructure and Cleanup of `DispatchCreateRequest` and more

thomas-nicolai-frequenz commented 5 months ago

What's needed?

We need to restructure the API to improve alignment with the other APIs and make it easier extendable. Moreover we need a more clear seperation of concerns.

Proposed solution

service MicrogridDispatchService {
  // Returns a list of all dispatches
-  rpc ListMicrogridDispatches(DispatchListRequest) returns (DispatchList);
+  rpc ListMicrogridDispatches(ListMicrogridDispatchRequest) 
+         returns (ListMicrogridDispatchResponse);

  // Create a new dispatch
-  rpc CreateMicrogridDispatch(DispatchCreateRequest) returns (google.protobuf.Empty);
+  rpc CreateMicrogridDispatch(CreateMicrogridDispatchRequest) 
+         returns (CreateMicrogridDispatchResponse);

  // Update a dispatch
-  rpc UpdateMicrogridDispatch(DispatchUpdateRequest) returns (google.protobuf.Empty);
+  rpc UpdateMicrogridDispatch(UpdateMicrogridDispatchRequest) 
+         returns (UpdateMicrogridDispatchResponse);

  // Get a single dispatch
-  rpc GetMicrogridDispatch(DispatchGetRequest) returns (Dispatch);
+  rpc GetMicrogridDispatch(GetMicrogridDispatchRequest) 
+         returns (GetMicrogridDispatchResponse);

  // Delete a given dispatch
  rpc DeleteMicrogridDispatch(DispatchDeleteRequest) returns (google.protobuf.Empty);
}

// Message representing one dispatch.
-//
-// Timezone Note: Timestamps are in UTC. It is the responsibility of each microgrid to translate UTC
-// to its local timezone.
message Dispatch {
-  // The dispatch identifier
-  uint64 id = 1;

-  // The microgrid identifier
-  uint64 microgrid_id = 2;

  // The dispatch type.
  // Contains user-defined information about what "type" of dispatch this is.
  // Downstream applications that consume the dispatch API are responsible for
  // understanding and processing this field.
-  string type = 3;
+  string type = 1;

-  // The creation time in UTC
-  // This is set when a dispatch is created via the create request message
-  google.protobuf.Timestamp create_time = 4;

-  // The update time in UTC
-  // This is set when a dispatch is modified via the update request message
-  google.protobuf.Timestamp update_time = 5;

-  // The start time in UTC
+  // The dispatch start time in UTC. 
+  // For reoccuring dispatches this is when the first time execution occurs. When 
+  // creating a dispatch, ensure that the starting timestamp is set to the current 
+  // time or any future time. Timestamps earlier than the current time are not allowed.
-  google.protobuf.Timestamp start_time = 6;
+  google.protobuf.Timestamp start_time = 2;

  // Duration in seconds
-  uint32 duration = 7;
+  optional uint32 duration = 3;

-  // The component selector
+  // Dispatch microgrid component selector.
-  ComponentSelector selector = 8;
+  ComponentSelector selector = 4;

-  // The "active" status
-  // An active dispatch is eligible for processing, either immediately or at a scheduled
-  // time in the future, including recurring dispatches. If a dispatch is set to
-  // inactive, it won't be processed even if it matches all other conditions, allowing
-  // for temporary disabling of dispatches without deletion.
-  bool is_active = 9;
+  bool is_active = 5;

-  // The "dry run" status
-  // A dry run dispatch is executed for logging and monitoring purposes
-  // without affecting the microgrid components. This is useful, for example,
-  // in scenarios where a user may want to test dispatch behavior without
-  // actually affecting any component states.
-  // Notably, a dispatch can be both "dry run" and "active," allowing for
-  // the system to generate logs and observe behavior without making actual changes.
-  bool is_dry_run = 10;
+  bool is_dry_run = 6;

-  // The dispatch payload
-  google.protobuf.Struct payload = 11;
+  google.protobuf.Struct payload = 7;

-  // The recurrence rule
-  RecurrenceRule recurrence = 12;
+  -  RecurrenceRule recurrence = 8;
}

+// Represents a microgrid dispatch with full details, including its ID, state and associated
+// UTC timestamps.
+message DispatchDetail {
+  // Unique identifier of the microgrid dispatch.
+  uint64 dispatch_id = 1;
+
+  // The details of the dispatch.
+  Dispatch dispatch = 2;
+
+  // UTC Timestamp when the order was created.
+  google.protobuf.Timestamp create_time = 3;
+
+  // UTC Timestamp of the last update to the order.
+  google.protobuf.Timestamp modification_time = 4;
+}

// Message to create a new dispatch with the given attributes
-message DispatchCreateRequest {
+message CreateMicrogridDispatchRequest {
  // The microgrid identifier
  uint64 microgrid_id = 1;

-  // The type of dispatch
-  string type = 2;
+  // The details of the dispatch being created.
+  Dispatch dispatch = 2;

-  // The start time
-  // When creating a dispatch, ensure that the starting timestamp is set to
-  // the current time or any future time.
-  // Timestamps earlier than the current time are not allowed.
-  google.protobuf.Timestamp start_time = 3;

-  // Duration in seconds
-  uint32 duration = 4;

-  // The component selector
-  ComponentSelector selector = 5;

-  // The "active" status
-  bool is_active = 6;

-  // The "dry run" status
-  bool is_dry_run = 7;

-  // The dispatch payload
-  google.protobuf.Struct payload = 8;

-  // The recurrence rule
-  RecurrenceRule recurrence = 9;
}

+// Represents the server's response after a new dispatch has been successfully
+// created for a specific microgrid.
+//
+// This response provides essential details about the newly created dispatch, such
+// as the unique dispatch ID and the state of the dispatch.
+message CreateMicrogridDispatchResponse {
+  // ID of the microgrid the dispatch belongs to.
+  uint64 microgrid_id = 1;
+
+  // Details of the newly created microgrid dispatch.
+  DispatchDetail dispatch_detail = 2;
+}

// Message for listing dispatches for a given microgrid, and an optional filter
-message DispatchListRequest {
+message ListMicrogridDispatchRequest {
  // ...
}

-// A list of dispatches
+// Response from listing dispatches for a given microgrid.
-message DispatchList {
+message ListMicrogridDispatchResponse {
+  // ID of the microgrid the list of dispatches belongs to.
+  uint64 microgrid_id = 1;
+  
-  // The dispatches
+  // List of all dispatches with their details.
-   repeated Dispatch dispatches = 1;
+  repeated DispatchDetail dispatch_detail = 2;
}

-// Message to get a single dispatch by its ID
+// // Request to retrieve a single dispatch for a given microgrid.
-message DispatchGetRequest {
+message GetMicrogridDispatchRequest {
+  // ID of the microgrid the dispatch belongs to.
+  uint64 microgrid_id = 1;

  // The dispatch identifier
-  uint64 id = 1;
+  uint64 dispatch_id = 2;
}

+// Response from requesting order details for a specific order.
+message GetMicrogridDispatchResponse {
+  // ID of the microgrid the dispatch belongs to.
+  uint64 microgrid_id = 1;
+
+  // Details of the retrieved dispatch.
+  DispatchDetail dispatch_detail = 2;
+}

-// Message to update the dispatch with the given ID, with the given attributes
+// Request to update an existing dispatch for a given microgrid.
-message DispatchRequest {
+message UpdateMicrogridDispatchRequest {
  // Message containing the updated dispatch attributes
  message DispatchUpdate {
      // ...
  }

+  // ID of the microgrid the dispatch belongs to.
+  uint64 microgrid_id = 1;
+  
   // The dispatch identifier
-  uint64 id = 1;
+  uint64 dispatch_id = 2;

  // Field mask specifying which fields should be updated
-  google.protobuf.FieldMask update_mask = 2;
+  google.protobuf.FieldMask update_mask = 3;

  // The updated dispatch attributes
-  DispatchUpdate update = 3;
+  DispatchUpdate update = 4;
}

+// Response from updating an existing dispatch for a given microgrid.
+message UpdateMicrogridDispatchResponse {
+  // ID of the microgrid the dispatch belongs to.
+  uint64 microgrid_id = 1;
+
+  // Details of the updated dispatch.
+  DispatchDetail dispatch_detail = 2;
+}

Use cases

No response

Alternatives and workarounds

No response

Additional context

Additional change to the duration field that can be incorporated into this PR is https://github.com/frequenz-floss/frequenz-api-dispatch/issues/162 and https://github.com/frequenz-floss/frequenz-api-dispatch/issues/151

llucax commented 5 months ago

This is a long diff. Am I understanding correctly that these are only changes in name and structure, so basically the old and the new stuff can still be mapped 1-1?

Marenz commented 4 months ago

The numbers in the diff change/reassign things to different names, but as far as I recall, in protobuf you should avoid re-assigning numbers as much as possible.

I assume that is also what Luca is referring to?

llucax commented 4 months ago

Nope. Reassigning numbers in protobuf should be definitely be avoided, as it will fuck up everything, but since we are still in "development" mode I think it is OK, it also happened in the microgrid and common APIs. I was just trying to make sure this change is only about changing names/protobuf IDs but there is no really new feature added or removed, that they are just cosmetic changes.

Marenz commented 3 months ago

This is a long diff. Am I understanding correctly that these are only changes in name and structure, so basically the old and the new stuff can still be mapped 1-1?

No, this also modifies e.g. the response message from Create, and List

frequenz-floss / frequenz-api-dispatch