Commit Graph

248 Commits

Author SHA1 Message Date
Oswaldo Ferreira bb8f2520b4 Persist truncated note diffs on a new table
We request Gitaly in a N+1 manner to build discussion diffs. Once the diffs are from different revisions, it's hard to make a single request to the service in order to build the whole response.
With this change we solve this problem and simplify a lot fetching this piece of info.
2018-05-24 15:34:43 -03:00
Bob Van Landuyt 5f7f5eda92 Method to track recoverable exceptions in sentry
This adds a method to track errors that can be recovered from in
sentry.

It is useful when debugging performance issues, or exceptions that are
hard to reproduce.
2018-04-17 11:39:23 +02:00
Sean McGivern bb226a294b Ensure that we never assume old_blob or new_blob are nil
These can be a `BatchLoader` which is proxying a nil, while not being concrete
nils themselves.
2018-03-16 18:19:33 +00:00
Sean McGivern 6cd7f679d0 Only cache highlight results for latest MR diffs
Previously, we kept them all in the cache. We don't need the highlight results
for older diffs - if someone does view that (which is rare), we can do the
highlighting on the fly.
2018-03-15 11:49:53 +00:00
Sean McGivern cdf3ae04f8 Fix 500 error when diff context line has broken encoding
Rugged sometimes chops a context line in between bytes, resulting in the context
line having an invalid encoding: https://github.com/libgit2/rugged/issues/716

When that happens, we will try to detect the encoding for the diff, and
sometimes we'll get it wrong. If that difference in encoding results in a
difference in string lengths between the diff and the underlying blobs, we'd
fail to highlight any inline diffs, and return a 500 status to the user.

As we're using the underlying blobs, the encoding is 'correct' anyway, so if the
string range is invalid, we can just discard the inline diff highlighting. We
still report to Sentry to ensure that we can investigate further in future.
2018-02-22 12:26:23 +00:00
Stan Hu c2b7147c2f Fix error 500 when viewing commit and merge request diffs
Due to the refactoring in !16082, `Blob#batch` no longer falls back
to a default `blob_size_limit`. Since `Repository#batch_blobs` was using
a default `nil` value, this would cause issues in the `Blob#find_by_rugged`
method.

This fix here is to be consistent and use a non-nil default value in
`Repository#batch_blobs`.

The problem was masked in development and tests because Gitaly is always
enabled by default for all features.

Closes #41735
2018-01-07 04:49:07 -08:00
Sean McGivern 528b5eeb76 Fix error when viewing diffs without blobs
Old merge requests can have diffs without corresponding blobs. (This also may be
possible for commit diffs in corrupt repositories.)

We can't use the `&.` operator on the blobs, because the blob objects are never
nil, but `BatchLoader` instances that delegate to `Blob`. We can't use
`Object#try`, because `Blob` doesn't inherit from `Object`.

`BatchLoader` provides a `__sync` method that returns the delegated object, but
using `itself` also works because it's forwarded, and will work for
non-`BatchLoader` instances too. So the simplest solution is to just use that
with the `&.` operator.
2018-01-04 14:33:12 +00:00
Martin Nowak b7a5125f02 fix #39233 - 500 in merge request
- handle unchanged empty lines in inline diff
2017-12-07 20:19:37 +01:00
Sean McGivern 3c6a4d6363 Ensure MRs always use branch refs for comparison
If a merge request was created with a branch name that also matched a tag name,
we'd generate a comparison to or from the tag respectively, rather than the
branch. Merging would still use the branch, of course.

To avoid this, ensure that when we get the branch heads, we prepend the
reference prefix for branches, which will ensure that we generate the correct
comparison.
2017-11-28 17:01:38 +00:00
Zeger-Jan van de Weg f9565e3039
Batchload blobs for diff generation
After installing a new gem, batch-loader, a construct can be used to
queue data to be fetched in bulk. The gem was also introduced in both
gitlab-org/gitlab-ce!14680 and gitlab-org/gitlab-ce!14846, but those mrs
are not merged yet.

For the generation of diffs, both the old blob and the new blob need to
be loaded. This for every file in the diff, too. Now we collect all
these so we do 1 fetch. Three `.allow_n_plus_1_calls` have been removed,
which I expect to be valid, but this needs to be confirmed by a full CI
run.

