You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@synapse.apache.org by GitBox <gi...@apache.org> on 2022/09/25 14:39:51 UTC
[GitHub] [synapse] isudana commented on a diff in pull request #55: Add environment variable injection for SOAP and WSDL endpoints
isudana commented on code in PR #55:
URL: https://github.com/apache/synapse/pull/55#discussion_r979414432
##########
modules/core/src/main/java/org/apache/synapse/config/xml/endpoints/AddressEndpointFactory.java:
##########
@@ -59,6 +62,8 @@
*/
public class AddressEndpointFactory extends DefaultEndpointFactory {
+ private static final String SYSTEM_VARIABLE_PREFIX = "$SYSTEM";
Review Comment:
have we used this constant anywhere in the code?
##########
modules/core/src/main/java/org/apache/synapse/endpoints/WSDLEndpoint.java:
##########
@@ -93,15 +105,27 @@ public String getServiceName() {
return serviceName;
}
+ public String getGetOriginalWsdlServiceName() {
Review Comment:
add javadoc comment
##########
modules/core/src/main/java/org/apache/synapse/endpoints/WSDLEndpoint.java:
##########
@@ -93,15 +105,27 @@ public String getServiceName() {
return serviceName;
}
+ public String getGetOriginalWsdlServiceName() {
+ return this.getOriginalWsdlServiceName;
+ }
+
public void setServiceName(String serviceName) {
- this.serviceName = serviceName;
+ this.getOriginalWsdlServiceName = serviceName;
+ String fetchedServiceName = ConfigUtils.fetchEnvironmentVariables(serviceName);
+ this.serviceName = fetchedServiceName;
}
public String getPortName() {
return portName;
}
+ public String getOriginalWsdlPort() {
+ return this.originalWsdlPort;
+ }
+
public void setPortName(String portName) {
- this.portName = portName;
+ this.originalWsdlPort = portName;
+ String fetchedPortName = ConfigUtils.fetchEnvironmentVariables(portName);
Review Comment:
same comment as AddressEndpoint. fetchedPortName local variable is not required.
##########
modules/core/src/main/java/org/apache/synapse/endpoints/WSDLEndpoint.java:
##########
@@ -77,8 +83,14 @@ public String getWsdlURI() {
return wsdlURI;
}
+ public String getOriginalWsdlURI() {
+ return this.originalWsdlURI;
+ }
+
public void setWsdlURI(String wsdlURI) {
- this.wsdlURI = wsdlURI;
+ this.originalWsdlURI = wsdlURI;
+ String fetchedURI = ConfigUtils.fetchEnvironmentVariables(wsdlURI);
Review Comment:
same comment as AddressEndpoint. fetchedURI local variable is not required.
##########
modules/core/src/main/java/org/apache/synapse/config/xml/endpoints/AddressEndpointFactory.java:
##########
@@ -59,6 +62,8 @@
*/
public class AddressEndpointFactory extends DefaultEndpointFactory {
+ private static final String SYSTEM_VARIABLE_PREFIX = "$SYSTEM";
+ private static final Log LOG = LogFactory.getLog(AddressEndpointFactory.class);
Review Comment:
are we using this anywhere in the code?
##########
modules/core/pom.xml:
##########
@@ -160,6 +160,9 @@
<value>org.apache.xerces.jaxp.validation.XMLSchemaFactory</value>
</property>
</systemProperties>
+ <environmentVariables>
+ <SOAP_SERVICE_TEST>http://localhost:9000/services/SimpleStockQuoteService</SOAP_SERVICE_TEST>
Review Comment:
Is this system variable getting set only when we are running tests? If not can we limit the scope to tests?
##########
pom.xml:
##########
@@ -1115,6 +1119,15 @@
<enabled>false</enabled>
</snapshots>
</repository>
+ <repository>
Review Comment:
what's reason for adding this repo?
##########
modules/core/src/main/java/org/apache/synapse/endpoints/EndpointDefinition.java:
##########
@@ -208,7 +216,18 @@ public String getAddress(MessageContext messageContext) {
* @param address the absolute address to be used
*/
public void setAddress(String address) {
- this.address = address;
+ this.originalAddress = address;
+ String fetchedAddress = ConfigUtils.fetchEnvironmentVariables(address);
Review Comment:
We don't need fetchedAddress local variable. We can directly assign it to this.address.
##########
pom.xml:
##########
@@ -281,6 +281,11 @@
<version>2.11</version>
<configuration>
<argLine>-Xms128m -Xmx256m</argLine>
+ <environmentVariables>
+ <SOAP_SERVICE_TEST>http://localhost:9000/services/MyService1</SOAP_SERVICE_TEST>
Review Comment:
Is this system variable getting set only when we are running tests? If not can we limit the scope to tests?
##########
modules/core/src/main/java/org/apache/synapse/config/xml/endpoints/utils/ConfigUtils.java:
##########
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.synapse.config.xml.endpoints.utils;
+
+import org.apache.synapse.SynapseConstants;
+
+public class ConfigUtils {
+
+ public static String fetchEnvironmentVariables(String parameter) {
+ String injectedValue = parameter;
+ if (parameter.contains(SynapseConstants.SYSTEM_VARIABLE_PREFIX)) {
Review Comment:
shall we use startsWith() instead of contains?
##########
modules/core/src/main/java/org/apache/synapse/endpoints/WSDLEndpoint.java:
##########
@@ -93,15 +105,27 @@ public String getServiceName() {
return serviceName;
}
+ public String getGetOriginalWsdlServiceName() {
+ return this.getOriginalWsdlServiceName;
+ }
+
public void setServiceName(String serviceName) {
- this.serviceName = serviceName;
+ this.getOriginalWsdlServiceName = serviceName;
+ String fetchedServiceName = ConfigUtils.fetchEnvironmentVariables(serviceName);
Review Comment:
same comment as AddressEndpoint. fetchedServiceName local variable is not required.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@synapse.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@synapse.apache.org
For additional commands, e-mail: dev-help@synapse.apache.org