Skip to content

create Subscription Rule with CorrelationFilter does not create respect properties #22556

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

Merged
YijunXieMS merged 3 commits into from
Jul 1, 2021
Merged

create Subscription Rule with CorrelationFilter does not create respect properties #22556

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b4d4f49
Fix issue#21299 [BUG] create Subscription Rule with CorrelationFilter…
v-hongli1 Jun 25, 2021
78a9c73
Fix comment for issue#21299 [BUG] create Subscription Rule with Corre…
v-hongli1 Jun 28, 2021
4dd525d
Fix comment for issue#21299 [BUG] create Subscription Rule with Corre…
v-hongli1 Jun 30, 2021
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
16 changes: 15 additions & 1 deletion ...in/java/com/azure/messaging/servicebus/implementation/ServiceBusManagementSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ public class ServiceBusManagementSerializer implements SerializerAdapter {
Pattern.MULTILINE);
private static final Pattern FILTER_ACTION_PATTERN = Pattern.compile("<(Filter|Action) type=",
Pattern.MULTILINE);
private static final Pattern FILTER_VALUE_PATTERN = Pattern.compile("<(Value)",
Pattern.MULTILINE);
private static final String RULE_VALUE_ATTRIBUTE_XML = "<$1 xmlns:d6p1=\"http://www.w3.org/2001/XMLSchema\" ns0:type=\"d6p1:string\"";

private final JacksonAdapter jacksonAdapter = new JacksonAdapter();
private final ClientLogger logger = new ClientLogger(ServiceBusManagementSerializer.class);
Expand All @@ -56,14 +59,25 @@ public String serialize(Object object, SerializerEncoding encoding) throws IOExc
}

final String namespace = namespaceMatcher.group("namespace");
final String replaced = contents
String replaced = contents
.replaceAll(namespace + ":", "")
.replace("xmlns:" + namespace + "=", "xmlns=");

if (!CreateRuleBody.class.equals(clazz)) {
return replaced;
}

// This hack is here because value of custom property within RuleFilter should have a namespace like xmlns:d6p1="http://www.w3.org/2001/XMLSchema" ns0:type="d6p1:string".
if (CreateRuleBody.class.equals(clazz)) {
final Matcher filterValue = FILTER_VALUE_PATTERN.matcher(replaced);
if (filterValue.find()) {
replaced = filterValue.replaceAll(RULE_VALUE_ATTRIBUTE_XML);
} else {
logger.warning("Could not find filter name pattern '{}' in {}.", FILTER_VALUE_PATTERN.pattern(),
contents);
}
}

// This hack is here because RuleFilter and RuleAction type="Foo" should have a namespace like n0:type="Foo".
final Matcher filterType = FILTER_ACTION_PATTERN.matcher(replaced);
if (filterType.find()) {
Expand Down
4 changes: 2 additions & 2 deletions ...main/java/com/azure/messaging/servicebus/implementation/models/CorrelationFilterImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ public final class CorrelationFilterImpl extends RuleFilterImpl {
private String contentType;

private static final class PropertiesWrapper {
@JacksonXmlProperty(localName = "KeyValueOfstringanyType")
@JacksonXmlProperty(localName = "KeyValueOfstringanyType", namespace = "http://schemas.microsoft.com/netservices/2010/10/servicebus/connect")
Copy link
Contributor

@YijunXieMS YijunXieMS Jun 25, 2021

Choose a reason for hiding this comment

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

This code is generated so we better not change it manually.
@jianghaolu, @srnagar, @alzimmermsft does the code gen customization support this customization?

Copy link
Member

@alzimmermsft alzimmermsft Jun 25, 2021

Choose a reason for hiding this comment

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

This should be do-able using a Swagger transform to add a namespace sub-property to the XML declaration of keyValueOfstringanyType.

Copy link
Member

@srnagar srnagar Jun 25, 2021

Choose a reason for hiding this comment

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

I agree, this should not be hand-edited. Also, updating the swagger might be useful if all other languages are also doing something similar.

Copy link
Contributor Author

@v-hongli1 v-hongli1 Jun 28, 2021

Choose a reason for hiding this comment

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

This setting is added to correspond to the problem that the namespace attribute of the KeyValueOfstringanyType in the XML is a blank value. If this problem can be corrected through swagger, please inform the location of the swagger configuration file storage.

Copy link
Contributor

@YijunXieMS YijunXieMS Jun 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@alzimmermsft The swagger already has namespace property. But the generated code doesn't have it.
https://github.com/Azure/azure-rest-api-specs/blob/39ffff91d145f6c8f7ec1a6ae46bda17256e8d2e/specification/servicebus/data-plane/servicebus-swagger.json#L1513

    "KeyValue": {
      "description": "Key Values of custom properties",
      "type": "object",
      "xml": {
        "name": "KeyValueOfstringanyType",
        "namespace": "http://schemas.microsoft.com/netservices/2010/10/servicebus/connect"
      },
      "properties": {
        "key": {
          "type": "string",
          "xml": {
            "name": "Key",
            "namespace": "http://schemas.microsoft.com/netservices/2010/10/servicebus/connect"
          }
        },
        "value": {
          "type": "string",
          "xml": {
            "name": "Value",
            "namespace": "http://schemas.microsoft.com/netservices/2010/10/servicebus/connect"
          }
        }
      }
    },

Copy link
Member

@alzimmermsft alzimmermsft Jun 29, 2021

Choose a reason for hiding this comment

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

@srnagar, @jianghaolu, @weidongxu-microsoft, could this be an code generation issue when there is a combination of XML wrapped and namespace attribute, where the namespace attribute doesn't get propagated to the wrapped property?

Copy link
Member

@srnagar srnagar Jun 29, 2021

Choose a reason for hiding this comment

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

Yes, this is a codegen issue. It's missing the namespace when generating the internal wrapper classes. I'll have this fixed.

Copy link
Member

@srnagar srnagar Jun 29, 2021

Choose a reason for hiding this comment

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

PR to fix this - Azure/autorest.java#1081

private final List<KeyValueImpl> items;

@JsonCreator
private PropertiesWrapper(@JacksonXmlProperty(localName = "KeyValueOfstringanyType") List<KeyValueImpl> items) {
private PropertiesWrapper(@JacksonXmlProperty(localName = "KeyValueOfstringanyType", namespace = "http://schemas.microsoft.com/netservices/2010/10/servicebus/connect") List<KeyValueImpl> items) {
this.items = items;
}
}
Expand Down