Partial support for parsing XML namespaces #3925

ammachado · 2024-01-16T16:41:16Z

What's changed?

Partial support for #3919, including namespace parsing and resolution

What's your motivation?

Support for XML Namespaces #3919

Anything in particular you'd like reviewers to focus on?

No

Anyone you would like to review specifically?

No

Have you considered any alternatives or workarounds?

No

Any additional context

No

Checklist

I've added unit tests to cover both positive and negative cases
I've read and applied the recipe conventions and best practices
I've used the IntelliJ IDEA auto-formatter on affected files

Remove duplicate NonNull; see package-info.java Validate argument not literal Apply formatter

timtebeek · 2024-01-16T19:57:19Z

Thanks for getting this started @ammachado ! I've also tagged @knutwannheden for review since we're adding methods in the xml/tree package which require some additional scrutiny just to be safe.

rewrite-xml/src/main/java/org/openrewrite/xml/internal/XmlNamespaceUtils.java

timtebeek · 2024-01-17T19:21:10Z

Once merged we can revisit these recipes in rewrite-migrate-java:
openrewrite/rewrite-migrate-java@9181655

# Conflicts: # rewrite-xml/src/test/java/org/openrewrite/xml/ChangeNamespaceValueTest.java

rewrite-xml/src/main/java/org/openrewrite/xml/ChangeNamespaceValue.java

knutwannheden · 2024-01-17T10:26:14Z

rewrite-xml/src/main/java/org/openrewrite/xml/tree/Xml.java

+            if (this.root == root) {
+                return this;
+            }
+            Map<String, String> namespaces = XmlNamespaceUtils.extractNamespaces(root.getAttributes());


What is the purpose of extracting the namespaces here?

Would we need this back in to later support namespace lookup for particular elements?

For example, using the test file, and wanting to look for the XPath /client/conduitSelector with the http://cxf.apache.org/jaxws namespace:

<jaxws:client name="{http://cxf.apache.org/hello_world_soap_http}SoapPort" createdFromAPI="true" xmlns:jaxws="http://cxf.apache.org/jaxws"> <jaxws:conduitSelector> <bean class="org.apache.cxf.endpoint.DeferredConduitSelector"/> </jaxws:conduitSelector> </jaxws:client>

…alue.java Co-authored-by: Knut Wannheden <knut.wannheden@gmail.com>

timtebeek · 2024-06-13T16:54:01Z

rewrite-xml/src/main/java/org/openrewrite/xml/internal/Namespaces.java

+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.openrewrite.xml.internal;


Since this is returned from tree.Xml perhaps we should not have this class in an internal package?

timtebeek · 2024-06-13T16:54:42Z

rewrite-xml/src/main/java/org/openrewrite/xml/internal/XmlNamespaceUtils.java

