You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@metron.apache.org by sardell <gi...@git.apache.org> on 2018/07/19 17:53:52 UTC
[GitHub] metron pull request #1122: METRON-1683: Fix the download progress bar in PCA...
GitHub user sardell opened a pull request:
https://github.com/apache/metron/pull/1122
METRON-1683: Fix the download progress bar in PCAP UI
## Contributor Comments
The download progress bar in the current PCAP UI work does not appear when a PCAP search is submitted.
To test, submit a PCAP search. As progress is made, you'll see the blue background of the progress bar animate from left to right.
## Pull Request Checklist
Thank you for submitting a contribution to Apache Metron.
Please refer to our [Development Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235) for the complete guide to follow for contributions.
Please refer also to our [Build Verification Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview) for complete smoke testing guides.
In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
- [ ] Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically master)?
### For code changes:
- [ ] Have you included steps to reproduce the behavior or problem that is being changed or addressed?
- [ ] Have you included steps or a guide to how the change may be verified and tested manually?
- [ ] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:
```
mvn -q clean integration-test install && dev-utilities/build-utils/verify_licenses.sh
```
- [ ] Have you written or updated unit tests and or integration tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via `site-book/target/site/index.html`:
```
cd site-book
mvn site
```
#### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
It is also recommended that [travis-ci](https://travis-ci.org) is set up for your personal repository such that your branches are built there before submitting a pull request.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sardell/metron METRON-1683
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metron/pull/1122.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1122
----
commit 872d1b1ee13e358c18956945d71d3667d19fca8a
Author: merrimanr <me...@...>
Date: 2018-04-12T14:57:48Z
Merge branch 'pcap-front' of https://github.com/simonellistonball/metron into pcaprest
Conflicts:
metron-interface/metron-alerts/src/app/app.module.ts
commit b1b6a7dabea1a1d0d132482c8d97af29c0ac2683
Author: merrimanr <me...@...>
Date: 2018-04-13T15:00:15Z
initial commit
Conflicts:
metron-interface/metron-rest/src/main/java/org/apache/metron/rest/MetronRestApplication.java
metron-interface/metron-rest/src/main/java/org/apache/metron/rest/controller/PcapQueryController.java
metron-interface/metron-rest/src/main/java/org/apache/metron/rest/util/pcapQueryThread.java
commit 55cf2d945a4fcff1e7e2e47a234037ed6f394b2e
Author: merrimanr <me...@...>
Date: 2018-04-18T15:52:56Z
added license headers
commit 70696d047c6ef4b8ce5fcda03588474ff5b2c506
Author: tiborm <ti...@...>
Date: 2018-07-11T14:58:56Z
METRON-1662: adding download button to pcap tab
commit 4a83d46259736d8c721fb9f640b6ad522f91a559
Author: tiborm <ti...@...>
Date: 2018-07-12T07:09:14Z
METRON-1662: removing unused function from the first iteration
commit 9f57724597240a05d1caeec73882a4ed50aadd9a
Author: tiborm <ti...@...>
Date: 2018-07-12T08:36:15Z
METRON-1662: changing download button label from pdml to pcap
commit 49385eeab612d4f02882888f45defe5cca61f0b9
Author: tiborm <ti...@...>
Date: 2018-07-16T15:00:55Z
METRON-1662: cleaning pdml.ts
commit 32caf44f81cf073e60518e7acccd4a30028f0dd2
Author: tiborm <ti...@...>
Date: 2018-07-16T15:01:47Z
METRON-1662: making unit tests running
commit 38f20f40e213f9785ddbff429453eef541881a8a
Author: tiborm <ti...@...>
Date: 2018-07-17T07:26:38Z
METRON-1662 Cleaning packet component
commit 7983d1e50c5016e904d65c722b3cd081f89e4d26
Author: tiborm <ti...@...>
Date: 2018-07-17T07:27:27Z
METRON-1662 Aligning pcap service and panel to the current status api
commit 28fca20eae9e72c5e4b398dfada3dc920916824a
Author: tiborm <ti...@...>
Date: 2018-07-17T09:06:06Z
METRON-1662 Moving download button below the gird
commit 1d58cc9f112c3c11607e4bdcb755bfa38d4c201f
Author: tiborm <ti...@...>
Date: 2018-07-17T09:06:35Z
METRON-1662 Making progress bar code cleaner
commit e062a63c686854a0301b6e252f14e4ddcdbaf53d
Author: tiborm <ti...@...>
Date: 2018-07-17T09:36:34Z
METRON-1662 Cleaning pcap.service
commit 7a87a0ae9778538e479a1626abde1ecaf81daa2e
Author: tiborm <ti...@...>
Date: 2018-07-17T09:38:32Z
METRON-1662: adding process percentage to PcapStatusResponse
commit 68d9f7aeb7618f76d648165f52b2f9b43946eb5d
Author: tiborm <ti...@...>
Date: 2018-07-17T09:43:37Z
METRON-1662 Aligning download url to the API, adding selectedPage field
commit 3847cb4d0750d789bd3adace746656a7f9a60231
Author: tiborm <ti...@...>
Date: 2018-07-17T16:07:45Z
Adding date range selector to the filter bar
commit c25fd634fa690e02389056d2831518a7e05dccd8
Author: tiborm <ti...@...>
Date: 2018-07-16T15:01:47Z
METRON-1662: making unit tests running
commit b5fe431e14dbffc65ac421162f478776969438ae
Author: tiborm <ti...@...>
Date: 2018-07-18T09:43:30Z
removing validator, adding default value for date fields
commit f0111804e506abc287b99d1b9fedce3c0a82cec6
Author: tiborm <ti...@...>
Date: 2018-07-18T09:48:25Z
adding missing dependency to test
commit 3b645698a42efce819049a41e259ccfde9d1bba0
Author: tiborm <ti...@...>
Date: 2018-07-19T14:33:56Z
aligning pcap request to the current state of the REST API
commit ff294179ae367eec7766d8df4f2bd6d6ede0f16d
Author: tiborm <ti...@...>
Date: 2018-07-19T14:34:26Z
covering pcap filter functionality with unit tests
commit 7326344863201a49d50decfd35a66d1e91dd9c04
Author: tiborm <ti...@...>
Date: 2018-07-19T14:37:47Z
fixing TypeScript compilation error
commit 5720f87ae73385783d9c64a689404516ecbd8a8e
Author: tiborm <ti...@...>
Date: 2018-07-19T14:41:23Z
fixing broken protocol field
commit 4fb13bfbc9e7bbcf3198aff56a511fc7d184e7f7
Author: tiborm <ti...@...>
Date: 2018-07-19T14:58:19Z
Merge branch 'feature/METRON-1554-pcap-query-panel' into METRON-1662
commit 1624cc01e94d65687b028fc45eab7565742fc8ee
Author: tiborm <ti...@...>
Date: 2018-07-19T14:59:28Z
Merge branch 'METRON-1662' into METRON-1676
commit a359b6b5947e8098bcfbfae87f400521b17c81e6
Author: tiborm <ti...@...>
Date: 2018-07-19T15:57:08Z
aligning api to the current available version of the REST API
commit f1cd9801429a9aa8bb48e81e9443adf246b7f5e4
Author: Shane Ardell <sa...@...>
Date: 2018-07-19T11:10:09Z
fix download progress bar
----
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by sardell <gi...@git.apache.org>.
Github user sardell commented on the issue:
https://github.com/apache/metron/pull/1122
@mmiklavc I've updated my PR description to include a testing section with steps to reproduce. Please let me know if I can clarify any further.
@justinleet An empty results message is now included with the other error message improvements I merged in thanks to @merrimanr's [contribution](https://github.com/apache/metron/pull/1122/commits/779cbed8b23d82226f3690594109e2720f644f12)
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by sardell <gi...@git.apache.org>.
Github user sardell commented on the issue:
https://github.com/apache/metron/pull/1122
@merrimanr All tests should run fine now. As @ruffle1986 pointed out, I accidentally left a focused describe method in my tests, which causes Jasmine to only execute focused describe blocks. I have since removed it.
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/1122
The unit tests look good to me. I am +1 contingent on the issues that @justinleet found are fixed with the recent commits.
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/1122
Couple quick comments on this after spinning it up.
1. The progress bar doesn't show up until the job submission returns. However, because this is a MapReduce job, it can take a few seconds to spin up and get the call to return. Can we get a 0% progress bar until that call returns (then do whatever with progress / error / etc.)?
2. Seems like I can at least occasionally get two jobs submitted at the same time during the period where there's no user feedback. This might be a consequence of 1), but I'm not really sure.
Otherwise, this is really good, and I'm definitely in favor of giving a user focused feedback on this sort of long running job.
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/1122
+1, this there's good stuff here. Thanks!
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/1122
@sardell Heads up, looks like you have some merge conflicts to take care off.
Thanks for taking care of those, that's a great fix to have in!
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by sardell <gi...@git.apache.org>.
Github user sardell commented on the issue:
https://github.com/apache/metron/pull/1122
@justinleet No problem at all. Those conflicts have now been resolved.
---
[GitHub] metron pull request #1122: METRON-1683: Fix the download progress bar in PCA...
Posted by sardell <gi...@git.apache.org>.
Github user sardell commented on a diff in the pull request:
https://github.com/apache/metron/pull/1122#discussion_r204802074
--- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-list/pcap-list.component.spec.ts ---
@@ -0,0 +1,75 @@
+/**
+ * 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.
+ */
+import { async, ComponentFixture, TestBed } from '@angular/core/testing';
+
+import { PcapListComponent } from './pcap-list.component';
+import { PcapPagination } from '../model/pcap-pagination';
+import { PcapPaginationComponent } from '../pcap-pagination/pcap-pagination.component';
+import { FormsModule } from '../../../../node_modules/@angular/forms';
+import { PdmlPacket } from '../model/pdml';
+import { Component, Input } from '@angular/core';
+import { PcapPacketLineComponent } from '../pcap-packet-line/pcap-packet-line.component';
+import { PcapPacketComponent } from '../pcap-packet/pcap-packet.component';
+
+@Component({
+ selector: '[app-pcap-packet-line]',
+ template: ``,
+})
+class FakePcapPacketLineComponent {
+ @Input() packet: PdmlPacket;
+}
+
+@Component({
+ selector: 'app-pcap-packet',
+ template: ``,
+})
+class FakePcapPacketComponent {
+ @Input() packet: PdmlPacket;
+}
+
+describe('PcapListComponent', () => {
+ let component: PcapListComponent;
+ let fixture: ComponentFixture<PcapListComponent>;
+
+ beforeEach(async(() => {
+ TestBed.configureTestingModule({
+ imports: [
+ FormsModule
+ ],
+ declarations: [
+ FakePcapPacketLineComponent,
+ FakePcapPacketComponent,
+ PcapListComponent,
+ PcapPaginationComponent
+ ]
+ })
+ .compileComponents();
+ }));
+
+ beforeEach(() => {
+ fixture = TestBed.createComponent(PcapListComponent);
+ component = fixture.componentInstance;
+ component.pagination = new PcapPagination();
+ component.pagination.total = 10;
+ fixture.detectChanges();
+ });
+
+ it('should create', () => {
+ expect(component).toBeTruthy();
--- End diff --
This is the auto-generated init test from angular cli. I didn't remove it because all of the test files have this test as a "default" test. If we remove all of these tests, I think it should be addressed as a separate issue outside of the PCAP work. I added a test to cover the onPageChange method.
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on the issue:
https://github.com/apache/metron/pull/1122
> Have you included steps to reproduce the behavior or problem that is being changed or addressed?
>
> Have you included steps or a guide to how the change may be verified and tested manually?
Can you submit the test script/instructions for this? It's checked off in the list, but has not provided.
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/1122
Thanks for the recent changes @sardell. I think we're almost there. Do you mind taking a look at pcap.service.spec.ts? As far as I can tell only getPackets is tested and it contains a large amount of test data. Can we use the mock data here as well?
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by tiborm <gi...@git.apache.org>.
Github user tiborm commented on the issue:
https://github.com/apache/metron/pull/1122
Opened a PR for pcap-packet-line.component.spec.ts. Component covered with unit test. Size of test data aligned to the needs of the unit tests and shared across pcap-packet and pcap-packet-line components.
---
[GitHub] metron pull request #1122: METRON-1683: Fix the download progress bar in PCA...
Posted by sardell <gi...@git.apache.org>.
Github user sardell commented on a diff in the pull request:
https://github.com/apache/metron/pull/1122#discussion_r204801057
--- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.ts ---
@@ -0,0 +1,71 @@
+import { Component, OnInit, Input } from '@angular/core';
+
+import { PcapService, PcapStatusResponse } from '../service/pcap.service';
+import { PcapRequest } from '../model/pcap.request';
+import { Pdml } from '../model/pdml';
+import {Subscription} from 'rxjs/Rx';
+import { PcapPagination } from '../model/pcap-pagination';
+
+class Query {
+ id: String
+}
+
+@Component({
+ selector: 'app-pcap-panel',
+ templateUrl: './pcap-panel.component.html',
+ styleUrls: ['./pcap-panel.component.scss']
+})
+export class PcapPanelComponent {
+
+ @Input() pdml: Pdml = null;
+ @Input() pcapRequest: PcapRequest;
+ @Input() resetPaginationForSearch: boolean;
+
+ statusSubscription: Subscription;
+ queryRunning: boolean = false;
+ queryId: string;
+ progressWidth: number = 0;
+ pagination: PcapPagination = new PcapPagination();
+ savedPcapRequest: {};
+ selectedPage: number = 1;
+
+ constructor(private pcapService: PcapService ) { }
+
+ changePage(page) {
+ this.selectedPage = page;
+ this.pcapService.getPackets(this.queryId, this.selectedPage).toPromise().then(pdml => {
+ this.pdml = pdml;
+ });
+ }
+
+ onSearch(pcapRequest, resetPaginationForSearch = true, from = 1) {
+ this.savedPcapRequest = pcapRequest;
+ if (resetPaginationForSearch === true) {
+ pcapRequest.from = 1;
--- End diff --
Thanks for pointing this out, @merrimanr. I updated the component so that the selectedPage always resets to 1 when a user submits a new query. I also removed the `from` field of the pcapRequest object and am now instead referencing `pagination.selectedPage` to determine the current page.
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:
https://github.com/apache/metron/pull/1122
Right now there's no UI indication that a job has completed with no results. The download bar finishes, and disappears leaving the user to wonder where results are. Can we improve that portion of the user experience?
---
[GitHub] metron pull request #1122: METRON-1683: Fix the download progress bar in PCA...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on a diff in the pull request:
https://github.com/apache/metron/pull/1122#discussion_r204552793
--- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-list/pcap-list.component.spec.ts ---
@@ -0,0 +1,75 @@
+/**
+ * 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.
+ */
+import { async, ComponentFixture, TestBed } from '@angular/core/testing';
+
+import { PcapListComponent } from './pcap-list.component';
+import { PcapPagination } from '../model/pcap-pagination';
+import { PcapPaginationComponent } from '../pcap-pagination/pcap-pagination.component';
+import { FormsModule } from '../../../../node_modules/@angular/forms';
+import { PdmlPacket } from '../model/pdml';
+import { Component, Input } from '@angular/core';
+import { PcapPacketLineComponent } from '../pcap-packet-line/pcap-packet-line.component';
+import { PcapPacketComponent } from '../pcap-packet/pcap-packet.component';
+
+@Component({
+ selector: '[app-pcap-packet-line]',
+ template: ``,
+})
+class FakePcapPacketLineComponent {
+ @Input() packet: PdmlPacket;
+}
+
+@Component({
+ selector: 'app-pcap-packet',
+ template: ``,
+})
+class FakePcapPacketComponent {
+ @Input() packet: PdmlPacket;
+}
+
+describe('PcapListComponent', () => {
+ let component: PcapListComponent;
+ let fixture: ComponentFixture<PcapListComponent>;
+
+ beforeEach(async(() => {
+ TestBed.configureTestingModule({
+ imports: [
+ FormsModule
+ ],
+ declarations: [
+ FakePcapPacketLineComponent,
+ FakePcapPacketComponent,
+ PcapListComponent,
+ PcapPaginationComponent
+ ]
+ })
+ .compileComponents();
+ }));
+
+ beforeEach(() => {
+ fixture = TestBed.createComponent(PcapListComponent);
+ component = fixture.componentInstance;
+ component.pagination = new PcapPagination();
+ component.pagination.total = 10;
+ fixture.detectChanges();
+ });
+
+ it('should create', () => {
+ expect(component).toBeTruthy();
--- End diff --
What is this spec testing? I would expect tests for toggle and onPageChange.
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by ruffle1986 <gi...@git.apache.org>.
Github user ruffle1986 commented on the issue:
https://github.com/apache/metron/pull/1122
@merrimanr Yes. accidentally, we forgot to change `fdescribe` for `describe` in the pcap-list component's test file before performing the commit. It means that the runner runs those tests only.
https://github.com/sardell/metron/blob/METRON-1683/metron-interface/metron-alerts/src/app/pcap/pcap-list/pcap-list.component.spec.ts#L46
cc @sardell
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by sardell <gi...@git.apache.org>.
Github user sardell commented on the issue:
https://github.com/apache/metron/pull/1122
@merrimanr No problem. I began work on the service tests yesterday and should have time to finish things up later today.
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/1122
I tested this in full dev and I think it's working well from a functional perspective.
I do believe the tests are still lacking though. Here are the issues I found:
- When I run `npm test` I see this:
```
Executed 4 of 63 (skipped 59)
```
Why are tests being skipped? Is it something with my environment?
- It looks like `pcap.service.spec.ts` hasn't been updated in a while
- As far as I can tell, `pcap-panel.component.spec.ts` isn't testing the onSearch function
- I think we're still missing tests in `pcap-packet.component.spec.ts`
- I see sample data declared as `fakePacket` in some tests. Can we make this smaller? It seems like this test data is bigger than it needs to be and redundant. Is there a way to share this across tests?
---
[GitHub] metron pull request #1122: METRON-1683: Fix the download progress bar in PCA...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on a diff in the pull request:
https://github.com/apache/metron/pull/1122#discussion_r204895078
--- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-list/pcap-list.component.spec.ts ---
@@ -0,0 +1,75 @@
+/**
+ * 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.
+ */
+import { async, ComponentFixture, TestBed } from '@angular/core/testing';
+
+import { PcapListComponent } from './pcap-list.component';
+import { PcapPagination } from '../model/pcap-pagination';
+import { PcapPaginationComponent } from '../pcap-pagination/pcap-pagination.component';
+import { FormsModule } from '../../../../node_modules/@angular/forms';
+import { PdmlPacket } from '../model/pdml';
+import { Component, Input } from '@angular/core';
+import { PcapPacketLineComponent } from '../pcap-packet-line/pcap-packet-line.component';
+import { PcapPacketComponent } from '../pcap-packet/pcap-packet.component';
+
+@Component({
+ selector: '[app-pcap-packet-line]',
+ template: ``,
+})
+class FakePcapPacketLineComponent {
+ @Input() packet: PdmlPacket;
+}
+
+@Component({
+ selector: 'app-pcap-packet',
+ template: ``,
+})
+class FakePcapPacketComponent {
+ @Input() packet: PdmlPacket;
+}
+
+describe('PcapListComponent', () => {
+ let component: PcapListComponent;
+ let fixture: ComponentFixture<PcapListComponent>;
+
+ beforeEach(async(() => {
+ TestBed.configureTestingModule({
+ imports: [
+ FormsModule
+ ],
+ declarations: [
+ FakePcapPacketLineComponent,
+ FakePcapPacketComponent,
+ PcapListComponent,
+ PcapPaginationComponent
+ ]
+ })
+ .compileComponents();
+ }));
+
+ beforeEach(() => {
+ fixture = TestBed.createComponent(PcapListComponent);
+ component = fixture.componentInstance;
+ component.pagination = new PcapPagination();
+ component.pagination.total = 10;
+ fixture.detectChanges();
+ });
+
+ it('should create', () => {
+ expect(component).toBeTruthy();
--- End diff --
I would prefer we not remove any tests. What I meant was we should make sure code included in this PR is covered by tests. I realize this is built on some preexisting PRs so we can delegate creating tests to the appropriate PR.
---
[GitHub] metron pull request #1122: METRON-1683: Fix the download progress bar in PCA...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on a diff in the pull request:
https://github.com/apache/metron/pull/1122#discussion_r204553631
--- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.ts ---
@@ -0,0 +1,71 @@
+import { Component, OnInit, Input } from '@angular/core';
+
+import { PcapService, PcapStatusResponse } from '../service/pcap.service';
+import { PcapRequest } from '../model/pcap.request';
+import { Pdml } from '../model/pdml';
+import {Subscription} from 'rxjs/Rx';
+import { PcapPagination } from '../model/pcap-pagination';
+
+class Query {
+ id: String
+}
+
+@Component({
+ selector: 'app-pcap-panel',
+ templateUrl: './pcap-panel.component.html',
+ styleUrls: ['./pcap-panel.component.scss']
+})
+export class PcapPanelComponent {
+
+ @Input() pdml: Pdml = null;
+ @Input() pcapRequest: PcapRequest;
+ @Input() resetPaginationForSearch: boolean;
+
+ statusSubscription: Subscription;
+ queryRunning: boolean = false;
+ queryId: string;
+ progressWidth: number = 0;
+ pagination: PcapPagination = new PcapPagination();
+ savedPcapRequest: {};
+ selectedPage: number = 1;
+
+ constructor(private pcapService: PcapService ) { }
+
+ changePage(page) {
+ this.selectedPage = page;
+ this.pcapService.getPackets(this.queryId, this.selectedPage).toPromise().then(pdml => {
+ this.pdml = pdml;
+ });
+ }
+
+ onSearch(pcapRequest, resetPaginationForSearch = true, from = 1) {
+ this.savedPcapRequest = pcapRequest;
+ if (resetPaginationForSearch === true) {
+ pcapRequest.from = 1;
--- End diff --
Shouldn't this set the selectedPage to 1? The `from` property on PcapRequest doesn't do anything. I am seeing this bug:
- execute a search
- navigate to page 2 in the results
- execute another search
- the search results are retrieved for the page I was previously on instead of the first page
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:
https://github.com/apache/metron/pull/1122
Due to a bug in apache rat, our license checking component, it's ignoring all license checks on typescript files. I have submitted a PR to up the rat plugin version and have fixed it in master #1126, let's make sure to add license headers here for new typescript files added in this PR though.
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by sardell <gi...@git.apache.org>.
Github user sardell commented on the issue:
https://github.com/apache/metron/pull/1122
@merrimanr I've increased test coverage in the pcap service to cover all functions there. I also removed that large set of test pdml data and replaced it with mock data.
---
[GitHub] metron pull request #1122: METRON-1683: Fix the download progress bar in PCA...
Posted by sardell <gi...@git.apache.org>.
Github user sardell commented on a diff in the pull request:
https://github.com/apache/metron/pull/1122#discussion_r204802426
--- Diff: metron-interface/metron-alerts/src/app/pcap/model/pcap.request.ts ---
@@ -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
+ *
+ * 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.
+ */
+
+export class PcapRequest {
+ startTimeMs: number = 0;
+ endTimeMs: number = 0;
+ ipSrcAddr: string = '';
+ ipSrcPort: string = '';
+ ipDstAddr: string = '';
+ ipDstPort: string = '';
+ protocol: string = '';
+ packetFilter: string = '';
+ includeReverse: boolean = false;
+ from?: number;
--- End diff --
This property has now been removed. Thanks for pointing this out.
---
[GitHub] metron pull request #1122: METRON-1683: Fix the download progress bar in PCA...
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on a diff in the pull request:
https://github.com/apache/metron/pull/1122#discussion_r204552197
--- Diff: metron-interface/metron-alerts/src/app/pcap/model/pcap.request.ts ---
@@ -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
+ *
+ * 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.
+ */
+
+export class PcapRequest {
+ startTimeMs: number = 0;
+ endTimeMs: number = 0;
+ ipSrcAddr: string = '';
+ ipSrcPort: string = '';
+ ipDstAddr: string = '';
+ ipDstPort: string = '';
+ protocol: string = '';
+ packetFilter: string = '';
+ includeReverse: boolean = false;
+ from?: number;
--- End diff --
This property doesn't do anything and is ignored by the API. It should be removed.
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:
https://github.com/apache/metron/pull/1122
@sardell this is merged. Can you close?
---
[GitHub] metron pull request #1122: METRON-1683: Fix the download progress bar in PCA...
Posted by sardell <gi...@git.apache.org>.
Github user sardell closed the pull request at:
https://github.com/apache/metron/pull/1122
---
[GitHub] metron issue #1122: METRON-1683: Fix the download progress bar in PCAP UI
Posted by sardell <gi...@git.apache.org>.
Github user sardell commented on the issue:
https://github.com/apache/metron/pull/1122
@justinleet Thanks for the feedback. I've updated the panel component to disable the submit button and display the progress bar on search submit rather than first response from REST.
---