Embedding separate tool inside Gerrit UI

255 views
Skip to first unread message

pian...@gmail.com

unread,
Mar 1, 2021, 2:55:36 PM3/1/21
to Repo and Gerrit Discussion

Hello!

I apologize in advance for the length of this post.

My organization is upgrading from Gerrit 2.13 to 3.3.  With our current version of Gerrit, we have modified the source code to embed one of our tools (let's call it "hammer") inside the Gerrit UI.  We had first considered creating a Gerrit plugin to do that, but I could not figure out how to do that so we went with modification of the source code.

I added a new section to the UI to contain the hammer tool.  Hammer does some communicating with the Gerrit UI via the postMessage() function to size itself properly vertically and then do a few other things.  To imbed the Hammer section inside the Gerrit UI, we made the following changes to files in gerrit-gwtui/src/main/java/com/google/gerrit/client/change:

ChangeScreen.ui.xml - after the Files section and before the History section, we added this code:
<div id='hammerSectionHeader' class='{style.sectionHeader} {style.headerButtons}'>
  <ui:msg>Hammer</ui:msg>
  <img id='hammerLoadingImage' src='https://our-tools-server.com/img/loading.gif'>
  </img>
</div>
<c:HammerSection ui:field='hammerSection'/>

HammerSection.java - new file that extends FlowPanel.  It has 3 pieces of private data (changeId, project, and branch) and in the set() function which takes a ChangeInfo object as input, it sets that data to the values from the ChangeInfo object.  Then it runs this code:

  HTML oHTML = new HTML("<div id='hammer-frame-wrapper'><iframe id='hammer-frame' src='https://our-tools-server.com/hammer.php?sGerritChangeId=" + this.changeid + "&sGerritProject=" + this.project + "&sGerritBranch=" + this.branch + "&sGerritUser=" + Gerrit.getUserAccount().username() + "'></iframe></div>");

  add(oHTML);

When the Gerrit review page loads, the iframe loads with the hammer tool data that corresponds with the specified Gerrit change id, project, and branch. 

I have been trying to get this to work in Gerrit 3.3 and I have a few problems.  Keep in mind that I am new to Polymer and web components.  I added the following code to polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts right before <paper-tabs id="secondaryTabs"....> because I wanted this to look like just another section in the review:

<paper-tabs id="tertiaryTabs">
  <paper-tab>Hammer</paper-tab>
</paper-tabs>

<section id="hammerSection">
</section>

That change compiled fine, but when the review page tried to load, there were 3 errors in the console from gr-app.js saying "Error: Refused to bind value as RESOURCE_URL: https://our-tools-server/hammer.php?sGerritChangeId=Ic05061da8d78801726db9a08e38cb683c4c54197&sGerritProject=&sGerritBranch=&sGerritUser=pianoman

Note that the URL does have a change id in it so it did use [[_change.change_id]] fine, but the project and branch data was not filled in.  Here are some questions:
  1. Does gr-change-view_html.ts not have permission to use those variables to form a URL inside an iframe? 
  2. Why would it fill in the change id but not the project or branch? 
  3. How would I make it so that code has access to the _change object or is there something else I should use to form that URL to the hammer tool?
Another question - we configured our Gerrit server to use a MySQL database and our hammer tool currently has the capability to query the DB directly.  One of the things it can do is query information about change messages or patch comments using the change message or patch comment uuid.  We modified the comment-added hook to pass along not only the change message id but also a list of all of the ids of line comments that were added at the same time.  The hammer tool is called by that hook and it queries the Gerrit MySQL DB to get info about all of those line comments.  Questions about that:
  1. How would I do this with Gerrit 3.3? 
  2. Can I get direct access to its NotesDB internal database? 
  3. Can I query change messages or line comments using the REST API providing change message or line comment uuids?

Any help would be greatly appreciated.  Thanks!

Nasser Grainawi

unread,
Mar 2, 2021, 3:29:23 PM3/2/21
to pian...@gmail.com, Repo and Gerrit Discussion
In general I’ll defer these to one of the Gerrit UI devs, but I would strongly recommend seeing if you can do this through a simple UI plugin [1] instead of directly modifying the UI code. I think the tabs you’re referring to would be accessible as the change-view-tab-* endpoints [2]. You can see a real plugin using this at [3]. I think that completely solves the challenges of 1/2/3 here since you’ll have a ChangeInfo object provided to your UI plugin and that should have all the fields you need.


Another question - we configured our Gerrit server to use a MySQL database and our hammer tool currently has the capability to query the DB directly.  One of the things it can do is query information about change messages or patch comments using the change message or patch comment uuid.  We modified the comment-added hook to pass along not only the change message id but also a list of all of the ids of line comments that were added at the same time.  The hammer tool is called by that hook and it queries the Gerrit MySQL DB to get info about all of those line comments.  Questions about that:
  1. How would I do this with Gerrit 3.3? 
  1. Can I get direct access to its NotesDB internal database? 
This is discouraged. Writing a Gerrit plugin [6] would be the safest way to get direct access to NoteDb data since a plugin would understand Gerrit types natively.

  1. Can I query change messages or line comments using the REST API providing change message or line comment uuids?
There is a ‘comment:’ search operator [7] that should match comment text. I don’t think it knows about UUIDs for comments though.



Any help would be greatly appreciated.  Thanks!

Hopefully that helped :)

