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

doc.Unmarshal does not play well with json tag #142

Closed
semolex opened this issue Dec 22, 2023 · 5 comments · Fixed by #143
Closed

doc.Unmarshal does not play well with json tag #142

semolex opened this issue Dec 22, 2023 · 5 comments · Fixed by #143

Comments

@semolex
Copy link

semolex commented Dec 22, 2023

Hello Clover DB team,

I've encountered an issue while integrating Clover DB with the Wails framework. It seems that when using struct tags for JSON unmarshaling alongside Clover's own tags, the unmarshaled struct ends up with default values for certain fields, rather than the values stored in the database.

Environment:

  • Clover DB version: v2.0.0-alpha.3
  • Wails version: v2.7.1
  • Go version: 1.20.6
  • Operating System: macOS Sonoma 14.1.1

Steps to Reproduce

  1. Create a struct with both json and clover tags.
  2. Store a document in Clover DB with non-default values for all fields.
  3. Unmarshal the document into the struct using Clover DB's unmarshaling logic.
func (f *WorkersCollection) GetAllWorkers() ([]Worker, error) {
	workerModel := &Worker{}
	docs, err := f.manager.getAll(workerMode) // just a call to db.FindAll(query.NewQuery(collectionName))

	if err != nil {
		return nil, err
	}
	var workers []Worker

	for _, doc := range docs {
		log.Infof("", doc)
		var worker Worker
		err := doc.Unmarshal(&worker)
		if err != nil {
			return nil, NewCustomError(err.Error(), failedToUnmarshal)
		}
		log.Printf("Unmarshalled worker: %+v\n", worker)
		workers = append(workers, worker)
	}

	return workers, nil
}

Expected Behavior

The struct fields should reflect the values stored in the database.

Actual Behavior

Some fields in the struct are set to their default Go values, despite having different values stored in the database.

Example

  • Document stored in Clover DB:
{
    "_id": "124b0a50-a6ce-4297-8718-69553f9c4222",
    "is_worker": true,
    "balance_actual": 91,
    "balance_by_account": 100,
    "name": "Test if marshalling works",
    "manager_id": "5560100f-52a3-483d-a8c6-3059397d1197"
}
  • Corresponding Go struct:
type Worker struct {
	Id               string `json:"id" clover:"_id"`
	Name             string `json:"name" clover:"name"`
	BalanceActual    int    `json:"balance_actual" clover:"balance_actual"`
	ManagerId        string `json:"manager_id" clover:"manager_id"`
	IsWorker         bool   `json:"is_worker" clover:"is_worker"`
	BalanceByAccount int    `json:"balance_by_account" clover:"balance_by_account"`
}
  • Result after unmarshaling (struct):
    {Id:124b0a50-a6ce-4297-8718-69553f9c4222 Name:Test if marshalling works BalanceActual:0 ManagerId:"" BalanceByAccount:0 IsWorker:false}
    I receive this unmarshaled result on frontend (with json-specified fields e.g. snake_case).

Is there is some undocumented way to use both tags or some designed way to specify those fields?
I could definitely use are maps but in a certain way I am using type validations, possible method calls etc.
Thank you for looking into this issue. I am looking forward to your response or guidance on how to address this problem.

@univac490
Copy link
Contributor

I made a local copy of clover/internal/encoding.go and updated createRenameMap as follows:

func createRenameMap(rv reflect.Value) map[string]string {
	renameMap := make(map[string]string)
	for i := 0; i < rv.NumField(); i++ {
		fieldType := rv.Type().Field(i)

		tagStr, found := fieldType.Tag.Lookup("clover")
		if found {
			name, _ := processStructTag(tagStr)
			jsonTagStr, found := fieldType.Tag.Lookup("json")
			if found {
				jsonName, _ := processStructTag(jsonTagStr)
				if jsonName != name {
					renameMap[name] = jsonName
				}
			} else {
				renameMap[name] = fieldType.Name
			}
		}
	}
	return renameMap
}

@univac490
Copy link
Contributor

actually, the json tag should be always be checked, not just when the clover tag is found.

@ostafen
Copy link
Owner

ostafen commented Dec 23, 2023

@semolex: thank you for reporting the issue.

@univac490: could you submit a PR?

@ostafen ostafen pinned this issue Dec 23, 2023
@ostafen ostafen linked a pull request Dec 24, 2023 that will close this issue
@ostafen
Copy link
Owner

ostafen commented Dec 24, 2023

@semolex: please check if still have the issue.
@univac490: thank you very much for fix :=)

@semolex
Copy link
Author

semolex commented Dec 25, 2023

@ostafen Thanks. Confirmed. Waiting for a next release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants