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

We are missing a serialize function in the modular layer #1933

Closed
qiaozha opened this issue Jul 21, 2023 · 0 comments · Fixed by Azure/azure-sdk-for-js#26592 or #1934
Closed

We are missing a serialize function in the modular layer #1933

qiaozha opened this issue Jul 21, 2023 · 0 comments · Fixed by Azure/azure-sdk-for-js#26592 or #1934
Assignees
Labels

Comments

@qiaozha
Copy link
Member

qiaozha commented Jul 21, 2023

In the eventgrid_modular, test case,
we have a CloudEvent model. it's looking like

/** Properties of an event published to an Azure Messaging EventGrid Namespace topic using the CloudEvent 1.0 Schema. */
export interface CloudEvent {
  /** An identifier for the event. The combination of id and source must be unique for each distinct event. */
  id: string;
  /** Identifies the context in which an event happened. The combination of id and source must be unique for each distinct event. */
  source: string;
  /** Event data specific to the event type. */
  data?: any;
  /** Event data specific to the event type, encoded as a base64 string. */
  dataBase64?: Uint8Array;
  /** Type of event related to the originating occurrence. */
  type: string;
  /** The time (in UTC) the event was generated, in RFC3339 format. */
  time?: Date;
  /** The version of the CloudEvents specification which the event uses. */
  specversion: string;
  /** Identifies the schema that data adheres to. */
  dataschema?: string;
  /** Content type of data value. */
  datacontenttype?: string;
  /** This describes the subject of the event in the context of the event producer (identified by source). */
  subject?: string;
}

here we are using the dataBase64 as the property name. But in the rest layer,

/** Properties of an event published to an Azure Messaging EventGrid Namespace topic using the CloudEvent 1.0 Schema. */
export interface CloudEvent {
  /** An identifier for the event. The combination of id and source must be unique for each distinct event. */
  id: string;
  /** Identifies the context in which an event happened. The combination of id and source must be unique for each distinct event. */
  source: string;
  /** Event data specific to the event type. */
  data?: unknown;
  /** Event data specific to the event type, encoded as a base64 string. */
  data_base64?: string;
  /** Type of event related to the originating occurrence. */
  type: string;
  /** The time (in UTC) the event was generated, in RFC3339 format. */
  time?: Date | string;
  /** The version of the CloudEvents specification which the event uses. */
  specversion: string;
  /** Identifies the schema that data adheres to. */
  dataschema?: string;
  /** Content type of data value. */
  datacontenttype?: string;
  /** This describes the subject of the event in the context of the event producer (identified by source). */
  subject?: string;
}

the property name is data_base64, and in the send function, we just assign the modular layer model CloudEvent to the rest layer model

export function _publishCloudEventSend(
  context: Client,
  event: CloudEvent,
  topicName: string,
  options: PublishCloudEventOptions = { requestOptions: {} }
): StreamableMethod<
  PublishCloudEvent200Response | PublishCloudEventDefaultResponse
> {
  return context
    .path("/topics/{topicName}:publish", topicName)
    .post({
      ...operationOptionsToRequestParameters(options),
      contentType:
        (options.contentType as any) ??
        "application/cloudevents+json; charset=utf-8",
      body: { event: event },
    });
}

The reason it could build now, it's because the dataBase64 and data_base64 property are optional.
We should add an internal serialize function for each modular api if there's some propery rename and type mapping.

In this case, there's still another issue,
In the serialize we need to convert the Unit8Array into string.
And in the deserialize, we need to convert the string into Uint8Array.

Another question need to mention is that although the property name and description says it's base64 encoded, its typespec doesn't really add this information.

    @doc("Event data specific to the event type, encoded as a base64 string.")
    data_base64?: bytes;

if we want to test against base64 encoding, we should define a property like

    @doc("Event data specific to the event type, encoded as a base64 string.")
    @encode("base64")
    data_base64?: bytes;
@qiaozha qiaozha self-assigned this Jul 21, 2023
@qiaozha qiaozha added the HRLC label Jul 21, 2023
@qiaozha qiaozha reopened this Aug 2, 2023
@qiaozha qiaozha reopened this Aug 3, 2023
dgetu pushed a commit to Azure/azure-sdk-for-js that referenced this issue Sep 6, 2023
Partially fix Azure/autorest.typescript#1933 
Add some helper function in core-util to support converter from bytes
array to string with different kinds of character encoding. Currently
only support base64 and utf-8 and base64url. which should be able to
work in both Node environment and browser environment.

We will need to wait for typespec team's response to see if we need to
support more. issue tracked here
microsoft/typespec#2204

---------

Co-authored-by: Jeff Fisher <xirzec@xirzec.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
1 participant