Possibly closes:
- https://gitlab.com/gitlab-org/gitlab-ce/issues/37445
- https://gitlab.com/gitlab-org/gitlab-ce/issues/37599
- https://gitlab.com/gitlab-org/gitlab-ce/issues/37431
2017-11-21 13:53:26 +01:00
AlexWayfer 7ba7fa5048 Fix 500 error for old (somewhat) MRs 2017-10-30 12:30:31 +00:00
Sean McGivern b2553840e8 Merge branch 'conflict-resolution-refactor' into 'master'
Conflict resolution refactor

See merge request gitlab-org/gitlab-ce!14747
2017-10-16 10:36:06 +00:00
Valery Sizov d09895a0de Fix diff parser so it tolerates to diff special markers in the content 2017-10-15 17:00:36 +03:00
Alejandro Rodríguez 9fdde3693b Move line code generation into Gitlab::Git
Having a distinct class just for that was a bit overkill
2017-10-12 22:13:05 -03:00
Alejandro Rodríguez faa9bd402d Create a Gitlab::Git submodule for conlict-related files
Rename classes to (hopefully) clearer names while we're doing that.
2017-10-12 22:03:15 -03:00
Alejandro Rodríguez f72598b659 Move Gitlab::Diff::LineCode to module Gitlab::Git 2017-10-12 21:45:16 -03:00
Felipe Artur b54203f0ad Commenting on image diffs 2017-10-07 04:25:17 +00:00
Douwe Maan c19030332c Add specs 2017-09-25 16:56:14 +02:00
Jacob Vosmaer 902b5347dc Prepare Repository#merge for migration to Gitaly 2017-09-15 16:39:20 +02:00
Sean McGivern e8525e107d Show un-highlighted diffs when blobs are the same
For some old merge requests, we don't have enough information to figure out the
old blob and the new blob for the file. This means that we can't highlight the
diff correctly, but we can still display it without highlighting.
2017-08-24 11:11:18 +01:00
Sean McGivern b0f09406f5 Always return a simple diff viewer
We didn't have a fallback case before, because we believed the conditions were
exhaustive. They weren't, so we can always fallback to not previewing.
2017-08-22 14:04:54 +01:00
Jarka Kadlecova c271fdc80f Don't escape html entities when rich == raw line 2017-08-17 19:55:07 +02:00
Robert Speicher 9513bd18c4 Ensure all project factories use `:repository` trait or `:empty_project` 2017-08-01 14:51:52 -04:00
Jacob Vosmaer 7b18c42464 Remove unused (?) code 2017-07-31 17:19:25 +02:00
Rémy Coutable cddc5cacfb Use described_class when possible
Signed-off-by: Rémy Coutable <remy@rymai.me>
2017-07-27 14:31:53 +02:00
Rémy Coutable ddccd24c13 Remove superfluous lib: true, type: redis, service: true, models: true, services: true, no_db: true, api: true
Signed-off-by: Rémy Coutable <remy@rymai.me>
2017-07-27 14:31:53 +02:00
Douwe Maan 7944254563 Implement diff viewers 2017-06-14 10:12:21 -05:00
Douwe Maan ffbbd4112e Move diffable? method from Repository to Diff::File 2017-06-08 09:32:57 -05:00
Douwe Maan 3a5d375e49 Fix Diff::Position#diff_file for positions on straight diffs 2017-06-06 14:48:05 -05:00
Robert Speicher 8e2fefc6c4 Merge branch 'dm-diff-cleanup' into 'master'
Clean up diff rendering

See merge request !11390
2017-05-25 22:15:57 +00:00
Douwe Maan ab91f76e8b Add system note with link to diff comparison when MR discussion becomes outdated 2017-05-23 16:27:30 -05:00
Douwe Maan d9a0188d2f Add question mark to Gitlab::Diff::File predicate methods 2017-05-23 15:37:04 -05:00
Sean McGivern ec1a3c093f Merge branch 'dm-dependency-linker-gemfile' into 'master'
Autolink package names in Gemfile

