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.

Sign up for GitHub

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

Jump to bottom

#975 - Replace /content? endpoint by /content/scan? #976

Closed
wants to merge 3 commits into from
Closed

#975 - Replace /content? endpoint by /content/scan? #976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c813902
Replace /content? endpoint by /content/scan?
johannesloibl May 17, 2023
acf6ce9
Fix style
johannesloibl Jun 4, 2024
c969b38
Replace /content/scan by /content/search
johannesloibl Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions sphinxcontrib/confluencebuilder/publisher.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,12 +550,13 @@ def get_page(self, page_name, expand='version', status='current'):
'title': page_name,
})
else:
rsp = self.rest.get(f'{self.APIV1}content', {
'type': 'page',
'spaceKey': self.space_key,
'title': page_name,
'status': status,
'expand': expand,
# Workaround for https://jira.atlassian.com/browse/CONFSERVER-57639:
# Hitting the base Content API endpoint can cause performance problem for large instances
rsp = self.rest.get(f'{self.APIV1}content/search', {
"cql": f"title='{page_name}'",
"cqlcontext": json.dumps({"contentStatuses": [status], "spaceKey": self.space_key}),
"expand": ["version"],
"limit": 1
})

if rsp['results']:
Expand Down
7 changes: 7 additions & 0 deletions sphinxcontrib/confluencebuilder/storage/translator.py
View file
Open in desktop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i forgot this is another local fix i did. Feel free to cherry-pick this if you can reproduce the problem ;)

Copy link
Member

Choose a reason for hiding this comment

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

I'd like to investigate this more and generate some unit testing around it. Anchor/reference management has not always been perfect in this extension in the past, and been trying to force the addition of checks to prevent any regressive that has been seen in a couple of releases.

If you wanted to re-submit in another pull request (with a DCO), I could try to extend off it with some tests (when I have the time to check some things). If you don't, no worries. I have noted the issue you have mentioned and will try to address this in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I won't update the PR then, just take the changes how you like ;)

Copy link
Member

Choose a reason for hiding this comment

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

@johannesloibl, by chance, would you happen to have a small sample document(s) showing this case? Just starting looking into it and have not yet been able to reproduce when creating multiple labels on heading and playing with a couple of reference scenarios.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could reproduce it like this:

Page Title
##########

.. _abc:

Foo
---

bla

:ref:`abc`

:ref:`abc`

:ref:`abc`

Bar
---

bla

Without the fix, the 3 references abc will result in a link suffix Foo.1, which is not existing, but Foo is.

Copy link
Member

Choose a reason for hiding this comment

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

Is there any additional configuration entries/extensions used to produce this result? Trying a local test with the provided sample document does not appear to generate links with the mention suffix.

Output 
[v1]
----

<h2>Foo</h2>
<p>bla</p><p><ac:link ac:anchor="Foo">
<ac:link-body>Foo</ac:link-body>
</ac:link></p><p><ac:link ac:anchor="Foo">
<ac:link-body>Foo</ac:link-body>
</ac:link></p><p><ac:link ac:anchor="Foo">
<ac:link-body>Foo</ac:link-body>
</ac:link></p><h2>Bar</h2>
<p>bla</p>

[v2]
----

<h2><ac:structured-macro ac:name="anchor">
<ac:parameter ac:name="">Foo</ac:parameter>
</ac:structured-macro><ac:structured-macro ac:name="anchor">
<ac:parameter ac:name="">PageTitle-Foo</ac:parameter>
</ac:structured-macro>Foo</h2>
<p>bla</p><p><a href="#Foo">Foo</a></p><p><a href="#Foo">Foo</a></p><p><a href="#Foo">Foo</a></p><h2><ac:structured-macro ac:name="anchor">
<ac:parameter ac:name="">Bar</ac:parameter>
</ac:structured-macro><ac:structured-macro ac:name="anchor">
<ac:parameter ac:name="">PageTitle-Bar</ac:parameter>
</ac:structured-macro>Bar</h2>
<p>bla</p>

[report]
--------

(system)
 platform: Windows-10-10.0.19045-SP0
   python: 3.12.0 (tags/v3.12.0:0fb18b0, Oct 2 2023, 13:03:39) [MSC v.1935 64 bit (AMD64)]
   sphinx: 7.3.7
 docutils: 0.20.1
 requests: 2.31.0
  urllib3: 2.1.0
  builder: 2.6.0.dev0

(configuration)
confluence_editor: v2
confluence_publish: True
confluence_server_pass: (set)
confluence_server_url: https://(removed) (cloud)
confluence_server_user: (set)
confluence_space_key: (set; upper)

(confluence instance)
 connected: yes
   fetched: yes
   decoded: yes
    parsed: yes
      type: confluence
   version: 1000.0.0-b93331b30cee
     build: 6452

Copy link
Contributor Author

@johannesloibl johannesloibl Jun 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

good that you ask :D I just found it the problem only exists if i use the singleconfluence builder. The confluence builder works as expected!
Other than that i'm using this config:

"confluence_publish": True,
"confluence_server_url": ...,
"confluence_editor": "v1",
"singleconfluence_toctree": True,
"confluence_page_hierarchy": True,
"confluence_ignore_titlefix_on_index": False,
"confluence_publish_dryrun": False,
"confluence_page_generation_notice": True,
"confluence_publish_delay": 0.25,
"space_key": ...,
"parent_page": ...,
"publish_prefix": ...,
"confluence_request_session_override": <a function to b64 encode user pw for basic auth>,
"confluence_adv_ignore_nodes": ("PassthroughTextElement",)  # fix to work with sphinx-design

Copy link
Member

Choose a reason for hiding this comment

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

Applied a change (#1016) which should address issues when using singleconfluence. If willing to test out the development build and providing any feedback, it would be appreciated.

Original file line number Diff line number Diff line change
Expand Up @@ -1279,6 +1279,13 @@ def _visit_reference_intern_id(self, node):
# identifier value instead
target = self.state.target(anchorname)
if target:
# Johannes Loibl: If multiple anchors are generated for the same reference (e.g. when an explicit reference
# is placed directly before a heading, Sphinx will generate two anchors and increase the suffix counter,
# e.g. HEADING, HEADING.1, HEADING.2, ... . This leads to a wrong naming of the final anchor link,
# since the heading can only be accessed via the root name (HEADING in this case).
# So we have to strip off the number suffix
if "." in target:
target = target.split(".")[0]
anchor_value = target
anchor_value = self.encode(anchor_value)
else:
Expand Down
Loading