[GitHub] rmannibucau commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
rmannibucau commented on a change in pull request #313: [CXF-7493] Add a
default ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139451602
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,42 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.List;
+import static java.util.Arrays.asList;
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final List UNWRAPPED_MARKERS =
asList("OwbNormalScopeProxy", "WeldClientProxy");
Review comment:
For owb you need to check AbstractProxyFactory subclasses out, without more
checking I had in mind $$OwbSubClass. In TomEE we also have $$LocalBeanProxy
which would break probably, and a lot of frameworks use $$ for that purpose.
That is why we use $$ and not something more precise which will always hit a
pitfall. Do you know any framework using $$ and would break cxf unwrapping the
class? If so we can surely just make the suffixes configurable. Throughts?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] rmannibucau commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
rmannibucau commented on a change in pull request #313: [CXF-7493] Add a
default ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139451602
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,42 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.List;
+import static java.util.Arrays.asList;
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final List UNWRAPPED_MARKERS =
asList("OwbNormalScopeProxy", "WeldClientProxy");
Review comment:
For owb you need to check AbstractProxyFactory subclasses out, without more
checking I had in mind $$OwbSubClass. In TomEE we also have $$LocalBeanProxy
which would break probably, and a lot of frameworks use $$ for that purpose.
That is why we use $$ and not something more precise which will always hit a
pitfall. Do you know any framework using $$ and would break cxf unwrapping the
class? If so we can surely just make the suffixes configurable. Thoughts?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] asfgit closed pull request #310: Cache URITemplates instead of creating them over and over resulting i?
asfgit closed pull request #310: Cache URITemplates instead of creating them over and over resulting i? URL: https://github.com/apache/cxf/pull/310 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sberyozkin commented on issue #310: Cache URITemplates instead of creating them over and over resulting i?
sberyozkin commented on issue #310: Cache URITemplates instead of creating them over and over resulting i? URL: https://github.com/apache/cxf/pull/310#issuecomment-330246498 Sorry for a delay, yes, when UriBuilderImpl does "new URiTemplate(...)" it is sensitive to the forward slash being added by default, so your code is fine. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] johnament commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
johnament commented on a change in pull request #313: [CXF-7493] Add a default
ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139439822
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,42 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.List;
+import static java.util.Arrays.asList;
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final List UNWRAPPED_MARKERS =
asList("OwbNormalScopeProxy", "WeldClientProxy");
Review comment:
I have only seen the one suffix from OWB. Where can I find the list of
suffixes? I saw what you did in TomEE, and I think using `$$` could be
problematic for non-CDI frameworks, so I wanted to explicitly list out the
known suffixes.
I'm fine with putting something like this in core, and explicitly activating
it in CDI, but would be concerned with being over eager with the swap out.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] reta commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
reta commented on a change in pull request #313: [CXF-7493] Add a default
ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139557045
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,42 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.List;
+import static java.util.Arrays.asList;
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final List UNWRAPPED_MARKERS =
asList("OwbNormalScopeProxy", "WeldClientProxy");
Review comment:
@johnament @rmannibucau Was going through this discussion
(https://issues.jboss.org/browse/CDI-10), seems to be a known problem people
struggle with. Not sure if it is better or not, what if we detect the CDI
implementation at runtime (class probing) and delegate the unwrapping to the
framework itself? F.e. in case of OWB, we could
use`NormalScopeProxyFactory.unwrapInstance(o)`, something similar should be
available for Weld as well. Not ideal either, but more reliable may be?
Thoughts?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] reta commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
reta commented on a change in pull request #313: [CXF-7493] Add a default
ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139848270
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,40 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.regex.Pattern;
+
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final Pattern PROXY_PATTERN =
Pattern.compile(".+\\$\\$.+Proxy");
Review comment:
Thanks @johnament, I don't think it should be moved outside CDI integration,
@rmannibucau any particular reasons to do that?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] johnament commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
johnament commented on a change in pull request #313: [CXF-7493] Add a default
ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139837797
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,40 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.regex.Pattern;
+
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final Pattern PROXY_PATTERN =
Pattern.compile(".+\\$\\$.+Proxy");
Review comment:
@reta @rmannibucau ok, this uses a pattern based on the discussion. Do we
still want to move outside the CDI module?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] xldai opened a new pull request #314: CXF-7509 The authSupplier property is ignored in case of OSGi http co?
xldai opened a new pull request #314: CXF-7509 The authSupplier property is ignored in case of OSGi http co? URL: https://github.com/apache/cxf/pull/314 ?nduit configuration This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ffang closed pull request #314: CXF-7509 The authSupplier property is ignored in case of OSGi http co?
ffang closed pull request #314: CXF-7509 The authSupplier property is ignored in case of OSGi http co? URL: https://github.com/apache/cxf/pull/314 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] rmannibucau commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
rmannibucau commented on a change in pull request #313: [CXF-7493] Add a
default ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139873530
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,40 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.regex.Pattern;
+
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final Pattern PROXY_PATTERN =
Pattern.compile(".+\\$\\$.+Proxy");
Review comment:
Yep the reason is to ensure it is used by all integrations. Very few server
uses cdi extension to comply to spec but all use proxies. Since the perf and
behavior shouldnt hurt it can be added to the default IMO
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] sberyozkin commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
sberyozkin commented on a change in pull request #313: [CXF-7493] Add a default
ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139917279
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,40 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.regex.Pattern;
+
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final Pattern PROXY_PATTERN =
Pattern.compile(".+\\$\\$.+Proxy");
Review comment:
Hi guys, but the problem is only showing itself on the CDI path ?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] sberyozkin commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
sberyozkin commented on a change in pull request #313: [CXF-7493] Add a default
ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139920692
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,40 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.regex.Pattern;
+
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final Pattern PROXY_PATTERN =
Pattern.compile(".+\\$\\$.+Proxy");
Review comment:
Possibly :-) but Spring users are not opening any issues...
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] rmannibucau commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
rmannibucau commented on a change in pull request #313: [CXF-7493] Add a
default ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139923686
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,40 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.regex.Pattern;
+
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final Pattern PROXY_PATTERN =
Pattern.compile(".+\\$\\$.+Proxy");
Review comment:
isnt it cause it is built-in in cxf core -
https://github.com/apache/cxf/blob/master/core/src/main/java/org/apache/cxf/common/util/ClassHelper.java#L40?
:p
Also spring proxies are old enough to not all use $$ but it tends to be more
and more "standard" to do it for proxies.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] rmannibucau commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
rmannibucau commented on a change in pull request #313: [CXF-7493] Add a
default ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139918596
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,40 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.regex.Pattern;
+
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final Pattern PROXY_PATTERN =
Pattern.compile(".+\\$\\$.+Proxy");
Review comment:
Not really. it has nothing to do with CDI actually except CDI uses proxies.
It will happen each time a lib is using subclassing proxies, CDI is one big
consumer but there are a lot of other ways to get it even in plain standalone.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] sberyozkin commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
sberyozkin commented on a change in pull request #313: [CXF-7493] Add a default
ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139924914
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,40 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.regex.Pattern;
+
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final Pattern PROXY_PATTERN =
Pattern.compile(".+\\$\\$.+Proxy");
Review comment:
Oh, indeed, support for Spring is there, you are right; I'm just not sure
the proposed ClassUnwrapper should be called there by default every time
ClassHelper is called, may be do it in 2 stages, 1) test as part of the CDI 2)
move it to the core ?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] rmannibucau commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
rmannibucau commented on a change in pull request #313: [CXF-7493] Add a
default ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139925631
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,40 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.regex.Pattern;
+
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final Pattern PROXY_PATTERN =
Pattern.compile(".+\\$\\$.+Proxy");
Review comment:
Works for me
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] sberyozkin commented on issue #315: [CXF-7512] Support searching swaggerUi by name and version in all bun?
sberyozkin commented on issue #315: [CXF-7512] Support searching swaggerUi by name and version in all bun? URL: https://github.com/apache/cxf/pull/315#issuecomment-331536096 Hey Christian, thanks, I'll play with it next week. Enjoy a well-deserved break :-) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sberyozkin commented on issue #315: [CXF-7512] Support searching swaggerUi by name and version in all bun?
sberyozkin commented on issue #315: [CXF-7512] Support searching swaggerUi by name and version in all bun? URL: https://github.com/apache/cxf/pull/315#issuecomment-331410470 Would you be OK I applied your name-based resolution code per the above comment for you to see how it will look like ? You may end up liking it :-) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sberyozkin commented on issue #315: [CXF-7512] Support searching swaggerUi by name and version in all bun?
sberyozkin commented on issue #315: [CXF-7512] Support searching swaggerUi by name and version in all bun? URL: https://github.com/apache/cxf/pull/315#issuecomment-331410019 Hi Christian, many thanks. The original pattern looks cleaner to me (we apply it in various places in CXF - proxies - CGLIB or not, and Class resolution and scanning, probably in other places too), as opposed to having the code which is not relevant for the non-OSGI case which will guarantee 2 Throwable will always be thrown and caught and a non-reachable code in the end in the OSGI case if the bundle is installed and of no use - if it's not installed (it checks URLClassLoader), and all of that 'bundled' in a single function body. I'd like to propose that we deprecate 'swaggerUiMavenGroupAndArtifact' and have you new 'swaggerUiName' property being the only property used by the resolvers. We'd be able to use this property in the non-OSGI case too (to find the correct jar if needed). In OSGI case, as I suggested earlier, we just check if the name contains '/' then it is Maven otherwise it is a bundle name only. See many people know the Maven artifacts but finding the bundle name requires more work so if people use Maven artifacts it is just simpler for then to use the artifacts, 'swagger-ui' is not a very uniique value to be honest...Anyway, we'd still be able to use a single OSGI Resolver and be able to skip the Maven related code in the short term if the user sets a swaggerUiName property without '/' This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] cschneider commented on issue #315: [CXF-7512] Support searching swaggerUi by name and version in all bun?
cschneider commented on issue #315: [CXF-7512] Support searching swaggerUi by name and version in all bun? URL: https://github.com/apache/cxf/pull/315#issuecomment-331416753 Hi Sergey. Feel free to edit the code. Please remember though the swaggerUiName is not a bundle name. It is a path below META-INF/resources/webjar. I chose this approach as it can work in OSGi as well as outside. I think the swaggerUiName should only mean this. If you want to still use maven coordinates then I suggest to keep using the existing swaggerUiMavenGroupAndArtifact. Christian This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] cschneider opened a new pull request #315: [CXF-7512] Support searching swaggerUi by name and version in all bun?
cschneider opened a new pull request #315: [CXF-7512] Support searching swaggerUi by name and version in all bun? URL: https://github.com/apache/cxf/pull/315 ?dles This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] johnament commented on issue #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
johnament commented on issue #313: [CXF-7493] Add a default ClassUnwrapper for
CDI integration.
URL: https://github.com/apache/cxf/pull/313#issuecomment-331001682
Sure sounds good. Feel free to merge if good.
On Sep 20, 2017 6:41 PM, "Andriy Redko" <notificati...@github.com> wrote:
> *@reta* commented on this pull request.
> --
>
> In integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
> <https://github.com/apache/cxf/pull/313#discussion_r14036>:
>
> > + *
> + * 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.cxf.cdi;
> +
> +import java.util.regex.Pattern;
> +
> +import org.apache.cxf.common.util.ClassUnwrapper;
> +
> +class CdiClassUnwrapper implements ClassUnwrapper {
> +private static final Pattern PROXY_PATTERN =
Pattern.compile(".+\\$\\$.+Proxy");
>
> @johnament <https://github.com/johnament> Sorry, was not clear, I was
> referring to @sberyozkin <https://github.com/sberyozkin> suggestion for 2
> stages, let us start from CDI first, and than make a decision if it worth
> going to core or not. Make sense?
>
> ?
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/cxf/pull/313#discussion_r14036>, or mute
> the thread
>
<https://github.com/notifications/unsubscribe-auth/AAGmh5HMBhr0c0Q4hzG8G44zuxOsC72Cks5skZSDgaJpZM4Pau6x>
> .
>
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] reta commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
reta commented on a change in pull request #313: [CXF-7493] Add a default
ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r14036
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,40 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.regex.Pattern;
+
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final Pattern PROXY_PATTERN =
Pattern.compile(".+\\$\\$.+Proxy");
Review comment:
@johnament Sorry, was not clear, I was referring to @sberyozkin suggestion
for 2 stages, let us start from CDI first, and than make a decision if it worth
going to core or not. Make sense?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] reta closed pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
reta closed pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration. URL: https://github.com/apache/cxf/pull/313 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] reta commented on issue #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
reta commented on issue #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration. URL: https://github.com/apache/cxf/pull/313#issuecomment-331013732 Thanks @johnament ! This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] reta commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
reta commented on a change in pull request #313: [CXF-7493] Add a default
ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139581187
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,42 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.List;
+import static java.util.Arrays.asList;
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final List UNWRAPPED_MARKERS =
asList("OwbNormalScopeProxy", "WeldClientProxy");
Review comment:
It is definitely the easiest / simplest way to do that, not sure how
reliable it is though ... May be we could have some regex, like
`^.+$$.+Proxy$`, the `$$` usually depict a generated class, so we would cover
`Xxx$$OwbNormalScopeProxy`, `Xxx$$WeldClientProxy` as well as
`Xxx$$LocalBeanProxy` like @rmannibucau pointed out as well. Should be pretty
good heuristic, what do you think?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] johnament commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
johnament commented on a change in pull request #313: [CXF-7493] Add a default
ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139574059
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,42 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.List;
+import static java.util.Arrays.asList;
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final List UNWRAPPED_MARKERS =
asList("OwbNormalScopeProxy", "WeldClientProxy");
Review comment:
Well, I don't think Weld has a behavior like `unwrapInstance` but it could
be done by us if its Weld. I also figured that checking by name would be
easier since it avoid any compile time dependencies.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] nhtzr closed pull request #253: [CXF-7309] JAX-RS @Context fields throw NPE in OSGI hot deployed filters
nhtzr closed pull request #253: [CXF-7309] JAX-RS @Context fields throw NPE in OSGI hot deployed filters URL: https://github.com/apache/cxf/pull/253 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sberyozkin commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
sberyozkin commented on a change in pull request #313: [CXF-7493] Add a default
ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139946094
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,40 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.regex.Pattern;
+
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final Pattern PROXY_PATTERN =
Pattern.compile(".+\\$\\$.+Proxy");
Review comment:
Hi Andriy, if all 3 of you are happy, then yes, please merge :-), I did not
really contribute myself
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] reta commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
reta commented on a change in pull request #313: [CXF-7493] Add a default
ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139943801
##
File path: integration/cdi/src/main/java/org/apache/cxf/cdi/CdiBusBean.java
##
@@ -63,6 +65,9 @@ public String getName() {
@Override
public ExtensionManagerBus create(final CreationalContext<
ExtensionManagerBus > ctx) {
final ExtensionManagerBus instance = injectionTarget.produce(ctx);
+if
("true".equals(SystemPropertyAction.getProperty("org.apache.cxf.cdi.useCdiUnwrapper",
"true"))) {
Review comment:
@johnament pretty minor, before merging could you please rename property
`org.apache.cxf.cdi.useCdiUnwrapper` to `org.apache.cxf.cdi.unwrap.proxies`
(or similar) so it kind of explains its purpose.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] reta commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
reta commented on a change in pull request #313: [CXF-7493] Add a default
ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139943866
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,40 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.regex.Pattern;
+
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final Pattern PROXY_PATTERN =
Pattern.compile(".+\\$\\$.+Proxy");
Review comment:
Sounds like a good plan, @sberyozkin are we good to merge?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] johnament commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
johnament commented on a change in pull request #313: [CXF-7493] Add a default
ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139945372
##
File path:
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiClassUnwrapper.java
##
@@ -0,0 +1,40 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.regex.Pattern;
+
+import org.apache.cxf.common.util.ClassUnwrapper;
+
+class CdiClassUnwrapper implements ClassUnwrapper {
+private static final Pattern PROXY_PATTERN =
Pattern.compile(".+\\$\\$.+Proxy");
Review comment:
It's not clear what the plan is, just leave it?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] johnament commented on a change in pull request #313: [CXF-7493] Add a default ClassUnwrapper for CDI integration.
johnament commented on a change in pull request #313: [CXF-7493] Add a default
ClassUnwrapper for CDI integration.
URL: https://github.com/apache/cxf/pull/313#discussion_r139945342
##
File path: integration/cdi/src/main/java/org/apache/cxf/cdi/CdiBusBean.java
##
@@ -63,6 +65,9 @@ public String getName() {
@Override
public ExtensionManagerBus create(final CreationalContext<
ExtensionManagerBus > ctx) {
final ExtensionManagerBus instance = injectionTarget.produce(ctx);
+if
("true".equals(SystemPropertyAction.getProperty("org.apache.cxf.cdi.useCdiUnwrapper",
"true"))) {
Review comment:
Sure
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] gonzalad commented on issue #20: FEDIZ-212: fix logout when no httpSession present
gonzalad commented on issue #20: FEDIZ-212: fix logout when no httpSession
present
URL: https://github.com/apache/cxf-fediz/pull/20#issuecomment-335427742
Hi Sergey,
The important thing about this PR is more the fact that doing a logout with
a null principal (ie oidc httpSession expired, user already clicked on logout
button on another OIDC Client app, ...) behaves in the same way (from the end
user perspective) as a logout request with a non-null principal :
- it doesn't raise an exception
- and redirects to idpLogoutUri
- and ultimately redirects back to the post_logout_redirect_url configured
for the configured OIDC Client.
> should we move the session invalidation inside the branch with the
non-null principal ?
The call mc.getHttpServletRequest().getSession().invalidate() can be moved
to the
```
if (mc.getSecurityContext().getUserPrincipal() != null) {
```
block or be kept outside.
I've just tested both way, it works both way.
Thanks !
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] gonzalad opened a new pull request #20: FEDIZ-212: fix logout when no httpSession present
gonzalad opened a new pull request #20: FEDIZ-212: fix logout when no httpSession present URL: https://github.com/apache/cxf-fediz/pull/20 When no httpSession is present, LogoutService throws a OAuthServiceException. This can be an issue if logout is called multiples times (i.e. from different OIDC Clients) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] davidkarlsen opened a new pull request #317: CXF-7524: Use property from parentclass
davidkarlsen opened a new pull request #317: CXF-7524: Use property from parentclass URL: https://github.com/apache/cxf/pull/317 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] asfgit closed pull request #317: CXF-7524: Use property from parentclass
asfgit closed pull request #317: CXF-7524: Use property from parentclass URL: https://github.com/apache/cxf/pull/317 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sberyozkin commented on issue #318: CXF-7506: Upgrade guava
sberyozkin commented on issue #318: CXF-7506: Upgrade guava URL: https://github.com/apache/cxf/pull/318#issuecomment-334708318 Hi, why is this update needed, we usually update guava if some of the CXF features indirectly needs it, example, CXF needs to update a newer swagger-core/etc which requires newer guava This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] davidkarlsen commented on issue #318: CXF-7506: Upgrade guava
davidkarlsen commented on issue #318: CXF-7506: Upgrade guava URL: https://github.com/apache/cxf/pull/318#issuecomment-334736797 AFAIK it is not needed - I just found the JIRA issue sticking around for the target release. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sberyozkin commented on issue #318: CXF-7506: Upgrade guava
sberyozkin commented on issue #318: CXF-7506: Upgrade guava URL: https://github.com/apache/cxf/pull/318#issuecomment-334739034 Thanks; yes, IMHO it would make sense to postpone until the newer Guava version is actually needed This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] davidkarlsen closed pull request #318: CXF-7506: Upgrade guava
davidkarlsen closed pull request #318: CXF-7506: Upgrade guava URL: https://github.com/apache/cxf/pull/318 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] davidkarlsen commented on issue #318: CXF-7506: Upgrade guava
davidkarlsen commented on issue #318: CXF-7506: Upgrade guava URL: https://github.com/apache/cxf/pull/318#issuecomment-334740076 Agreed - close the jira and abandon this. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sberyozkin commented on issue #318: CXF-7506: Upgrade guava
sberyozkin commented on issue #318: CXF-7506: Upgrade guava URL: https://github.com/apache/cxf/pull/318#issuecomment-334708318 Hi, why is this update needed, we usually update guava if some of the CXF features indirectly need it, example, CXF needs to update a newer swagger-core/etc which requires newer guava This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ffang closed pull request #319: [CXF-7528] Tone down logging when unregistering CXF servlet in OSGi d?
ffang closed pull request #319: [CXF-7528] Tone down logging when unregistering CXF servlet in OSGi d? URL: https://github.com/apache/cxf/pull/319 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ffang closed pull request #320: [CXF-7529] Clean up JAX-RS RuntimeDelegate when rt-frontend-jaxrs bun?
ffang closed pull request #320: [CXF-7529] Clean up JAX-RS RuntimeDelegate when rt-frontend-jaxrs bun? URL: https://github.com/apache/cxf/pull/320 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] gnodet opened a new pull request #316: [CXF-7519] The cxf-rt-transports-http-jetty should not rely on the ol?
gnodet opened a new pull request #316: [CXF-7519] The cxf-rt-transports-http-jetty should not rely on the ol? URL: https://github.com/apache/cxf/pull/316 ?d blueprint compatibility bundle This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] gnodet commented on issue #316: [CXF-7519] The cxf-rt-transports-http-jetty should not rely on the ol?
gnodet commented on issue #316: [CXF-7519] The cxf-rt-transports-http-jetty should not rely on the ol? URL: https://github.com/apache/cxf/pull/316#issuecomment-332740149 Retest this please. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] gnodet closed pull request #316: [CXF-7519] The cxf-rt-transports-http-jetty should not rely on the ol?
gnodet closed pull request #316: [CXF-7519] The cxf-rt-transports-http-jetty should not rely on the ol? URL: https://github.com/apache/cxf/pull/316 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] sberyozkin commented on issue #20: FEDIZ-212: fix logout when no httpSession present
sberyozkin commented on issue #20: FEDIZ-212: fix logout when no httpSession present URL: https://github.com/apache/cxf-fediz/pull/20#issuecomment-335299741 Hi Adrian - should we move the session invalidation inside the branch with the non-null principal ? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] davidkarlsen opened a new pull request #318: CXF-7506: Upgrade guava
davidkarlsen opened a new pull request #318: CXF-7506: Upgrade guava URL: https://github.com/apache/cxf/pull/318 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] deki commented on issue #318: CXF-7506: Upgrade guava
deki commented on issue #318: CXF-7506: Upgrade guava URL: https://github.com/apache/cxf/pull/318#issuecomment-334579061 Thanks for your PR. Have you checked for OSGi compatibility? we had several conflicts in the past... This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] davidkarlsen commented on issue #318: CXF-7506: Upgrade guava
davidkarlsen commented on issue #318: CXF-7506: Upgrade guava URL: https://github.com/apache/cxf/pull/318#issuecomment-334596524 No - I did not - and the build broke - downloading the internet and running locally now... This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] grgrzybek opened a new pull request #319: [CXF-7528] Tone down logging when unregistering CXF servlet in OSGi d?
grgrzybek opened a new pull request #319: [CXF-7528] Tone down logging when unregistering CXF servlet in OSGi d? URL: https://github.com/apache/cxf/pull/319 ?uring fast reprovisioning This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] grgrzybek opened a new pull request #320: [CXF-7529] Clean up JAX-RS RuntimeDelegate when rt-frontend-jaxrs bun?
grgrzybek opened a new pull request #320: [CXF-7529] Clean up JAX-RS RuntimeDelegate when rt-frontend-jaxrs bun? URL: https://github.com/apache/cxf/pull/320 ?dle is stopped/refreshed This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
cxf pull request: httpj:engine-factory
GitHub user seijoed opened the pull request at https://github.com/apache/cxf/pull/1 httpj:engine-factory You can merge this pull request into a Git repository by running $ git pull https://github.com/seijoed/cxf trunk Alternatively you can review and apply these changes as the patch at https://github.com/apache/cxf/pull/1.patch
cxf pull request: httpj:engine-factory
Github user seijoed closed the pull request at https://github.com/apache/cxf/pull/1