See merge request !11224
2017-05-12 17:27:42 +00:00
Rémy Coutable d40e1f547e Enable the Style/TrailingCommaInLiteral cop
Use the EnforcedStyleForMultiline: no_comma option.

Signed-off-by: Rémy Coutable <remy@rymai.me>
2017-05-10 18:25:45 +02:00
Douwe Maan 02650d97e5 Fix specs 2017-05-10 08:26:21 -05:00
Douwe Maan e179707844 Extract generic parts of Gitlab::Diff::InlineDiffMarker 2017-05-10 08:26:21 -05:00
Douwe Maan 63d38a303b Fix commenting on an existing discussion on an unchanged line that is no longer in the diff 2017-04-24 11:17:15 -05:00
Douwe Maan d170133bde Refactor changing files in web UI 2017-04-20 00:37:44 +00:00
Douwe Maan f90909307e Fix specs 2017-03-14 15:29:31 -06:00
mhasbini 985af1a670 take nonewline context into account in diff parser 2017-03-13 22:09:43 +02:00
Douwe Maan bdbc7d967a Revert "Enable Style/BarePercentLiterals"
This reverts commit 96bef54154e669f9a3e92c3a4bc76c0be3a52e48.
2017-02-23 09:32:42 -06:00
Douwe Maan baafd8de26 Enable Style/BarePercentLiterals 2017-02-23 09:31:57 -06:00
dixpac fa2339641f Rename Files::DeleteService to Files::DestroyService
Reason for renaming is to comply with naming convention of services in
codebase.
2017-02-10 18:22:18 +01:00
Sean McGivern 7550f60dde Backport changes from EE squash
Backport changes from the EE-only squash implementation, which would
otherwise conflict when merge CE into EE.

<https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1024>
2017-02-03 09:46:03 +00:00
Douwe Maan e7d530a624 Merge branch 'fix-git-hooks-when-creating-file' into 'master'
Don't execute git hooks if you create branch as part of other change

Closes #23439

See merge request !7237
2017-02-02 23:26:35 +00:00
Valery Sizov ceb1ebd959 Active tense test coverage
Ports changes from https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/642 back into CE
2017-02-01 22:45:41 +00:00
Lin Jen-Shin 406dfd6e0f Merge remote-tracking branch 'upstream/master' into fix-git-hooks-when-creating-file
* upstream/master:
  Ensure we have a project with a repo in GitlabMarkdownHelper specs
  Revert "Make sure TraceReader uses Encoding.default_external"
  Make sure TraceReader uses Encoding.default_external
  Update CONTRIBUTING.md after merging "up-for-grabs" and "Accepting Merge Requests" [ci skip]
  Use `:empty_project` where possible in finder specs
  Use `empty_project` where possible in controller specs
  Use `:empty_project` where possible in helper specs
  Don’t count tasks that are not defined as list items correctly
  Use a project factory with a repository where necessary
  Use `:empty_project` where possible throughout spec/lib
  Use hashrocket for dasherized attribute
  Remove markdown file extension and add anchor to link
  Fixed builds info link on project settings page
  Factories with a project association use `:empty_project` by default
  Update enviroments.md the example for deleting an environment is missing the "s" in environments. curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/environments/1"  wil 404
2017-01-26 22:19:50 +08:00
Robert Speicher 80a6d2fda2 Use `:empty_project` where possible throughout spec/lib 2017-01-25 12:25:42 -05:00
Lin Jen-Shin a639454032 Fix renaming 2017-01-07 00:31:11 +08:00
Sean McGivern 1e5e56c698 Fix MR with files hidden by .gitattributes
Don't try to highlight and cache files hidden by .gitattributes entries.
2016-12-26 16:25:55 +00:00
Valery Sizov f5d7a61760 Fixes ActionView::Template::Error: undefined method `text?` for nil:NilClass 2016-12-02 14:03:30 +02:00
Valery Sizov 847ada36c4 Fix: Timeout creating and viewing merge request for binary file 2016-11-25 15:25:01 +02:00
Stan Hu 0f61bb2dc5
Fix Error 500 when creating a merge request that contains an image that was deleted and added
Steps to reproduce:

1. Start with a repo with an image
2. Add a commit to delete the image
3. Add another commit to replace the image with another image

