goadesign / gorma

Storage generation plugin for Goa
http://goa.design
MIT License
140 stars 35 forks source link

Incorrect column name is set by Field method #142

Closed pei0804 closed 7 years ago

pei0804 commented 7 years ago

A character string such as API or XML to be set in Filed is rewritten to a_p_i or x_m_l, causing a phenomenon in which gorm 's column name setting goes wrong. It will be a little longer, but I will explain it in detail.


Example

Field ("api_type", gorma.String)
↓
APIType string `gorm:" column: a_p_i_type "`

Flow that becomes wrong alias Xml_type -> codegen.Goify -> XMLType -> inflect.Undescore -> x_m_l_type

Although I tried to solve this problem, I will report it because a little difficult problem was involved.

Https://github.com/goadesign/gorma/blob/master/dsl/relationalfield.go#L144 The method SanitizeDBFieldName seems to cause this problem.

Https://github.com/goadesign/gorma/blob/master/dsl/relationalfield.go#L38 The previous method called Goify rewrites the character string. For example, if there is a column called api_type, it will be a character string APIType.

Https://github.com/goadesign/gorma/blob/master/dsl/relationalfield.go#L145 The logic to convert to the format of inflect.Undescore is judged based on uppercase letters, so it is converted to the name a_p_i_type and the result alias is completed.

To solve this problem, you need to rewrite the logic of SanitizeDBFieldName using the commonInitialisms = map [string] bool used by Goify. However, we could not refer because of the issue of public range of commonInitialisms, we abandoned improvement. Https://github.com/goadesign/goa/blob/master/goagen/codegen/types.go#L301

If the reference range is changed or a similar variable is created, a solution could be made.


The current situation can be solved by doing as follows.

Field ("api_type", gorma.String, func () {
       Alias ("api_type")
})
raphael commented 7 years ago

Just a off the cuff thought: Is there no way to get to the original attribute name and use that as a parameter for SanitizeDBFieldName?

pei0804 commented 7 years ago

Certainly, it seems to be solved more simply. I will try to code once.

tchssk commented 7 years ago

How about using codegen.SnakeCase instead of inflect.Undescore?

I tested that and it seems to work fine for this issue. You can see the actual tests at https://github.com/tchssk/goa/commit/22573c455933c6e06f0d185c03fad81aab00bb8d.

pei0804 commented 7 years ago

How about using codegen.SnakeCase instead of inflect.Undescore?

There was a problem with HasMany Method. It seems necessary to investigate the cause.

HasMany ("UserFollowPrefs", "UserFollowPref")
↓
PrefID int `gorm:" column: prefid "` // has many Event

https://github.com/goadesign/gorma/pull/143 Although it was clumsy, the pull request code did not cause problems.

https://github.com/tikasan/eventory-goa For your information, I'd post my code.

tchssk commented 7 years ago

It seems necessary to investigate the cause.

Do you mean prefid should be pref_id?

Although it was clumsy, the pull request code did not cause problems.

Thank you for your pull request. Maybe a field name (given to first argument of Field()) is not necessarily snake_case so we have to transform it.

pei0804 commented 7 years ago

Thank you for your pull request. Maybe a field name (given to first argument of Field()) is not necessarily snake_case so we have to transform it.

It was a point I was curious about, so thank you.

Do you mean prefid should be pref_id?

Until now I have written in habit, but when I read various articles, I understood that prefid looks better.

How about using codegen.SnakeCase instead of inflect.Undescore?

From the above, it seems to be solved by the above method. Is it OK if I rewrite the pull request?

tchssk commented 7 years ago

Is it OK if I rewrite the pull request?

Of course! Thank you for doing this.

pei0804 commented 7 years ago

It seems that gorma's test is looking for user_id instead of userid, but does this need to rewrite the test case?

• Failure [0.005 seconds]
RelationalModel with valid DSL with a belongs to relationship [It] sets the creates the foreign key in the child model 
/home/travis/gopath/src/github.com/goadesign/gorma/dsl/relationalmodel_test.go:328
  Expected
      <string>: userid
  to equal
      <string>: user_id
tchssk commented 7 years ago

hmm it seems this change will break the compatibilty so we must try to another way.

pei0804 commented 7 years ago

OK I will consider how to set userID to user_id.

tchssk commented 7 years ago

I got an idea.

The idea is adding a function like this before SnakeCase() so the argument of SanitizeDBFieldName() changes like below.

UserID -> camelCase() -> UserId -> SnakeCase() -> user_id

pei0804 commented 7 years ago

ok! I try it!

pei0804 commented 7 years ago

This problem was solved. Thank you for cooperation! https://github.com/goadesign/gorma/pull/143