Nasser



--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/5ef01036-d1e2-42bb-be43-9d4d4a95a9d9n%40googlegroups.com.

Ben Rohlfs

unread,
Mar 2, 2021, 4:05:58 PM3/2/21
to Nasser Grainawi, pian...@gmail.com, Repo and Gerrit Discussion
In general I’ll defer these to one of the Gerrit UI devs, ...

Thanks Nasser, but I don't have much to add. You have already summed it up very well. [3] is a very good pointer to learn from an example how you can hook into a custom tab.

Are there any specific reasons why "hammer" needs an iframe?

Does "hammer" by any chance just report build, test or analysis results? Then you should consider using the new Checks UI that will become available in release 3.4 (May?):

You don't have to use Polymer for building web components. For example you can also go native: https://developer.mozilla.org/en-US/docs/Web/Web_Components. Or you could use lit-element: https://lit-element.polymer-project.org/. The latter is also what we are using these days for new Gerrit components.

-Ben

pian...@gmail.com

unread,
Mar 3, 2021, 10:37:03 PM3/3/21
to Repo and Gerrit Discussion
Hi, Ben and Nasser -

First, thank you so much for your replies!  I have not posted here before so I didn't know what to expect - it was great to get not just 1 but 2 responses in very short order so thank you very much!

I can't say a lot about it or probably tell you where I work, but "hammer" is actually our own home-grown review tool (written in PHP) that has a lot of features that our development community is not willing to do without.  It is a standalone tool, but I made lots of changes to it such that it could be embedded inside Gerrit via an iframe.  If it detects that it is inside a frame, it shows specific sections of its UI and hides other parts of the UI that would only be applicable when it is being used as a standalone tool.  It was easiest to make this work inside Gerrit as a frame which is why I decided to use that.

It seems to have problems specifying those variables in the src attribute of the iframe specifically.  I tried adding several other attributes to the iframe element (see attributes sChangeIdAttr, sGerritProjectAttr, sGerritBranchAttr, and sGerritUserAttr in the iframe element below) and it was able to fill in the variable values in those attributes without a problem.  It just doesn't want to fill them in when I reference them in the src attribute.

<iframe id="hammerSectionFrame" src="https://our-tools-server.com/hammer.php?sGerritChangeId=[[_change.change_id]]&sGerritProject=[[_change.project]]&sGerritBranch=[[_change.branch]]&sGerritUser=[[_account.username]]" sChangeIdAttr$="[[_change.change_id]]" sGerritProjectAttr$="[[_change.project]]" sGerritBranchAttr$="[[_change.branch]]" sGerritUserAttr$="[[_account.username]]"></iframe>

Do you think that is a security thing or maybe I have the syntax wrong?  If it is security-related, what if I put hammer on our Gerrit 3 test server and used an iframe to include it from the same server?  then it wouldn't be cross-origin anymore.  Maybe that would fix this.  I'll probably try that within the next few days.

For the other thing with which I am struggling, I'm a bit concerned that I cannot query Gerrit using change message uuids and line comment uuids.  We make extensive use of that.  I'm fine not having the direct DB access, but is there really no way for the query via the REST API to get information about a particular message/comment (or several messages/comments) given a uuid (or several uuids)?  I am guessing it is going to be fairly painful to work around that.

Thanks again, Ben and Nasser!  I look forward to hearing from you again.

Ben Rohlfs

unread,
Mar 4, 2021, 2:27:19 AM3/4/21
to pian...@gmail.com, Repo and Gerrit Discussion
Hi,

I understand where you are coming from, but I still don't think that patching a couple of iframes into Gerrit is a great long-term solution. The problems that you are currently running into will just be the tip of the iceberg compared to what you will be facing over the years.

For general iframe questions this forum is not a good resource. For example I think that you cannot pass data into iframes through element attributes. You have to either use URL parameters in the src or post messages. But I think Stack Overflow knows more about that matter than Gerrit devs do. :-)

Re Rest API: Are you saying that GetComment does not work for you, because you don't have the change-id and the revision-id, but only the comment-id?

-Ben





--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.

bro...@google.com

unread,
Aug 15, 2022, 10:54:54 AM8/15/22
to Repo and Gerrit Discussion
I am considering to remove the `change-view-tab` extension point in https://gerrit-review.googlesource.com/c/gerrit/+/342836. It would make the code base simpler and easier to maintain.

Does anyone still need it?
Reply all
Reply to author
Forward
0 new messages