In a diff comparison, we really just compare about what the image was before the diff, not
the direct parent of the last commit. This MR fixes that.

Closes #3893, gitlab-org/gitlab-ee#678

Signed-off-by: Rémy Coutable <remy@rymai.me>
2016-11-14 15:57:01 +01:00
Sean McGivern 4a4c1765be Fix line commenting for the initial commit
The initial commit doesn't have a parent, so explicitly pass the blank
SHA and handle that when calculating the position.
2016-08-19 16:35:44 +01:00
Paco Guzman c86c1905b5 switch from diff_file_collection to diffs
So we have raw_diffs too
2016-08-03 07:00:20 +02:00
Douwe Maan 79214be727 Add Discussion model to represent MR/diff discussion 2016-07-20 16:18:18 -06:00
Sean McGivern 4f0780cc04 Ensure to_json methods take optional argument 2016-07-20 11:14:06 +01:00
Douwe Maan f0b446e555 Merge branch '19820-safer-diffs' into 'master'
Collapsed diffs lines/size don't accumulate to overflow diffs.

## What does this MR do?

Reduce the number of lines that we highlight on big diffs to try to reduce the degradation introduced by  !4990 as you can see on the issue description #19820 .

We collapse any files after the diff is over the number of safe files (100), or over the safe lines (500) or over the safe number of bytes (5KB x 100 files).

We still need to bump the gitlab_git and point this branch to the new gitlab_git version.

## What are the relevant issue numbers?

Closes #19820 
Closes #19885

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- ~~[ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~
- ~~[ ] API support added~~
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)


See merge request !5306
2016-07-18 21:28:24 +00:00
Paco Guzman a404ab380d Collapsed diffs lines/size don't accumulate to overflow diffs. 2016-07-18 14:43:28 -06:00
Rémy Coutable 2cf7f09b1e
Revert "Revert "Merge branch '18193-developers-can-merge' into 'master' ""
This reverts commit 530f5158e2.

See !4892.

Signed-off-by: Rémy Coutable <remy@rymai.me>
2016-07-18 10:16:56 +02:00
http://jneen.net/ e9f191907c apparently this gets encoded now? 2016-07-14 10:08:15 -07:00
http://jneen.net/ ff7e679bca we no longer encode double-quotes 2016-07-14 10:08:15 -07:00
Robert Speicher 530f5158e2 Revert "Merge branch '18193-developers-can-merge' into 'master' "
This reverts commit 9ca633eb4c, reversing
changes made to fb229bbf79.
2016-07-13 13:57:30 -05:00
Timothy Andrew 495db09653 Enforce "developers can merge" during `pre-receive`.
1. When a merge request is being merged, save the merge commit SHA in
   the `in_progress_merge_commit_sha` database column.

2. The `pre-receive` hook looks for any locked (in progress) merge
   request with `in_progress_merge_commit_sha` matching the `newrev` it
   is passed.

3. If it finds a matching MR, the merge is legitimate.

4. Update `git_access_spec` to test the behaviour we added here. Also
   refactored this spec a bit to make it easier to add more contexts / conditions.
2016-07-13 13:24:56 +05:30
Douwe Maan 5fea640e90 Merge branch 'multi-line-inline-diff' into 'master'
Render inline diffs for multiple changed lines following eachother

Before:

![Screen_Shot_2016-07-11_at_00.08.27](/uploads/b14664211e0f5cef6e77a78eadfcbcdf/Screen_Shot_2016-07-11_at_00.08.27.png)

After:

![Screen_Shot_2016-07-11_at_00.07.34](/uploads/567be631869a4867a2edf6ff7eda6369/Screen_Shot_2016-07-11_at_00.07.34.png)