+import java.util.Collection;
+import java.util.Map;
+
+public class XmlNamespaceUtils {


Instead of a separate utils class, could this be methods on Namespaces instead you think?

timtebeek · 2024-06-13T16:57:35Z

rewrite-xml/src/main/java/org/openrewrite/xml/internal/Namespaces.java

+@Value
+public class Namespaces {
+
+    Map<String, String> namespaces = new HashMap<>();


A mutable map here might lead to problems with our immutability convention and detecting changes.

sambsnyd · 2024-06-13T16:54:36Z

rewrite-xml/src/main/java/org/openrewrite/xml/internal/XmlNamespaceUtils.java

+import java.util.Collection;
+import java.util.Map;
+
+public class XmlNamespaceUtils {


These could be methods on the Namespaces class for easier discoverability.

sambsnyd · 2024-06-13T16:58:09Z

rewrite-xml/src/main/java/org/openrewrite/xml/internal/Namespaces.java

+    public Namespaces add(String prefix, String uri) {
+        namespaces.put(prefix, uri);
+        return this;
+    }


Namespaces should be immutable like other parts of the LST

sambsnyd · 2024-06-13T17:00:15Z

It seems like the recipes included in this PR are possible already without introducing new public API surface area in the form of this Namespaces concept. If this PR were just the recipes without the API changes it would be easier to accept.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Got rid of Namespaces class as it is mostly a thin wrapper around Map Merge XmlNamespaceUtils into Xml When you control the definition of the type creating a "utils" class for it makes those methods harder for users to discover than if they were defined on the class itself Moved unit test to use AssertJ assertions to be consistent with our other tests Removed Namespaces field from XPathMatcher because it was unused Sentence-cased recipe metadata

* Partial support for parsing XML namespaces * Adding namespace resolution * Missing license header * Adding recipes to search namespace URIs/prefixes * Namespace shortcut methods on \'Xml.Document\' * Change implementation to rely only on attributes * Javadocs and cleanup * Rename XmlNamespaceUtils & minor polish Remove duplicate NonNull; see package-info.java Validate argument not literal Apply formatter * Fix namespace search on XML hierarchy * `ChangeNamespaceValue` now updates the `schemaLocation` attribute * Consider namespaces on `SemanticallyEqual`. * Suggestions from code review. * Update rewrite-xml/src/main/java/org/openrewrite/xml/ChangeNamespaceValue.java Co-authored-by: Knut Wannheden <knut.wannheden@gmail.com> * Revert namespace comparison changes in `SemanticallyEqual`. * Adding a Namespaces abstraction * Add support for wildcard and local-name() * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Fix `Namespaces` mutability * Adding an iterator implementation for `Namespaces` * Replace `NotNull` with OpenRewrite's `NonNull` * Polish. Got rid of Namespaces class as it is mostly a thin wrapper around Map Merge XmlNamespaceUtils into Xml When you control the definition of the type creating a "utils" class for it makes those methods harder for users to discover than if they were defined on the class itself Moved unit test to use AssertJ assertions to be consistent with our other tests Removed Namespaces field from XPathMatcher because it was unused Sentence-cased recipe metadata --------- Co-authored-by: Knut Wannheden <tim@moderne.io> Co-authored-by: Knut Wannheden <knut.wannheden@gmail.com> Co-authored-by: Evie Lau <689163+evie-lau@users.noreply.github.com> Co-authored-by: Evie Lau <github@evie.ee> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Sam Snyder <sam@moderne.io>

ammachado added 4 commits January 16, 2024 11:08

Partial support for parsing XML namespaces

512d05e

Adding namespace resolution

7b65a68

Missing license header

9d4c924

Adding recipes to search namespace URIs/prefixes

520a282

ammachado marked this pull request as ready for review January 16, 2024 17:26

ammachado mentioned this pull request Jan 16, 2024

Support for XML Namespaces #3919

Closed

ammachado and others added 4 commits January 16, 2024 12:51

Namespace shortcut methods on \'Xml.Document\'

1ec44eb

Change implementation to rely only on attributes

f5a3621

Javadocs and cleanup

aecc3e6

Rename XmlNamespaceUtils & minor polish

385cb7b

Remove duplicate NonNull; see package-info.java Validate argument not literal Apply formatter

timtebeek requested a review from knutwannheden January 16, 2024 19:55

timtebeek assigned ammachado Jan 16, 2024

timtebeek added the enhancement New feature or request label Jan 16, 2024

timtebeek reviewed Jan 16, 2024

View reviewed changes

rewrite-xml/src/main/java/org/openrewrite/xml/internal/XmlNamespaceUtils.java Outdated Show resolved Hide resolved

ammachado added 3 commits January 16, 2024 19:44

Fix namespace search on XML hierarchy

c88e2ae

ChangeNamespaceValue now updates the schemaLocation attribute

546b99f

Merge branch 'main' into add-namespaces-xml-tree

5d2e2b8

timtebeek and others added 3 commits January 18, 2024 00:14

Merge branch 'main' into add-namespaces-xml-tree

e0e9527

Merge branch 'main' into add-namespaces-xml-tree

436049d

# Conflicts: # rewrite-xml/src/test/java/org/openrewrite/xml/ChangeNamespaceValueTest.java

Consider namespaces on SemanticallyEqual.

f621398

knutwannheden reviewed Jan 27, 2024

View reviewed changes

ammachado and others added 7 commits January 27, 2024 15:00

Suggestions from code review.

218827b

Update rewrite-xml/src/main/java/org/openrewrite/xml/ChangeNamespaceV…

a456198

…alue.java Co-authored-by: Knut Wannheden <knut.wannheden@gmail.com>

Revert namespace comparison changes in SemanticallyEqual.

b283aa0

Merge branch 'main' into add-namespaces-xml-tree

5c76231

Merge branch 'main' into add-namespaces-xml-tree

d1cffc1

Merge branch 'main' into add-namespaces-xml-tree

acb7c26

Merge branch 'main' into add-namespaces-xml-tree

6fec0b5

ammachado and others added 2 commits February 27, 2024 00:55

Adding a Namespaces abstraction

bf3b658

Merge branch 'main' into add-namespaces-xml-tree

ac084a1

timtebeek requested review from timtebeek and sambsnyd June 7, 2024 20:18

timtebeek reviewed Jun 13, 2024

View reviewed changes

sambsnyd reviewed Jun 13, 2024

View reviewed changes

evie-lau and others added 2 commits June 13, 2024 17:52

Add support for wildcard and local-name()

326b824

Merge branch 'main' into add-namespaces-xml-tree

50dc26f

This comment was marked as outdated.

Sign in to view

Apply suggestions from code review

20e6212

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

This comment was marked as outdated.

Sign in to view

Fix Namespaces mutability

69d6772

This comment was marked as outdated.

Sign in to view

timtebeek and others added 5 commits June 14, 2024 23:48

Merge branch 'main' into add-namespaces-xml-tree

527a239

Adding an iterator implementation for Namespaces

2a0212b

Replace NotNull with OpenRewrite's NonNull

cd2fcf3

Merge branch 'main' into add-namespaces-xml-tree

c7f84cc

sambsnyd merged commit 7a1b9c6 into openrewrite:main Jun 20, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Partial support for parsing XML namespaces #3925

Partial support for parsing XML namespaces #3925

ammachado commented Jan 16, 2024 •

edited by timtebeek

Loading

timtebeek commented Jan 16, 2024

timtebeek commented Jan 17, 2024

knutwannheden Jan 17, 2024

evie-lau Jun 4, 2024

timtebeek Jun 13, 2024

timtebeek Jun 13, 2024

timtebeek Jun 13, 2024

sambsnyd Jun 13, 2024

sambsnyd Jun 13, 2024

sambsnyd commented Jun 13, 2024 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Partial support for parsing XML namespaces #3925

Partial support for parsing XML namespaces #3925

Conversation

ammachado commented Jan 16, 2024 • edited by timtebeek Loading

What's changed?

What's your motivation?

Anything in particular you'd like reviewers to focus on?

Anyone you would like to review specifically?

Have you considered any alternatives or workarounds?

Any additional context

Checklist

timtebeek commented Jan 16, 2024

timtebeek commented Jan 17, 2024

knutwannheden Jan 17, 2024

Choose a reason for hiding this comment

evie-lau Jun 4, 2024

Choose a reason for hiding this comment

timtebeek Jun 13, 2024

Choose a reason for hiding this comment

timtebeek Jun 13, 2024

Choose a reason for hiding this comment

timtebeek Jun 13, 2024

Choose a reason for hiding this comment

sambsnyd Jun 13, 2024

Choose a reason for hiding this comment

sambsnyd Jun 13, 2024

Choose a reason for hiding this comment

sambsnyd commented Jun 13, 2024 • edited Loading

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

ammachado commented Jan 16, 2024 •

edited by timtebeek

Loading

sambsnyd commented Jun 13, 2024 •

edited

Loading