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.

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

Jump to bottom

Adding cdevents tag name for copying struct fields #82

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

Adding cdevents tag name for copying struct fields #82

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
6 changes: 3 additions & 3 deletions pkg/api/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ type Context struct {
// purpose of the source is to provide global uniqueness for source + id.
// The source MAY identify a single producer or a group of producer that
// belong to the same application.
Source string `json:"source" jsonschema:"required,minLength=1" validate:"uri-reference"`
Source string `json:"source" jsonschema:"required,minLength=1" validate:"uri-reference" cdevents:"context_source"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there some implied naming convention to map a field name and position in the json structure to a string?
So that context.source becomes context_source?

Copy link
Author

Choose a reason for hiding this comment

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

no, but just to differentiate subject's source with the context's source. Keeping tag name as context_source here and for subject's source using it as subject_source


// Spec: https://cdevents.dev/docs/spec/#type
// Description: defines the type of event, as combination of a subject and
Expand All @@ -98,14 +98,14 @@ type Reference struct {

// Spec: https://cdevents.dev/docs/spec/#format-of-subjects
// Description: Uniquely identifies the subject within the source
Id string `json:"id" jsonschema:"required,minLength=1"`
Id string `json:"id" jsonschema:"required,minLength=1" cdevents:"subject_id"`

// Spec: https://cdevents.dev/docs/spec/#format-of-subjects
// Description: defines the context in which an event happened. The main
// purpose of the source is to provide global uniqueness for source + id.
// The source MAY identify a single producer or a group of producer that
// belong to the same application.
Source string `json:"source,omitempty" validate:"uri-reference"`
Source string `json:"source,omitempty" validate:"uri-reference" cdevents:"subject_source"`
}

type SubjectBase struct {
Expand Down
6 changes: 3 additions & 3 deletions tools/templates/event.go.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ var (

type {{.Subject}}{{.Predicate}}SubjectContent struct{
{{ range $i, $field := .Contents }}
{{ .Name }} {{ .Type }} `json:"{{ .NameLower }}{{ if not .Required }},omitempty{{ end }}"{{ if eq .Name "ArtifactId" }} validate:"purl"{{ end }}`
{{ .Name }} {{ .Type }} `json:"{{ .NameLower }}{{ if not .Required }},omitempty{{ end }}"{{ if eq .Name "ArtifactId" }} validate:"purl"{{ end }} cdevents:"{{ .NameLower }}"`
Copy link
Contributor

Choose a reason for hiding this comment

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

The info in the cdevents tag is already in the json tag - I'm not sure I understand the value of adding them to a new tag. Also, this tag here has a different name structure compared to those in the context.

Copy link
Author

Choose a reason for hiding this comment

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

yes, we tried using json tag info for copygen, but the same json tag will also be used for serialize/deserialize and this will create a problem in matching the field names for different sources,

For Example the source ProjectCreated Gerrit event,

type ProjectCreated struct {
	ProjectName string `json:"projectName" cdevents:"name"`
	HeadName    string `json:"headName" cdevents:"subject_id"`
	Type           string  `json:"type"`
	EventCreatedOn float64 `json:"eventCreatedOn"`
	RepoURL        string  `json:"repoURL,omitempty" cdevents:"context_source"`
}

here the json tag names projectName, headName are used to serialize gerrit event.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I'm missing some context or maybe knowledge about copygen to better understand what's going on.

I understand that json tag in types that represent Gerrit events is used to marshal/unmarshal gerrit events to JSON - that's the standard way of doing JSON marshalling/unmarshalling in go.

I guess cdevents tag is meant to tell copygen that it should fill the field of a struct with the CDEvents field specified in the tag. What I don't understand is why we need to add cdevents tags on the CDEvents SDK side.

Copy link
Author

Choose a reason for hiding this comment

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

Actually the same tag should be present in all the structs(Gerrit + CDEvents) to match fields and generate functions to copy fields from Gerrit event to CDEvent.
So we need to have a tag names on CDEvents SDK and the same tag names can be used to match different source events, which can be translated into CDEvents.

An example from copygen shows model struct copied into domain struct, https://github.com/switchupcb/copygen/tree/main/examples/tag#example-tag

{{ end }}
}

Expand Down Expand Up @@ -189,7 +189,7 @@ func New{{.Subject}}{{.Predicate}}EventV{{.VersionName}}(specVersion string) (*{
// {{$.Subject}}{{$.Predicate}}SubjectContent{{ .Name }} holds the content of a {{ .Name }} field in the content
type {{$.Subject}}{{$.Predicate}}SubjectContent{{ .Name }} struct{
{{ range $j, $field := .Fields }}
{{ .Name }} {{ .Type }} `json:"{{ .NameLower }}{{ if not .Required }},omitempty{{ end }}"`
{{ .Name }} {{ .Type }} `json:"{{ .NameLower }}{{ if not .Required }},omitempty{{ end }}" cdevents:"{{ .NameLower }}"`
{{ end }}
}
{{ end }}
{{ end }}
Loading