See merge request !5174
2016-07-13 05:29:49 +00:00
Robert Speicher adc6ec4a9c Avoid `describe`-ing symbols in specs 2016-07-12 10:27:58 -05:00
Douwe Maan f5cc3f63a8 Render inline diffs for multiple changed lines following eachother 2016-07-11 18:58:15 -05:00
Douwe Maan e0ecfe5bd1 Add tests for Position 2016-07-06 18:51:01 -04:00
Douwe Maan ebb5f591e3 Add tests for PositionTracer 2016-07-06 18:51:00 -04:00
Douwe Maan e9e06ca627 Add Gitlab::Diff::LineMapper 2016-07-06 18:50:59 -04:00
Douwe Maan a9fa45f09e Represent DiffRefs as proper class instead of tuple array 2016-07-06 18:50:58 -04:00
Grzegorz Bizon 9e211091a8 Enable Style/EmptyLines cop, remove redundant ones 2016-07-01 21:56:17 +02:00
Yorick Peterse 3f0d780c19 Show a notice for diffs that are too large
This builds on the changes introduced in
https://gitlab.com/gitlab-org/gitlab_git/merge_requests/72 and results
in merge requests with large diffs (e.g. due to them containing minified
CSS) loading much faster.
2016-03-18 12:30:46 +01:00
Rubén Dávila 7e03b40221 Return an empty Array when there aren't lines to parse. 2016-03-15 19:51:27 -05:00
Jacob Vosmaer 1764e1b7cb Use Gitlab::Git::DiffCollections 2016-03-03 18:38:44 +01:00
Douwe Maan fa0cbb1399 Fix specs and add a new one 2016-01-30 12:53:12 +01:00
Douwe Maan 677b4db9e6 Mark inline difference between old and new paths when a file is renamed 2016-01-29 19:37:17 +01:00
Douwe Maan 26f7d023e6 Update tests 2016-01-20 17:14:26 +01:00
Douwe Maan a010db5db2 Properly handle HTML entities with inline diffs 2016-01-20 15:26:44 +01:00
Douwe Maan 701513dcc7 Move parallel diff logic to separate class 2016-01-20 14:53:20 +01:00
Douwe Maan 512bebe21d Refactor Gitlab::Highlight and fix tests 2016-01-19 14:52:41 +01:00
Douwe Maan 12546f895f Add tests 2016-01-15 16:29:26 +01:00
Rubén Dávila 6b9c730e91 More refactoring from last code review. #3945
* Use commit objects instead of IDs when generating diffs
* Use proper references when generating MR's source and target
* Update broken specs
2016-01-14 16:47:55 -05:00
Douwe Maan 8dfad143d4 Add inline diff markers in highlighted diffs. 2016-01-14 22:28:07 +01:00
Rubén Dávila f547e733d1 Add more specs. #3945 2016-01-12 17:49:11 -05:00
Rubén Dávila 78d7c0e0d8 Fix broken specs. #3945 2016-01-08 19:05:55 -05:00
Rubén Dávila 52f8286a02 Update specs. #3945 2016-01-08 18:40:05 -05:00
Rubén Dávila 3fbcf52ec8 Apply syntax highlighting when expanding diff plus some refactor. #3945 2015-12-31 01:05:52 -05:00
Rubén Dávila fd100e1ef1 Don't modify "match" diff lines. #3945 2015-12-30 21:44:12 -05:00
Rubén Dávila 8b079315d9 A bit of refactoring. #3945 2015-12-30 21:23:50 -05:00
Rubén Dávila d83275620a Add specs for Gitlab::Diff::Highlight. #3945 2015-12-30 20:18:40 -05:00
Douwe Maan 13d6bab177 Tag lib specs 2015-12-09 11:55:42 +01:00
Dmitriy Zaporozhets 3c16fb93fc
Move spec to proper place and fix unused variable
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
2015-11-13 09:45:21 +01:00
Stan Hu c36adb98aa Fix bug where backslashes in inline diffs could be dropped
Closes #2253
2015-08-11 18:39:27 -07:00
Douwe Maan 8ed7ac9d44 Use project.commit convenience method. 2015-04-24 12:29:36 +02:00
Jeroen van Baarsen 0c4a70a306 Updated rspec to rspec 3.x syntax
Signed-off-by: Jeroen van Baarsen <jeroenvanbaarsen@gmail.com>
2015-02-12 19:17:35 +01:00
Dmitriy Zaporozhets 218219abbd
Refactoring inside refactoring. We need to go deeper
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
2014-09-08 21:54:52 +03:00
Dmitriy Zaporozhets bde3f25d26
Specs for diff parser! Yay!
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
2014-09-08 20:42:12 +03:00