feat: Render `entry.go` (second stage) #34

sebastianczech · 2024-03-11T22:07:52Z

Description

PR delivers next stage for generating entry.go:

new functions for template.FuncMap
fix issues for settings default param type (function AddDefaultTypesForParams)
add not_present and from_version for profiles (used in next stage for versioning)
generating structs definitions using new function NestedSpecs
support for nested objects:
- specifier - func SpecifyEntry(o Entry) (any, error)
- normalizer - func (c *EntryXmlContainer) Normalize()
test for new functions
functions comments
feature to render entry.tmpl only if spec is defined for entry (e.g. for NTP or DNS we are NOT rendering entry.go)
extend entry.tmpl

Motivation and Context

#30

How Has This Been Tested?

Automated tests are part of PR

Types of changes

New feature (non-breaking change which adds functionality)

Checklist

I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes if appropriate.
All new and existing tests passed.

…d functions used in templates

…ed specs

…specs

shinmog

So yeah, please no string concatenation. Either do fmt.Sprintf() or a template. There will be lots of templates flying around in the generator, just like there are lots of templates flying around in the scm generator :)

Future you that has to do bug fixes and feature enhancement will appreciate it :)

pkg/translate/funcs.go

pkg/translate/structs.go

…mbers

… the issues from `golangci-lint run --enable-all`

pimielowski

Very good approach! But I will opting for improving this, specially the func.go

pkg/properties/normalized_test.go

pkg/translate/funcs.go

pkg/properties/normalized.go

pimielowski · 2024-03-13T15:07:04Z

pkg/translate/structs.go

