pareshddevalia commented on code in PR #622: URL: https://github.com/apache/atlas/pull/622#discussion_r3403997065
########## dev-support/atlas-docker/scripts/atlas-kafka-application.properties: ########## @@ -21,3 +21,11 @@ atlas.rest.address=http://atlas.example.com:21000 atlas.kafka.zookeeper.connect=atlas-zk.example.com:2181 atlas.kafka.bootstrap.servers=atlas-kafka.example.com:9092 + + +####### Atlas REST Notification ########### + +atlas.hook.rest.notification.address=http://atlas-rest.example.com:41000/rest Review Comment: same as here https://github.com/apache/atlas/pull/622/changes#r3403994818 ########## server-common/src/main/java/org/apache/atlas/server/common/filters/AtlasDelegatingAuthenticationEntryPoint.java: ########## @@ -25,34 +25,38 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; - import java.io.IOException; import java.util.LinkedHashMap; public class AtlasDelegatingAuthenticationEntryPoint extends DelegatingAuthenticationEntryPoint { + + public static final String SESSION_TIMEOUT = "Session Timeout"; private static final Logger LOG = LoggerFactory.getLogger(AtlasDelegatingAuthenticationEntryPoint.class); - public static final String SESSION_TIMEOUT = "Session Timeout"; public AtlasDelegatingAuthenticationEntryPoint(LinkedHashMap<RequestMatcher, AuthenticationEntryPoint> entryPoints) { super(entryPoints); - - LOG.debug("AtlasDelegatingAuthenticationEntryPoint-AjaxAwareAuthenticationEntryPoint(): constructor"); + if (LOG.isDebugEnabled()) { + LOG.info("AtlasDelegatingAuthenticationEntryPoint-AjaxAwareAuthenticationEntryPoint(): constructor"); + } } - public void commence(HttpServletRequest request, HttpServletResponse response, AuthenticationException authException) throws IOException { - String ajaxRequestHeader = request.getHeader(HeadersUtil.X_REQUESTED_WITH_KEY); + public void commence(HttpServletRequest request, HttpServletResponse response, Review Comment: Please keep the existing code as it is ########## server-common/src/main/java/org/apache/atlas/server/common/filters/AtlasDelegatingAuthenticationEntryPoint.java: ########## @@ -25,34 +25,38 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; - import java.io.IOException; import java.util.LinkedHashMap; public class AtlasDelegatingAuthenticationEntryPoint extends DelegatingAuthenticationEntryPoint { + + public static final String SESSION_TIMEOUT = "Session Timeout"; private static final Logger LOG = LoggerFactory.getLogger(AtlasDelegatingAuthenticationEntryPoint.class); - public static final String SESSION_TIMEOUT = "Session Timeout"; public AtlasDelegatingAuthenticationEntryPoint(LinkedHashMap<RequestMatcher, AuthenticationEntryPoint> entryPoints) { super(entryPoints); - - LOG.debug("AtlasDelegatingAuthenticationEntryPoint-AjaxAwareAuthenticationEntryPoint(): constructor"); + if (LOG.isDebugEnabled()) { Review Comment: Please remove the debug condition ########## server-common/src/main/java/org/apache/atlas/server/common/security/AtlasAuthenticationSuccessHandler.java: ########## @@ -27,41 +27,41 @@ import org.springframework.stereotype.Component; import javax.annotation.PostConstruct; +import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; - import java.io.IOException; + @Component public class AtlasAuthenticationSuccessHandler implements AuthenticationSuccessHandler { - private static final Logger LOG = LoggerFactory.getLogger(AtlasAuthenticationSuccessHandler.class); - - public static final String LOCALLOGIN = "locallogin"; + private static Logger LOG = LoggerFactory.getLogger(AuthenticationSuccessHandler.class); private int sessionTimeout = 3600; + public static final String LOCALLOGIN = "locallogin"; Review Comment: Keep existing code as it is. ########## server-common/src/main/java/org/apache/atlas/server/common/service/CuratorFactory.java: ########## @@ -63,19 +62,18 @@ public class CuratorFactory { private final Configuration configuration; private CuratorFramework curatorFramework; + private final HighAvailabilitySupport haSupport; /** * Initializes the {@link CuratorFramework} that is used for all interaction with Zookeeper. * @throws AtlasException */ - public CuratorFactory() throws AtlasException { - this(ApplicationProperties.get()); - } - - public CuratorFactory(Configuration configuration) { + @Inject + public CuratorFactory(Configuration configuration, HighAvailabilitySupport haSupport) { Review Comment: check the reference name ########## server-common/src/main/java/org/apache/atlas/server/common/service/ServiceState.java: ########## @@ -47,26 +39,29 @@ */ @Singleton @Component -public class ServiceState { +public class ServiceState implements ServiceStateProvider { private static final Logger LOG = LoggerFactory.getLogger(ServiceState.class); - @Autowired - AtlasAuditService auditService; - - private final Configuration configuration; + public enum ServiceStateValue { + ACTIVE, + PASSIVE, + BECOMING_ACTIVE, + BECOMING_PASSIVE, + MIGRATING + } + private Configuration configuration; private volatile ServiceStateValue state; + private final HighAvailabilitySupport haSupport; - public ServiceState() throws AtlasException { - this(ApplicationProperties.get()); - } - - public ServiceState(Configuration configuration) { + @Inject + public ServiceState(Configuration configuration, HighAvailabilitySupport haSupport) { this.configuration = configuration; + this.haSupport = haSupport; - state = !HAConfiguration.isHAEnabled(configuration) ? ServiceStateValue.ACTIVE : ServiceStateValue.PASSIVE; + state = !haSupport.isHAEnabled(configuration) ? ServiceStateValue.ACTIVE : ServiceStateValue.PASSIVE; Review Comment: check the name in internal branch highAvailability ########## server-common/src/main/java/org/apache/atlas/server/common/filters/AtlasHeaderFilter.java: ########## @@ -24,27 +27,28 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletResponse; - import java.io.IOException; public class AtlasHeaderFilter implements Filter { + private static final Logger LOG = LoggerFactory.getLogger(AtlasHeaderFilter.class); Review Comment: Remove the LOG if it is not used. ########## rest-notification-webapp/src/main/java/org/apache/atlas/notification/rest/ha/RestNotificationHighAvailabilitySupport.java: ########## @@ -0,0 +1,61 @@ +/** + * 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 + * <p> Review Comment: Class should be RestNotificationHighAvailabilityImpl please check the internal branch class name ########## dev-support/atlas-docker/scripts/atlas-hive-application.properties: ########## @@ -21,3 +21,11 @@ atlas.rest.address=http://atlas.example.com:21000 atlas.kafka.zookeeper.connect=atlas-zk.example.com:2181 atlas.kafka.bootstrap.servers=atlas-kafka.example.com:9092 + + +####### Atlas REST Notification ########### + +atlas.hook.rest.notification.address=http://atlas-rest.example.com:41000/rest Review Comment: Please comment the rest-notification properties. ########## rest-notification-webapp/src/main/java/org/apache/atlas/notification/rest/web/services/AtlasSecurityStateProviderConfig.java: ########## @@ -0,0 +1,58 @@ +/** + * 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.atlas.notification.rest.web.services; + +import org.apache.atlas.server.common.service.ActiveInstanceState; +import org.apache.atlas.server.common.service.ServiceState; +import org.apache.atlas.server.common.filters.spi.ActiveInstanceStateProvider; +import org.apache.atlas.server.common.filters.spi.ServiceStateProvider; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +public class AtlasSecurityStateProviderConfig { Review Comment: Check the internal branch class name AtlasServiceStateProviderConfig ########## server-common/src/main/java/org/apache/atlas/server/common/security/AtlasAuthenticationSuccessHandler.java: ########## @@ -27,41 +27,41 @@ import org.springframework.stereotype.Component; import javax.annotation.PostConstruct; +import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; - import java.io.IOException; + Review Comment: Remove this space line. ########## rest-notification-webapp/pom.xml: ########## @@ -0,0 +1,231 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ 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. + --> +<project xmlns="http://maven.apache.org/POM/4.0.0"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd";> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>org.apache.atlas</groupId> + <artifactId>apache-atlas</artifactId> + <version>3.0.0-SNAPSHOT</version> + </parent> + <groupId>org.apache.atlas</groupId> + <artifactId>rest-notification-webapp</artifactId> + <packaging>war</packaging> + + <name>Rest Notification Webapp</name> + + <dependencies> + <!-- SLF4J binding: use logback to avoid NOP logger warnings --> + <dependency> + <groupId>org.slf4j</groupId> + <artifactId>log4j-over-slf4j</artifactId> + <version>${slf4j.version}</version> + </dependency> + <dependency> + <groupId>ch.qos.logback</groupId> + <artifactId>logback-classic</artifactId> + </dependency> + <dependency> + <groupId>ch.qos.logback</groupId> + <artifactId>logback-core</artifactId> + </dependency> + <dependency> + <groupId>junit</groupId> + <artifactId>junit</artifactId> + <version>${junit.version}</version> + <scope>test</scope> + </dependency> + <dependency> + <groupId>org.eclipse.jetty</groupId> + <artifactId>jetty-server</artifactId> + </dependency> + <dependency> + <groupId>org.eclipse.jetty</groupId> + <artifactId>jetty-webapp</artifactId> + </dependency> + <dependency> + <groupId>org.springframework</groupId> + <artifactId>spring-web</artifactId> + </dependency> + <dependency> + <groupId>org.springframework</groupId> + <artifactId>spring-aop</artifactId> + </dependency> + <dependency> + <groupId>org.aspectj</groupId> + <artifactId>aspectjrt</artifactId> + <version>1.8.9</version> + </dependency> + <dependency> + <groupId>org.aspectj</groupId> + <artifactId>aspectjweaver</artifactId> + <version>1.8.9</version> + </dependency> + <dependency> + <groupId>org.apache.atlas</groupId> + <artifactId>atlas-intg</artifactId> + </dependency> + + <dependency> + <groupId>org.apache.atlas</groupId> + <artifactId>atlas-server-common</artifactId> + </dependency> + <dependency> + <groupId>com.google.guava</groupId> + <artifactId>guava</artifactId> + </dependency> + <dependency> + <groupId>org.apache.atlas</groupId> + <artifactId>atlas-notification</artifactId> + </dependency> + <dependency> + <groupId>org.apache.atlas</groupId> + <artifactId>atlas-client-v2</artifactId> + </dependency> + <dependency> + <groupId>org.apache.atlas</groupId> + <artifactId>atlas-authorization</artifactId> + <version>${project.version}</version> + </dependency> + <dependency> + <groupId>org.springframework.security</groupId> + <artifactId>spring-security-config</artifactId> + </dependency> + <dependency> + <groupId>org.springframework.security</groupId> + <artifactId>spring-security-ldap</artifactId> + <exclusions> + <exclusion> + <groupId>org.springframework.ldap</groupId> + <artifactId>spring-ldap-core</artifactId> + </exclusion> + </exclusions> + </dependency> + <dependency> + <groupId>org.springframework.ldap</groupId> + <artifactId>spring-ldap-core</artifactId> + <version>${spring-ldap-core.version}</version> + </dependency> + <dependency> + <groupId>org.springframework.security</groupId> + <artifactId>spring-security-web</artifactId> + </dependency> + + <dependency> + <groupId>org.apache.commons</groupId> + <artifactId>commons-configuration2</artifactId> + <version>${commons-conf2.version}</version> + <exclusions> + <exclusion> + <groupId>org.apache.commons</groupId> + <artifactId>commons-text</artifactId> + </exclusion> + </exclusions> + </dependency> + + <dependency> + <groupId>org.apache.commons</groupId> + <artifactId>commons-text</artifactId> + <version>${commons-text.version}</version> + </dependency> + + <dependency> + <groupId>commons-lang</groupId> + <artifactId>commons-lang</artifactId> + <version>2.6</version> + </dependency> + + <dependency> + <groupId>com.googlecode.json-simple</groupId> + <artifactId>json-simple</artifactId> + </dependency> + + <!-- PAM --> + <dependency> + <groupId>org.kohsuke</groupId> + <artifactId>libpam4j</artifactId> + <version>1.11</version> + </dependency> + + <!-- Jersey 1.x (com.sun.jersey), aligned with atlas-webapp — replaces org.glassfish.jersey (Jersey 2) from cdh_main --> + <dependency> + <groupId>com.sun.jersey</groupId> + <artifactId>jersey-client</artifactId> + </dependency> + <dependency> + <groupId>com.sun.jersey</groupId> + <artifactId>jersey-core</artifactId> + </dependency> + + <dependency> + <groupId>com.sun.jersey</groupId> + <artifactId>jersey-server</artifactId> + </dependency> + + <dependency> + <groupId>com.sun.jersey</groupId> + <artifactId>jersey-servlet</artifactId> + </dependency> + + <dependency> + <groupId>com.sun.jersey.contribs</groupId> + <artifactId>jersey-multipart</artifactId> + </dependency> + + <dependency> + <groupId>com.sun.jersey.contribs</groupId> + <artifactId>jersey-spring</artifactId> + </dependency> + + <dependency> + <groupId>com.sun.jersey</groupId> + <artifactId>jersey-json</artifactId> + <version>${jersey.version}</version> + </dependency> + + <dependency> + <groupId>javax.servlet</groupId> + <artifactId>javax.servlet-api</artifactId> + </dependency> + </dependencies> + + <build> + <plugins> Review Comment: Please check the build plugins with the internal branch rest-notification pom.xml file. ########## server-common/src/main/java/org/apache/atlas/server/common/service/ActiveInstanceElectorService.java: ########## @@ -74,15 +74,21 @@ public class ActiveInstanceElectorService implements Service, LeaderLatchListene * @throws AtlasException */ @Inject - ActiveInstanceElectorService(Configuration configuration, Set<ActiveStateChangeHandler> activeStateChangeHandlerProviders, - CuratorFactory curatorFactory, ActiveInstanceState activeInstanceState, ServiceState serviceState, AtlasMetricsUtil metricsUtil) { + public ActiveInstanceElectorService(Configuration configuration, + Set<ActiveStateChangeHandler> activeStateChangeHandlerProviders, + List<ServiceMetricsHook> metricsHooks, + CuratorFactory curatorFactory, + ActiveInstanceState activeInstanceState, + ServiceState serviceState, + HighAvailabilitySupport haSupport) { this.configuration = configuration; this.activeStateChangeHandlerProviders = activeStateChangeHandlerProviders; + this.metricsHooks = metricsHooks != null ? metricsHooks : Collections.emptyList(); this.activeStateChangeHandlers = new ArrayList<>(); this.curatorFactory = curatorFactory; this.activeInstanceState = activeInstanceState; this.serviceState = serviceState; - this.metricsUtil = metricsUtil; + this.haSupport = haSupport; Review Comment: Check the name in internal branch this.highAvailability ########## server-common/src/main/java/org/apache/atlas/server/common/filters/AtlasHeaderFilter.java: ########## @@ -24,27 +27,28 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletResponse; - import java.io.IOException; public class AtlasHeaderFilter implements Filter { + private static final Logger LOG = LoggerFactory.getLogger(AtlasHeaderFilter.class); + @Override public void init(FilterConfig filterConfig) { } @Override - public void doFilter(ServletRequest request, ServletResponse response, FilterChain filterChain) throws IOException, ServletException { + public void doFilter(ServletRequest request, ServletResponse response, FilterChain filterChain) Review Comment: If its irrelevent to rest-notification changes please keep code as same as existing. ########## server-common/src/main/java/org/apache/atlas/server/common/filters/RestUtil.java: ########## @@ -71,26 +71,26 @@ public static String constructForwardableURL(HttpServletRequest httpRequest) { } if (xForwardedHost.contains(",")) { - LOG.debug("xForwardedHost value is {} it contains multiple hosts, selecting the first host.", xForwardedHost); + LOG.debug("xForwardedHost value is " + xForwardedHost + " it contains multiple hosts, selecting the first host."); xForwardedHost = xForwardedHost.split(",")[0].trim(); } String xForwardedURL = ""; - if (StringUtils.trimToNull(xForwardedProto) != null) { + if (org.apache.commons.lang3.StringUtils.trimToNull(xForwardedProto) != null) { Review Comment: Why this package is required org.apache.commons.lang3. We already using StringUtils utilities ########## server-common/src/main/java/org/apache/atlas/server/common/service/ActiveInstanceElectorService.java: ########## @@ -93,19 +99,24 @@ public class ActiveInstanceElectorService implements Service, LeaderLatchListene */ @Override public void start() throws AtlasException { - metricsUtil.onServerStart(); + boolean haEnabled = haSupport.isHAEnabled(configuration); - if (!HAConfiguration.isHAEnabled(configuration)) { - metricsUtil.onServerActivation(); + for (ServiceMetricsHook hook : metricsHooks) { + hook.onServerStart(); + if (!haEnabled) { + hook.onServerActivation(); + } + } + if (!haEnabled) { LOG.info("HA is not enabled, no need to start leader election service"); return; } cacheActiveStateChangeHandlers(); - serverId = AtlasServerIdSelector.selectServerId(configuration); + serverId = haSupport.selectServerId(configuration); Review Comment: check this also ########## server-common/src/main/java/org/apache/atlas/server/common/filters/AtlasDelegatingAuthenticationEntryPoint.java: ########## @@ -25,34 +25,38 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; - import java.io.IOException; import java.util.LinkedHashMap; public class AtlasDelegatingAuthenticationEntryPoint extends DelegatingAuthenticationEntryPoint { + + public static final String SESSION_TIMEOUT = "Session Timeout"; private static final Logger LOG = LoggerFactory.getLogger(AtlasDelegatingAuthenticationEntryPoint.class); - public static final String SESSION_TIMEOUT = "Session Timeout"; public AtlasDelegatingAuthenticationEntryPoint(LinkedHashMap<RequestMatcher, AuthenticationEntryPoint> entryPoints) { super(entryPoints); - - LOG.debug("AtlasDelegatingAuthenticationEntryPoint-AjaxAwareAuthenticationEntryPoint(): constructor"); + if (LOG.isDebugEnabled()) { + LOG.info("AtlasDelegatingAuthenticationEntryPoint-AjaxAwareAuthenticationEntryPoint(): constructor"); + } } - public void commence(HttpServletRequest request, HttpServletResponse response, AuthenticationException authException) throws IOException { - String ajaxRequestHeader = request.getHeader(HeadersUtil.X_REQUESTED_WITH_KEY); + public void commence(HttpServletRequest request, HttpServletResponse response, + AuthenticationException authException) throws IOException { + String ajaxRequestHeader = request.getHeader(HeadersUtil.X_REQUESTED_WITH_KEY); response.setHeader(HeadersUtil.X_FRAME_OPTIONS_KEY, HeadersUtil.X_FRAME_OPTIONS_VAL); - if (HeadersUtil.X_REQUESTED_WITH_VALUE.equalsIgnoreCase(ajaxRequestHeader)) { + if (ajaxRequestHeader != null + && HeadersUtil.X_REQUESTED_WITH_VALUE.equalsIgnoreCase(ajaxRequestHeader)) { if (LOG.isDebugEnabled()) { - LOG.debug("commence() AJAX request. Authentication required. Returning {}. URL={}", HttpServletResponse.SC_UNAUTHORIZED, request.getRequestURI()); + LOG.debug("commence() AJAX request. Authentication required. Returning " + + HttpServletResponse.SC_UNAUTHORIZED + ". URL=" + request.getRequestURI()); } - response.sendError(HeadersUtil.SC_AUTHENTICATION_TIMEOUT, SESSION_TIMEOUT); } else { - response.sendError(HttpServletResponse.SC_UNAUTHORIZED, authException.getMessage()); + response.sendError(HttpServletResponse.SC_UNAUTHORIZED, + authException.getMessage()); Review Comment: Keep as it is. ########## server-common/src/main/java/org/apache/atlas/server/common/security/AtlasAuthenticationSuccessHandler.java: ########## @@ -27,41 +27,41 @@ import org.springframework.stereotype.Component; import javax.annotation.PostConstruct; +import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; - import java.io.IOException; + @Component public class AtlasAuthenticationSuccessHandler implements AuthenticationSuccessHandler { - private static final Logger LOG = LoggerFactory.getLogger(AtlasAuthenticationSuccessHandler.class); - - public static final String LOCALLOGIN = "locallogin"; + private static Logger LOG = LoggerFactory.getLogger(AuthenticationSuccessHandler.class); private int sessionTimeout = 3600; + public static final String LOCALLOGIN = "locallogin"; @PostConstruct public void setup() { sessionTimeout = AtlasConfiguration.SESSION_TIMEOUT_SECS.getInt(); } @Override - public void onAuthenticationSuccess(HttpServletRequest request, HttpServletResponse response, Authentication authentication) throws IOException { - LOG.debug("Login Success {}", authentication.getPrincipal()); + public void onAuthenticationSuccess(HttpServletRequest request, HttpServletResponse response, + Authentication authentication) throws IOException, ServletException { Review Comment: Keep exisiting code as it is. ########## server-common/src/main/java/org/apache/atlas/server/common/service/ActiveInstanceElectorService.java: ########## @@ -182,7 +195,7 @@ public void notLeader() { private void joinElection() { LOG.info("Starting leader election for {}", serverId); - String zkRoot = HAConfiguration.getZookeeperProperties(configuration).getZkRoot(); + String zkRoot = haSupport.getZookeeperProperties(configuration).getZkRoot(); Review Comment: Same as here also. ########## server-common/src/main/java/org/apache/atlas/server/common/service/ServiceMetricsHook.java: ########## @@ -0,0 +1,30 @@ +/** + * 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 + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * 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.atlas.server.common.service; + +/** + * Reacts to server lifecycle events; WARs may register beans (e.g. delegating to {@code AtlasMetricsUtil}). + */ +public interface ServiceMetricsHook { Review Comment: Please check the internal code interface name is different and it has comment also. ########## server-common/src/main/java/org/apache/atlas/server/common/service/ActiveInstanceState.java: ########## @@ -50,32 +48,25 @@ * provide for safety across multiple processes. */ @Component -public class ActiveInstanceState { +public class ActiveInstanceState implements ActiveInstanceStateProvider { private static final Logger LOG = LoggerFactory.getLogger(ActiveInstanceState.class); public static final String APACHE_ATLAS_ACTIVE_SERVER_INFO = "/active_server_info"; private final Configuration configuration; private final CuratorFactory curatorFactory; + private final HighAvailabilitySupport haSupport; /** * Create a new instance of {@link ActiveInstanceState}. * @param curatorFactory an instance of {@link CuratorFactory} to get the {@link InterProcessReadWriteLock} * @throws AtlasException */ @Inject - public ActiveInstanceState(CuratorFactory curatorFactory) throws AtlasException { - this(ApplicationProperties.get(), curatorFactory); - } - - /** - * Create a new instance of {@link ActiveInstanceState}. - * @param configuration an instance of {@link Configuration} created from Atlas configuration - * @param curatorFactory an instance of {@link CuratorFactory} to get the {@link InterProcessReadWriteLock} - */ - public ActiveInstanceState(Configuration configuration, CuratorFactory curatorFactory) { + public ActiveInstanceState(Configuration configuration, CuratorFactory curatorFactory, HighAvailabilitySupport haSupport) { this.configuration = configuration; - this.curatorFactory = curatorFactory; + this.curatorFactory = curatorFactory; + this.haSupport = haSupport; Review Comment: Please check the latest internal code ########## server-common/src/main/java/org/apache/atlas/server/common/service/ActiveInstanceElectorService.java: ########## @@ -93,19 +99,24 @@ public class ActiveInstanceElectorService implements Service, LeaderLatchListene */ @Override public void start() throws AtlasException { - metricsUtil.onServerStart(); + boolean haEnabled = haSupport.isHAEnabled(configuration); - if (!HAConfiguration.isHAEnabled(configuration)) { - metricsUtil.onServerActivation(); + for (ServiceMetricsHook hook : metricsHooks) { Review Comment: Please check the method code is different from internal branch changes. ########## server-common/src/main/java/org/apache/atlas/server/common/service/ActiveInstanceState.java: ########## @@ -50,32 +48,25 @@ * provide for safety across multiple processes. */ @Component -public class ActiveInstanceState { +public class ActiveInstanceState implements ActiveInstanceStateProvider { private static final Logger LOG = LoggerFactory.getLogger(ActiveInstanceState.class); public static final String APACHE_ATLAS_ACTIVE_SERVER_INFO = "/active_server_info"; private final Configuration configuration; private final CuratorFactory curatorFactory; + private final HighAvailabilitySupport haSupport; Review Comment: It should be highAvailability reference name ########## server-common/src/main/java/org/apache/atlas/server/common/service/ActiveInstanceState.java: ########## @@ -87,12 +78,16 @@ public ActiveInstanceState(Configuration configuration, CuratorFactory curatorFa * @param serverId ID of this server instance */ public void update(String serverId) throws AtlasBaseException { Review Comment: Please check this class code in internal branch, is different. also keep existing code same. ########## server-common/src/main/java/org/apache/atlas/server/common/service/EmbeddedServer.java: ########## @@ -15,16 +15,11 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -package org.apache.atlas.web.service; +package org.apache.atlas.server.common.service; Review Comment: Please check this class code with internal branch ########## webapp/src/main/java/org/apache/atlas/web/ha/WebappHighAvailabilitySupport.java: ########## @@ -0,0 +1,61 @@ +/** + * 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 + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * 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.atlas.web.ha; + +import org.apache.atlas.AtlasException; +import org.apache.atlas.ha.AtlasServerIdSelector; +import org.apache.atlas.ha.HAConfiguration; +import org.apache.atlas.server.common.service.HighAvailabilityProperties; +import org.apache.atlas.server.common.service.HighAvailabilitySupport; +import org.apache.commons.configuration.Configuration; +import org.springframework.stereotype.Component; + +/** + * Webapp implementation of {@link HighAvailabilitySupport} using {@link HAConfiguration}. + */ +@Component +public class WebappHighAvailabilitySupport implements HighAvailabilitySupport { Review Comment: Change the name HighAvailabilityImpl, please check the internal code ########## webapp/src/main/java/org/apache/atlas/web/metrics/WebappServiceMetricsHook.java: ########## @@ -0,0 +1,47 @@ +/** + * 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 + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * 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.atlas.web.metrics; + +import org.apache.atlas.server.common.service.ServiceMetricsHook; +import org.apache.atlas.util.AtlasMetricsUtil; +import org.springframework.stereotype.Component; + +import javax.inject.Inject; + +/** + * Bridges {@link ServiceMetricsHook} to {@link AtlasMetricsUtil} in the main webapp. + */ +@Component +public class WebappServiceMetricsHook implements ServiceMetricsHook { Review Comment: check the internal code ServiceStateChangeMetricHandler ########## webapp/src/main/java/org/apache/atlas/web/service/AtlasSecurityStateProviderConfig.java: ########## @@ -0,0 +1,58 @@ +/** + * 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 + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * 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.atlas.web.service; + +import org.apache.atlas.server.common.filters.spi.ActiveInstanceStateProvider; +import org.apache.atlas.server.common.filters.spi.ServiceStateProvider; +import org.apache.atlas.server.common.service.ActiveInstanceState; +import org.apache.atlas.server.common.service.ServiceState; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +public class AtlasSecurityStateProviderConfig { Review Comment: Class is different in internal branch AtlasServiceStateProviderConfig ########## webapp/src/main/java/org/apache/atlas/web/metrics/WebappAdminAuditHook.java: ########## @@ -0,0 +1,112 @@ +/** + * 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 + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * 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.atlas.web.metrics; + +import org.apache.atlas.exception.AtlasBaseException; +import org.apache.atlas.model.audit.AtlasAuditEntry.AuditOperation; +import org.apache.atlas.repository.audit.AtlasAuditService; +import org.apache.atlas.server.common.service.EmbeddedServer; +import org.apache.atlas.server.common.service.ServiceMetricsHook; +import org.apache.commons.lang3.StringUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Component; + +import javax.inject.Inject; + +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.util.Date; + +/** + * Persists {@code SERVER_START} and {@code SERVER_STATE_ACTIVE} rows for Administration → Audits. + * <p> + * Registered alongside {@link WebappServiceMetricsHook} as a {@link ServiceMetricsHook} so lifecycle + * matches {@link org.apache.atlas.server.common.service.ActiveInstanceElectorService}: non-HA invokes + * both hooks in sequence; HA emits {@code SERVER_START} at process init and {@code SERVER_STATE_ACTIVE} + * when this instance becomes leader. + * <p> + * Uses the eight-argument {@link AtlasAuditService#add} overload so user and client id are explicit + * and do not rely on {@link org.apache.atlas.RequestContext} during bootstrap. + */ +@Component +public class WebappAdminAuditHook implements ServiceMetricsHook { Review Comment: Please check the internal branch class name ServiceStateChangeAuditHandler and it code ########## webapp/src/test/java/org/apache/atlas/web/metrics/WebappAdminAuditHookTest.java: ########## @@ -0,0 +1,118 @@ +/** + * 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 + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * 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.atlas.web.metrics; + +import org.apache.atlas.exception.AtlasBaseException; +import org.apache.atlas.model.audit.AtlasAuditEntry.AuditOperation; +import org.apache.atlas.repository.audit.AtlasAuditService; +import org.apache.atlas.server.common.service.EmbeddedServer; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import java.util.Date; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; + +public class WebappAdminAuditHookTest { + private static final String ATLAS_USER = "atlas"; + + @Mock + private AtlasAuditService auditService; + + private WebappAdminAuditHook hook; + + @BeforeMethod + public void setup() { + MockitoAnnotations.initMocks(this); + hook = new WebappAdminAuditHook(auditService); + } + + @Test Review Comment: Check this test will fail. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
