Skip to content

Hurtki/git 32 username case#30

Merged
hurtki merged 19 commits intomasterfrom
hurtki/git-32-username-case
Mar 19, 2026
Merged

Hurtki/git 32 username case#30
hurtki merged 19 commits intomasterfrom
hurtki/git-32-username-case

Conversation

@hurtki
Copy link
Copy Markdown
Owner

@hurtki hurtki commented Mar 14, 2026

Changes

Username cases bug fix

Before in out system were a lot of problems with username case:

  • long-term banners. When someone was creating a banner, it was created with username user sent to server. And user could create banners with hurtki and HURTKI. And they were updating both.
  • stats service. Stats service had case sensetive cache and repository, that led to going fetching data instead of taking it from cache or db
  • important, that db was always containing correct username ( from github fetcher. ) So all requests to db with not correct username ( with wrong case ) were failing.

How I fixed this:

I clearly set, that domain logic shouldn't know about normalization, but all repos/infrastructures/caches it uses, should work with any case.
So, I refactored:

  • repo level ( username_normalized/owner_username_normalized fields )
  • cache level ( preview cache to use lower case for key computing and stats cache to use lower case as cache key )
  • infratsructure level ( github fetcher already works with any case )

Postgres tables changes:

Create github_data schema to separate github information from our service's data ( users table was very confusing )

users -> github_data.users
repositories -> github_data.repositories

users:

before:
username text primary key

after:
username_normalized text primary key
username text not null

repositories:

before:
owner_username text not null

after:
owner_username_normalized text not null

banners:

before:
github_username text not null + unique ( github_username, banner_type ) 

after:
github_username_normalized text not null + unique ( github_username_normalized, banner_type ) 

As standart I decided to use lower() function every time I can, when inserting, selecting data using sql.
It removes a bit load from go serivce and standardizes normalization place.

HTTP handlers

Clear response errors defined in handlers

Now, handlers, when recognize error, are not using err.Error() for http response. they determine it themselves.
Example:

...
		switch {
		case errors.Is(err, preview.ErrInvalidBannerType):
			h.error(rw, http.StatusBadRequest, "invalid banner type")
		case errors.Is(err, preview.ErrUserDoesntExist):
			h.error(rw, http.StatusNotFound, "user not found on github")
		case errors.Is(err, preview.ErrInvalidInputs):
...

With this approach domain logic ( usecases ) can be not afraid of wrapping infrastructure or any other error before returning it.

Start of docs

I by my hands started writing docs of api ms in architecture.md, I highlighted, in my opinion, the most important points, and I will continue to do so in new PRs.
Fully updated api.yaml file, accoring to current api.
Deleted manual.md, еhere were a lot of confusing things there. This service can be run separately from the others, but it's not as convenient as running everything together in Docker Compose. It might be reworked in the future.

Also global repo README.md should be reworked in near future.

@hurtki hurtki requested a review from R7rainz March 14, 2026 18:07
@hurtki hurtki self-assigned this Mar 14, 2026
@hurtki hurtki added bug Something isn't working enhancement New feature or request labels Mar 14, 2026
Comment on lines +29 to +37
// don't forget to use lower for OwnerUsername for normalization
// 1 (2) 3 4 5 6 7 8
// 9 (10) 11 12 13 14
// ...
if j%8 == 2 {
tempPosArgs = append(tempPosArgs, fmt.Sprintf("lower($%d)", j))
} else {
tempPosArgs = append(tempPosArgs, fmt.Sprintf("$%d", j))
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

for j := i; j < i+8; j++ {
	tempPosArgs = append(tempPosArgs, fmt.Sprintf("$%d", j))
}

i think this part can be shorten by this

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

I mentioned it in PR description, I wanted to lower it only in sql:

As standart I decided to use lower() function every time I can, when inserting, selecting data using sql.
It removes a bit load from go serivce and standardizes normalization place.


I got intresting approach in my head:

As, I understand it's better to have one function in project for username normalization.
And use it from everywhere, but I thought it was a problem to use it in migration.
But, eventually goose has ability to create .go files migrations, where we can use our unified function.

Let me know, what do you think, with this approach we can get rid of lower() functions in SQL.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

yea this is a more modular approach and makes sense with our project architecture as well

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

fine, starting working on it

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

@R7rainz, check out latest commits

I moved two migrations into go format, where used unified function to normalize columns.

Now NormalizeGithubUsername is used all over the project.

@hurtki hurtki force-pushed the hurtki/git-32-username-case branch from 5cd9b1f to 86abe5f Compare March 16, 2026 12:49
@hurtki hurtki force-pushed the hurtki/git-32-username-case branch from baa8fa4 to f134fa6 Compare March 17, 2026 09:15
@hurtki hurtki merged commit 5b37c2f into master Mar 19, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants