Gitea: Topics added using API don't display on explore page

[mooror]

• Gitea version: 1.12.3
• Git version: 2.7.4
• Operating system: Ubuntu 16.04 (xenial) LTS
• Database (use [x]):
  
  
  
  • [ ] PostgreSQL
  
  
  • [x] MySQL
  
  
  • [ ] MSSQL
  
  
  • [ ] SQLite
  
  
• Can you reproduce the bug at https://try.gitea.io:
  
  
  
  • [ ] Yes (provide example URL)
  
  
  • [ ] No
  
  
  • [x] Not relevant
  
  

Description

If you use the /repos/{owner}/{repo}/topics/{topic} API route to add a topic to a repository it will not be displayed on the explore page, but will show up properly on the repository page.

Steps to reproduce

1. Go to the API route on your Gitea installation (/api/swagger)
2. Click the Authorize button and provide some form of authentication (if the repository is private)
3. Expand the "Add a topic to a repository" section (under the "Repository" heading)
4. Click the "Try it out" button, fill in the fields, and click "Execute"
5. Go to the explore page and find the repository that you have added a topic to (notice that the topic is not displayed)
6. Click on the repository to go to its repository page (notice that the topics were added successfully)

Screenshots

[mooror]

This issue does not apply to the the /repos/{owner}/{repo}/topics API route. If you use that route to update the entire topic list for a repository the new topics will show up on the explore page after a page refresh.

[mooror]

I also just confirmed that this problem occurs with repositories that have previously added topics.

Previously I had only tested this with repositories that didn't have any topics at all.

[mooror]

Another interesting thing to note, if you add a topic using the web interface or using the /repos/{owner}/{repo}/topics API route the "last updated" time gets changed which will bump the repository to the top of the explore page. However, using the /repos/{owner}/{repo}/topics/{topic} API route will add a new topic to the list but will NOT change the "last updated" value or bump the repository up to the top of the explore page.

[jolheiser]

I think I've figured out what's happening. Topics can be manipulated with multiple funcs, but not all are equal. 😅

Using the topics endpoint or the UI will invoke
https://github.com/go-gitea/gitea/blob/502e38c33c08cc6e9e7df86a934eb8b47ae26c49/models/topic.go#L242-L318

Whereas the API invokes
https://github.com/go-gitea/gitea/blob/502e38c33c08cc6e9e7df86a934eb8b47ae26c49/models/topic.go#L212-L224
https://github.com/go-gitea/gitea/blob/502e38c33c08cc6e9e7df86a934eb8b47ae26c49/models/topic.go#L100-L129

The top code invokes the bottom, but also seems to update the repo table as well, because we store topics on the repo table _as well as_ the topics table. (Presumably to save on queries later??)

[mooror]

@jolheiser Hmm, I see what you mean. That's tricky. Ideally you'd have one function to rule them all (Lord of the Rings style :wink: ). Do you think this will just be a simple function call replacement? Or are there other complications?

[jolheiser]

@mooror This should be easy to fix. Unfortunately we'll need to retrieve the topics for a repo, add/delete a topic, then serialize it back to the DB unless there's more context further up.

[mooror]

Unfortunately we'll need to retrieve the topics for a repo, add/delete a topic, then serialize it back to the DB unless there's more context further up.

Are you saying we will have to up the number of database queries for this API route?

[jolheiser]

At a glance I think so. Unless we have the data available earlier in the code. I've only taken a glance so far.

[mooror]

@jolheiser In this case, you gotta do what you gotta do I suppose.

Also I wish I could be of more use to you in fixing this issue but I haven't fully learned GO lang quite yet. Its on the list :smile:

[zeripath]

@jolheiser:

Why do we still have the this Topics field?

If we still need it then I think:

diff --git a/models/topic.go b/models/topic.go
index 4a5bffa08..7f7c7cb9a 100644
--- a/models/topic.go
+++ b/models/topic.go
@@ -197,10 +197,13 @@ func FindTopics(opts *FindTopicOptions) (topics []*Topic, err error) {

 // GetRepoTopicByName retrives topic from name for a repo if it exist
 func GetRepoTopicByName(repoID int64, topicName string) (*Topic, error) {
+   return getRepoTopicByName(x, repoID, topicName)
+}
+func getRepoTopicByName(e Engine, repoID int64, topicName string) (*Topic, error) {
    var cond = builder.NewCond()
    var topic Topic
    cond = cond.And(builder.Eq{"repo_topic.repo_id": repoID}).And(builder.Eq{"topic.name": topicName})
-   sess := x.Table("topic").Where(cond)
+   sess := e.Table("topic").Where(cond)
    sess.Join("INNER", "repo_topic", "repo_topic.topic_id = topic.id")
    has, err := sess.Get(&topic)
    if has {
@@ -211,7 +214,12 @@ func GetRepoTopicByName(repoID int64, topicName string) (*Topic, error) {

 // AddTopic adds a topic name to a repository (if it does not already have it)
 func AddTopic(repoID int64, topicName string) (*Topic, error) {
-   topic, err := GetRepoTopicByName(repoID, topicName)
+   sess := x.NewSession()
+   if err := sess.Begin(); err != nil {
+       return nil, err
+   }
+
+   topic, err := getRepoTopicByName(sess, repoID, topicName)
    if err != nil {
        return nil, err
    }
@@ -220,7 +228,25 @@ func AddTopic(repoID int64, topicName string) (*Topic, error) {
        return topic, nil
    }

-   return addTopicByNameToRepo(x, repoID, topicName)
+   topic, err = addTopicByNameToRepo(sess, repoID, topicName)
+   if err != nil {
+       return nil, err
+   }
+
+   topicNames := make([]string, 0, 25)
+   if err := sess.Table("topic").Cols("name").
+       Join("INNER", "repo_topic", "repo_topic.topic_id = topic.id").
+       Where("repo_topic.repo_id = ?", repoID).Desc("topic.repo_count").Find(&topicNames); err != nil {
+       return nil, err
+   }
+
+   if _, err := sess.ID(repoID).Cols("topics").Update(&Repository{
+       Topics: topicNames,
+   }); err != nil {
+       return nil, err
+   }
+
+   return topic, sess.Commit()
 }

 // DeleteTopic removes a topic name from a repository (if it has it)

would probably fix this but we'd also need a migration to ensure that the repository.Topics are up-to-date.

That would probably need to use something like SQLITEs GROUP_CONCAT to set the Topics.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. I am here to help clear issues left open even if solved or waiting for more insight. This issue will be closed if no further activity occurs during the next 2 weeks. If the issue is still valid just add a comment to keep it alive. Thank you for your contributions.

[mooror]

Thanks @lafriks for keeping the issue from going stale

[mooror]

Thank you @zeripath for working on fixing this issue