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

Bug with the tag uniqueness check #30895

Open
2 of 3 tasks
zverok opened this issue Nov 11, 2024 · 2 comments
Open
2 of 3 tasks

Bug with the tag uniqueness check #30895

zverok opened this issue Nov 11, 2024 · 2 comments
Labels
api Related to the REST API

Comments

@zverok
Copy link

zverok commented Nov 11, 2024

Bug description

NB: I can’t currently run Superset 4 locally, but we backported the TagDAO.create_custom_tagged_objects to our Superset 3, and I assume that the bug affects Superset 4 too (considering the code logic).

Basically, the reproduction is to call POST /api/v1/tags/2/<chart_id> {"properties": {"tags": ["<some-tag>"]}} twice (with the same chart and the same tag). Superset will fail with the tag uniqueness check.

The problem seems to be here:

  • in line 60, a new TaggedObject is created unconditionally (I think this is wrong)
  • ...then, there is an existence check, and then in line 72, the same object is created conditionally.

At least when ported to the Superset 3 codebase, this fails predictably. Maybe Superset 4 somehow prevents that on another layer, but the method looks wrong in any case. (Or I am missing something.)

Screenshots/recordings

No response

Superset version

master / latest-dev

Python version

3.10

Node version

16

Browser

Chrome

Additional context

No response

Checklist

  • I have searched Superset docs and Slack and didn't find a solution to my problem.
  • I have searched the GitHub issue tracker and didn't find a similar bug report.
  • I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.
Copy link

dosubot bot commented Nov 11, 2024

Hey there, @zverok! I'm here to help you with any bugs, questions, or contributions you have related to Superset. Let's tackle this issue together!

Yes, there is indeed a bug in the TagDAO.create_custom_tagged_objects method related to the tag uniqueness check. The method incorrectly appends a TaggedObject to the tagged_objects list before checking if the association already exists. This results in the same TaggedObject being added twice if it does not already exist in the database. The correct logic should be to append the TaggedObject only if it does not already exist.

Here is the problematic code:

tagged_objects.append(
    TaggedObject(object_id=object_id, object_type=object_type, tag=tag)
)

# Check if the association already exists
existing_tagged_object = (
    db.session.query(TaggedObject)
    .filter_by(object_id=object_id, object_type=object_type, tag=tag)
    .first()
)

if not existing_tagged_object:
    tagged_objects.append(
        TaggedObject(object_id=object_id, object_type=object_type, tag=tag)
    )

The first tagged_objects.append should be removed, and the TaggedObject should only be appended if existing_tagged_object is None [1].

To continue talking to Dosu, mention @dosu.

@dosubot dosubot bot added the api Related to the REST API label Nov 11, 2024
@rusackas
Copy link
Member

@eschutho is anyone looking at tags much these days? Maybe @hughhhh?

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

No branches or pull requests

2 participants