+}
+
+// SpecParamType return param type (it can be nested spec) (for struct based on spec from YAML files).
+func SpecParamType(parent string, param *properties.SpecParam) string {


Function XmlParamType and SpecParamType are very similar, you can refactor them with some helper function.

I was thinking about it, but if I combine this 2 functions into one, more general, more if will be required and (IMO) it will be harder to understand.

We could split it into the smaller functions and deduplicate some code from them, it is up to you if you have cycle to take time for it, if no we can think about it a bit later.

I splitted it into the smaller functions and deduplicated some code from them . Are you able to quickly check new approach, please ?

pkg/translate/funcs.go

pimielowski · 2024-03-13T15:37:16Z

pkg/translate/funcs.go

+		}
+		builder.WriteString(fmt.Sprintf("entry.%s = &Spec%s%s{\n", param.Name.CamelCase, param.Name.CamelCase, specSuffix))
+		for _, subParam := range param.Spec.Params {
+			builder.WriteString(nestedObjectAssignment([]string{param.Name.CamelCase}, specSuffix, subParam))


Why not putting this to line 45?

We cannot put this line into 45 line, because before we start entry assignment, we need to declare variables. Here is example output from generator:

nestedProtocolTcpDestinationPort := o.Protocol.Tcp.DestinationPort nestedProtocolTcpSourcePort := o.Protocol.Tcp.SourcePort nestedProtocolTcpOverrideYesTimewaitTimeout := o.Protocol.Tcp.Override.Yes.TimewaitTimeout nestedProtocolTcpOverrideYesTimeout := o.Protocol.Tcp.Override.Yes.Timeout nestedProtocolTcpOverrideYesHalfcloseTimeout := o.Protocol.Tcp.Override.Yes.HalfcloseTimeout nestedProtocolTcpOverrideNo := o.Protocol.Tcp.Override.No nestedProtocolUdpDestinationPort := o.Protocol.Udp.DestinationPort nestedProtocolUdpSourcePort := o.Protocol.Udp.SourcePort nestedProtocolUdpOverrideYesTimeout := o.Protocol.Udp.Override.Yes.Timeout nestedProtocolUdpOverrideNo := o.Protocol.Udp.Override.No entry.Protocol = &SpecProtocolXml{ Tcp: &SpecProtocolTcpXml{ DestinationPort: nestedProtocolTcpDestinationPort, SourcePort: nestedProtocolTcpSourcePort, Override: &SpecProtocolTcpOverrideXml{ Yes: &SpecProtocolTcpOverrideYesXml{ TimewaitTimeout: nestedProtocolTcpOverrideYesTimewaitTimeout, Timeout: nestedProtocolTcpOverrideYesTimeout, HalfcloseTimeout: nestedProtocolTcpOverrideYesHalfcloseTimeout, }, No: nestedProtocolTcpOverrideNo, }, }, ..............

Oh ok, understand, we can try to move some part of this code to different function and just iterate over Param and OneOf, and second function will builder.WriteString operation.

I completely refactored funcs.go . Are you able to quickly check new approach, please ?

pimielowski · 2024-03-13T15:39:22Z

pkg/translate/funcs.go

We should rethink the implementation of prepareAssignment, nestedObjectAssignment to be more manageable. Nevertheless usage of the recursive call is fine :)

should I rebuild in this PR ? If yes, do you have something in mind ?

I mean the implementation is not bad, it actually do what is intend to do, but looks like a bit chaotic. Maybe let's stick to it now, and we can park a refactor for bit later when we have cycle to do this.

I completely refactored funcs.go . Are you able to quickly check new approach, please ?

shinmog

the blast radius of this PR is large.

if there are things you are specifically looking for feedback on, please be sure to bring attention to that somehow

pkg/properties/normalized.go

pkg/translate/funcs.go

pimielowski · 2024-03-14T13:11:42Z

pkg/translate/structs.go

+}
+
+// SpecParamType return param type (it can be nested spec) (for struct based on spec from YAML files).
+func SpecParamType(parent string, param *properties.SpecParam) string {


We could split it into the smaller functions and deduplicate some code from them, it is up to you if you have cycle to take time for it, if no we can think about it a bit later.

pimielowski · 2024-03-14T13:14:35Z

pkg/translate/funcs.go

+		}
+		builder.WriteString(fmt.Sprintf("entry.%s = &Spec%s%s{\n", param.Name.CamelCase, param.Name.CamelCase, specSuffix))
+		for _, subParam := range param.Spec.Params {
+			builder.WriteString(nestedObjectAssignment([]string{param.Name.CamelCase}, specSuffix, subParam))


Oh ok, understand, we can try to move some part of this code to different function and just iterate over Param and OneOf, and second function will builder.WriteString operation.

pimielowski · 2024-03-14T13:17:14Z

pkg/translate/funcs.go

I mean the implementation is not bad, it actually do what is intend to do, but looks like a bit chaotic. Maybe let's stick to it now, and we can park a refactor for bit later when we have cycle to do this.

pimielowski

It looks so much better, great work!!

sebastianczech added 9 commits March 11, 2024 12:42

add EntryXml struct, render all missing functions for entry.go, exten…

6ea5f9d

…d functions used in templates

start work on service.go

4d70add

refactor names in template for entry.go

401a470

add not_present and from_version to SpecParamProfile

53084e3

current service is only prepared for entry

c5ff1b4

clear service template

5da312c

render entry.go only for specs with entry

2e011a7

Merge branch 'main' into render-entry

8e301c0

create structs for nested specs

48db70c

sebastianczech requested review from pimielowski and shinmog and removed request for pimielowski March 11, 2024 22:07

sebastianczech changed the title ~~feat: Render entry.go (second stage)~~ feat: Render entry.go (second stage) Mar 12, 2024

sebastianczech added 3 commits March 12, 2024 13:15

generate content of function SpecifyEntry for specification with nest…

07ccc14

…ed specs

generate content of function Normalize for specification with nested …

60af14e

…specs

add function comments

917a301

shinmog requested changes Mar 12, 2024

View reviewed changes

pkg/translate/funcs.go Show resolved Hide resolved

pkg/translate/structs.go Outdated Show resolved Hide resolved

sebastianczech added 4 commits March 13, 2024 09:48

replace all string concatenation using "+" by function fmt.Sprintf()

2939d86

for item with type object, render []string, which will contain all me…

b51801b

…mbers

refactor NormalizeAssignment(), SpecifyEntryAssignment(), fix some of…

6461db6

… the issues from `golangci-lint run --enable-all`

extend template functions after tests for service.yaml

30bcb0d

pimielowski suggested changes Mar 13, 2024

View reviewed changes

shinmog approved these changes Mar 14, 2024

View reviewed changes

sebastianczech added 5 commits March 14, 2024 13:28

apply changes after review (#1)

743ac40

remove comments

f0e47db

use fmt.Errorf to return errors

a0edfa2

apply changes after review (#2)

e55a5be

refactor SpecMatchesFunction()

f6e4f92

pimielowski reviewed Mar 14, 2024

View reviewed changes

sebastianczech added 2 commits March 15, 2024 09:12

refactor funcs.go

4df52fa

refactor structs.go

0ff2d59

pimielowski approved these changes Mar 15, 2024

View reviewed changes

sebastianczech merged commit 25ec702 into main Mar 15, 2024
1 check passed

sebastianczech deleted the render-entry branch March 15, 2024 11:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Render `entry.go` (second stage) #34

feat: Render `entry.go` (second stage) #34

sebastianczech commented Mar 11, 2024 •

edited

Loading

shinmog left a comment

pimielowski left a comment

pimielowski Mar 13, 2024

sebastianczech Mar 14, 2024

pimielowski Mar 14, 2024

sebastianczech Mar 15, 2024

pimielowski Mar 13, 2024

sebastianczech Mar 14, 2024

pimielowski Mar 14, 2024

sebastianczech Mar 15, 2024

pimielowski Mar 13, 2024

sebastianczech Mar 14, 2024

pimielowski Mar 14, 2024

sebastianczech Mar 15, 2024

shinmog left a comment

pimielowski Mar 14, 2024

pimielowski Mar 14, 2024

pimielowski Mar 14, 2024

pimielowski left a comment

feat: Render entry.go (second stage) #34

feat: Render entry.go (second stage) #34

Conversation

sebastianczech commented Mar 11, 2024 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist

shinmog left a comment

Choose a reason for hiding this comment

pimielowski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shinmog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pimielowski left a comment

Choose a reason for hiding this comment

feat: Render `entry.go` (second stage) #34

feat: Render `entry.go` (second stage) #34

sebastianczech commented Mar 11, 2024 •

edited

Loading